Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(release): assure new release version is greater than the current one #376

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(release): assure new release version is greater than the current one #376

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions dist.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,29 @@ sedEscapeArg() {
echo "$@" | sed 's/\//\\\//g'
}

# Adapted from https://stackoverflow.com/a/44660519/702738
compare_version() {
if [[ $1 == $2 ]]; then
return 0
fi
local IFS=.
local i a=(${1%%[^0-9.]*}) b=(${2%%[^0-9.]*})
local arem=${1#${1%%[^0-9.]*}} brem=${2#${2%%[^0-9.]*}}
for ((i=0; i<${#a[@]} || i<${#b[@]}; i++)); do
if ((10#${a[i]:-0} < 10#${b[i]:-0})); then
return 1
elif ((10#${a[i]:-0} > 10#${b[i]:-0})); then
return 0
fi
done
if [ "$arem" '<' "$brem" ]; then
return 1
elif [ "$arem" '>' "$brem" ]; then
return 0
fi
return 1
}

case $1 in
new-go-dist)
name="$2"
Expand Down Expand Up @@ -58,7 +81,7 @@ case $1 in

# Create vtag file that contains version tag prefix
if [ "$tag_prefix" != "." ]; then
echo "$tag_prefix" > "dists/${name}/vtag"
echo "$tag_prefix" > "dists/$name/vtag"
fi

echo "distribution $name created successfully! To start build: make $name"
Expand All @@ -72,9 +95,20 @@ case $1 in
exit 1
fi

# Test if the new version is greater than the current version.
test_version() {
current_version=$([ -f "dists/$dist/current" ] && cat "dists/$dist/current" | sed "s/v//")
new_version=$(echo $nvers | sed "s/v//")

if compare_version "$current_version" "$new_version"; then
echo "ERROR: The version provided ($new_version) has to be greater than the current release ($current_version)."
exit 1
fi
Copy link
Contributor

@gammazero gammazero Jul 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not appear to catch the problem when:

current_version="2.3-rc1"                                                                                      
new_version="2.3"

I get:

ERROR: The version provided (2.3) has to be greater than the current release (2.3-rc1).

The version "2.3" should, semver-wise, be greater than "2.3-rc1".

Running a tool like semver-cli does catch this:

> semver-cli greater -v "2.3-rc1" "2.3"; echo "returned: $?"
2.3
returned: 1
> semver-cli greater -v "2.3" "2.3-rc1"; echo "returned: $?"
2.3
returned: 0

Consider using semver-cli and changing the logic above to:

if ! semver-cli greater "$new_version" "$current_version"; then
	echo "ERROR: The version provided ($new_version) has to be greater than the current release ($current_version)."
	exit 1                                                                      
fi

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gammazero, that's because 2.3-rc1 is not compliant with the SemVer specification, as noted on Item 2 of the specification:

A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers, and MUST NOT contain leading zeroes. X is the major version, Y is the minor version, and Z is the patch version. Each element MUST increase numerically. For instance: 1.9.0 -> 1.10.0 -> 1.11.0.

Thus, to be correctly formatted, the version must be separated by three dots.

There are some useful tips on the SemVer's FAQ that I use myself:

Q: Is there a suggested regular expression (RegEx) to check a SemVer string?

A: There are two. One with named groups for those systems that support them (PCRE [Perl Compatible Regular Expressions, i.e. Perl, PHP and R], Python and Go).
See: https://regex101.com/r/Ly7O1x/3/

^(?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)\.(?P<patch>0|[1-9]\d*)(?:-(?P<prerelease>(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?P<buildmetadata>[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$

And one with numbered capture groups instead (so cg1 = major, cg2 = minor, cg3 = patch, cg4 = prerelease and cg5 = buildmetadata) that is compatible with ECMA Script (JavaScript), PCRE (Perl Compatible Regular Expressions, i.e. Perl, PHP and R), Python and Go.
See: https://regex101.com/r/vkijKf/1/

^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed explanation. I took an example directly from the specification (item 11-3)

When major, minor, and patch are equal, a pre-release version has lower precedence than a normal version:
Example: 1.0.0-alpha < 1.0.0.

The code provided does not catch that "1.0.0-alpha" is less than "1.0.0"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gammazero, thank you for your review! You're right! And thanks for commenting on the StackOverflow answer related to this!

}

case "$nvers" in
*-*) echo "WARNING: not marking pre-release $dist $nvers as the current version." ;;
*) echo "$nvers" > "dists/$dist/current" ;;
*-*) test_version && echo "WARNING: not marking pre-release $dist $nvers as the current version." ;;
*) test_version && echo "$nvers" > "dists/$dist/current" ;;
esac

echo "$nvers" >> "dists/$dist/versions"
Expand Down