RasppleII / a2server

AppleTalk server for Apple II computers
Other
31 stars 8 forks source link

Version format limitations (suggestion semantic versioning as an alternative) #49

Closed knghtbrd closed 7 years ago

knghtbrd commented 8 years ago

Currently the A2SERVER scripts use version strings like "126". This is variously interpreted as 1.26 or 1.2.6 and currently must be three digits.

Semantic Versioning suggests a well-known version format of major.minor.patch with some sort of strict definitions of when each should be incremented:

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards-compatible manner, and PATCH version when you make backwards-compatible bug fixes. Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

It's clear that this is somewhere in the back of @IvanExpert's mind since he's tagged 1.2.7fc1 more or less following this manner, however our existing scripts have no means of working with such a version string and must inevitably break one of these rules when a version component reaches 9 as a matter of backwards compatibility.

I propose the following snippet for dealing with the existing version files:

if [[ -f /usr/local/etc/A2SERVER-version ]]; then
    installedVersion=$(cat /usr/local/etc/A2SERVER-version)
    if [[ "$installedVersion" != *.*.* ]]; then
        if [[ "$installedVersion" == [0-9][0-9][0-9] ]]; then
            installedVersion="${installedVersion:0:1}.${installedVersion:1:1}.${installedVersion:2:1}"
        else
            # Whatever it is, it's not a recognized version
            installedVersion="1.0.0"
        fi
    else
else
    installedVersion="1.0.0"
fi

currentVersion would follow the semantic version format guidelines, at the moment being "1.2.7-fc1". We don't currently have a version comparator, but the logic for checking it would be an optimized version of something more complete than this:

is_equal=
is_less=
is_greater=

if (( "$A_major" < "$B_major" )); then
    is_less=1
elif (( "$A_major" > "$B_major" )); then
    is_greater=1
else
    if (( "$A_minor" < "$B_minor" )); then
        is_less=1
    elif (( "$A_minor" > "$B_minor" )); then
        is_greater=1
    else
        if (( "$A_patch" < "$B_patch" )); then
            is_less=1
        elif (( "$A_patch" > "$B_patch" )); then
            is_greater=1
        else
            # Fall back on ASCII
            if [ -n "$A_special" ]; then
                if [ -n "$B_special" ]; then
                    # both have a special, compare them
                    if [ "$A_special" < "$B_special" ]; then
                        is_less=1
                    elif [ "$A_special" > "$B_special" ]; then
                        is_greater=1
                    else
                        is_equal=1
                    fi
                else
                    # Neither has a special, so they're equal
                    is_equal=1
                fi
            else
                if [ -n "$B_special" ]; then
                    # B has a special, so it's older
                    is_greater=1
                else
                    # Neither A nor B has a special
                    is_equal=1
                fi
            fi
        fi
    fi
fi

Yeah, less than optimal. Reasonably you'd have a function which takes two versions and an operator, simplifies that to a simple operator and possible negation, and you MIGHT want a semantic comparison of specials instead of an ascii comparison. ASCII compares work assuming that you have alpha, beta, pre, and rc in that order, but you always need a special test since all of those are less than nothing at all.

And this is probably why one isn't implemented. :)

I'll implement this change in A2SERVER and likewise in A2CLOUD if Ivan's got no problem with it.

knghtbrd commented 8 years ago

My snippet has an else where it should have a fi.

if [[ -f /usr/local/etc/A2SERVER-version ]]; then
    installedVersion=$(cat /usr/local/etc/A2SERVER-version)
    if [[ "$installedVersion" != *.*.* ]]; then
        if [[ "$installedVersion" == [0-9][0-9][0-9] ]]; then
            installedVersion="${installedVersion:0:1}.${installedVersion:1:1}.${installedVersion:2:1}"
        else
            # Whatever it is, it's not a recognized version
            installedVersion="1.0.0"
        fi
    fi
else
    installedVersion="1.0.0"
fi
IvanExpert commented 8 years ago

Will consider, but I don't believe in, at least for my own software, release numbers in which minor or patch are more than one digit -- witness the hell that is OS X. Further, I'm happy with a subjective approach as to what the version number should be.

So the convention I've adopted is flexible using simple bash substring syntax, and easily evaluated as a simple integer. I'm not all that motivated to add complexity for a theoretical best practice that, at least for me and right now, has no practical application and solves no particular problem. With that said, I'll look more closely at this.

On Dec 9, 2015, at 3:55 AM, Joseph Carter notifications@github.com wrote:

Currently the A2SERVER scripts use version strings like "126". This is variously interpreted as 1.26 or 1.2.6 and currently must be three digits.

Semantic Versioning suggests a well-known version format of major.minor.patch with some sort of strict definitions of when each should be incremented:

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards-compatible manner, and PATCH version when you make backwards-compatible bug fixes. Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

It's clear that this is somewhere in the back of @IvanExpert's mind since he's tagged 1.2.7fc1 more or less following this manner, however our existing scripts have no means of working with such a version string and must inevitably break one of these rules when a version component reaches 9 as a matter of backwards compatibility.

I propose the following snippet for dealing with the existing version files:

if [[ -f /usr/local/etc/A2SERVER-version ]]; then installedVersion=$(cat /usr/local/etc/A2SERVER-version) if [[ "$installedVersion" != ..* ]]; then if [[ "$installedVersion" == [0-9][0-9][0-9] ]]; then installedVersion="${installedVersion:0:1}.${installedVersion:1:1}.${installedVersion:2:1}" else

Whatever it is, it's not a recognized version

        installedVersion="1.0.0"
    fi
else

else installedVersion="1.0.0" fi currentVersion would follow the semantic version format guidelines, at the moment being "1.2.7-fc1". We don't currently have a version comparator, but the logic for checking it would be an optimized version of something more complete than this:

is_equal= is_less= is_greater=

if (( "$A_major" < "$B_major" )); then is_less=1 elif (( "$A_major" > "$B_major" )); then is_greater=1 else if (( "$A_minor" < "$B_minor" )); then is_less=1 elif (( "$A_minor" > "$B_minor" )); then is_greater=1 else if (( "$A_patch" < "$B_patch" )); then is_less=1 elif (( "$A_patch" > "$B_patch" )); then is_greater=1 else

Fall back on ASCII

        if [ -n "$A_special" ]; then
            if [ -n "$B_special" ]; then
                # both have a special, compare them
                if [ "$A_special" < "$B_special" ]; then
                    is_less=1
                elif [ "$A_special" > "$B_special" ]; then
                    is_greater=1
                else
                    is_equal=1
                fi
            else
                # Neither has a special, so they're equal
                is_equal=1
            fi
        else
            if [ -n "$B_special" ]; then
                # B has a special, so it's older
                is_greater=1
            else
                # Neither A nor B has a special
                is_equal=1
            fi
        fi
    fi
fi

fi Yeah, less than optimal. Reasonably you'd have a function which takes two versions and an operator, simplifies that to a simple operator and possible negation, and you MIGHT want a semantic comparison of specials instead of an ascii comparison. ASCII compares work assuming that you have alpha, beta, pre, and rc in that order, but you always need a special test since all of those are less than nothing at all.

And this is probably why one isn't implemented. :)

I'll implement this change in A2SERVER and likewise in A2CLOUD if Ivan's got no problem with it.

— Reply to this email directly or view it on GitHub.

knghtbrd commented 8 years ago

To be fair, OS X is a mess because Apple has OS X 10. which doesn't actually match the OS's own reported version. You can use almost any version system you like, and as long as you use it uniformly, consistently, and follow some rules also consistently for version naming, it works fine.

Apple does NONE of those things. OS X 10.11 El Capitan has three version components and a codename, and none of those is a real OS version. The OS version was 15.0.0. Everything else is a label, and confusingly inconsistent labels at that.

I've seen in our scripts X.YZ, X.Y.Z, and XYZ. And the assumption hard-coded is that there will and cannot be a version number higher than 999. We're a far cry from that now but if we start bumping minor versions and even major versions for reasons, we'll get there soon enough.

There's an advantage to using an integer version in terms of comparisons. A lot of C programs' versions would be take the components 1 and 2 and 7 or 1 and 27 and have multipliers. You'd lose -alpha, -beta, -pre, and -rc versions that way, but … a lot of projects never use those anyway.

I'd actually suggest that trashing this patch and rewriting one that keeps an integer, but treats it as an integer rather than a string resolves my concerns just as well. I'd suggest using 1 and 27 and multiply both in case we need a patch/pre-release/whatever in future. Version integer would bet 12700 and be displayed as 1.27, with the last 100 being used for pre-release snapshots between versions? Just a thought.

This at least removes the stupid complexity in version comparison with Linux-style semantic versioning if it doesn't address any other problems.

IvanExpert commented 8 years ago

On Dec 10, 2015, at 5:28 AM, Joseph Carter notifications@github.com wrote:

To be fair, OS X is a mess because Apple has OS X 10. which doesn't actually match the OS's own reported version. You can use almost any version system you like, and as long as you use it uniformly, consistently, and follow some rules also consistently for version naming, it works fine.

Apple does NONE of those things. OS X 10.11 El Capitan has three version components and a codename, and none of those is a real OS version. The OS version was 15.0.0. Everything else is a label, and confusingly inconsistent labels at that.

Ok, point taken that it's not a fair example, but I still don't like multidigit minor and patch numbers. I've seen in our scripts X.YZ, X.Y.Z, and XYZ.

In general, X.Y.Z should be what the user sees, and XYZ is what we're using internally. X.YZ, if it was me who put it there, is a mistake. And the assumption hard-coded is that there will and cannot be a version number higher than 999. We're a far cry from that now but if we start bumping minor versions and even major versions for reasons, we'll get there soon enough.

Sure, I guess I've never personally crossed that bridge with my projects. But if I get to a minor or patch number past nine, then it's time to rev the major patch number, either by a linear a jump increment. There's an advantage to using an integer version in terms of comparisons. A lot of C programs' versions would be take the components 1 and 2 and 7 or 1 and 27 and have multipliers. You'd lose -alpha, -beta, -pre, and -rc versions that way, but … a lot of projects never use those anyway.

I tend to use d, b, and fc during development, e.g. 1.2.8d1, 1.2.8d2, etc. These are never supposed to be public facing. I'd actually suggest that trashing this patch and rewriting one that keeps an integer, but treats it as an integer rather than a string resolves my concerns just as well. I'd suggest using 1 and 27 and multiply both in case we need a patch/pre-release/whatever in future. Version integer would bet 12700 and be displayed as 1.27, with the last 100 being used for pre-release snapshots between versions? Just a thought.

I'm open to that, but I'm not going to implement it in 1.3.0 (which I'm increasingly thinking should be 1.5.0, but that's another discussion). I'm sort of assuming that once 1.3.0 is done, it's yours to tool with unless some other exciting feature or need drags me out of retirement again, so if you want to rework the version numbers however you see fit at that point, go nuts. I don't feel super strongly about it as long as the user isn't exposed to ten digit gobbledygook.

knghtbrd commented 8 years ago

I'd be open to 1.5.0, particularly if we do address the three digit issue, which is just a matter of parsing the version file as an integer rather than as a string of a fixed length. It's a much smaller patch to do that.

We currently express the version to the user as "127" in setup/index.txt on line 26-29 and again on line 61. In update/index.txt we actually parse it as a string (not a number) and display it thusly:

echo "installed version: ${installedVersion:0:1}.${installedVersion:1:1}.${installedVersion:2:1}"
echo "current version: ${currentVersion:0:1}.${currentVersion:1:1}.${currentVersion:2:1}"

Joseph

IvanExpert commented 8 years ago

This is not anything I feel terribly strongly about; if you want to rework it for equivalent or superior functionality and greater under the hood beauty, or want to instruct me as to what you want, go for it.

So, it's decided that the forthcoming release will be 1.5.0.

knghtbrd commented 8 years ago

I still don't like using single characters for versions, and I fixed us doing that with Debian's version already, but our own still poses a problem because we have that backwards compatibility thing to deal with. I was playing with this in A2CLOUD and came up with this, which is simple enough:

version="1.8.4"

if [ -f /usr/local/etc/A2CLOUD-version ]; then
    installedVersion="$(cat /usr/local/etc/A2CLOUD-version)"
    if [[ $installedVersion != *.*.* ]]; then
        installedVersion="${installedVersion:0:1}.${installedVersion:1:1}.${installedVersion:2}"
    fi
fi

I started to write a simple function to compare two versions, but then looked and ... we don't actually do that anywhere. I'll write it when/if we do.

knghtbrd commented 7 years ago

Status: Yes, moving to semantic version numbers, but it hasn't been done, and I think it got reverted in A2CLOUD or was done in a branch I've since removed. Either way, it's still a good idea and it still works. Just .. not a high priority now.