This vercmp issue has been a sticking point but this should resolve many of the issues that have come up. Only a few minor code changes were necessary to get the behavior we desired, and this version appears to beat any other vercmp rendition on a few more cases added in this commit. This commit passes all 58 vercmp tests currently out there. Other 'fixes' still fail on a few tests, namely these ones: test: ver1: 1.5.a ver2: 1.5 ret: -1 expected: 1 ==> FAILURE test: ver1: 1.5 ver2: 1.5.a ret: 1 expected: -1 ==> FAILURE test: ver1: 1.5-1 ver2: 1.5.b ret: 1 expected: -1 ==> FAILURE test: ver1: 1.5.b ver2: 1.5-1 ret: -1 expected: 1 ==> FAILURE 4 of 58 tests failed Signed-off-by: Dan McGee --- Xavier- I had to get this fixed somehow. :) The challenge is back to you. -Dan doc/pacman.8.txt | 2 +- lib/libalpm/package.c | 26 ++++++++++++++------------ pactest/vercmptest.sh | 36 +++++++++++++++++++++++++++++++++--- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 9e56a54..0a632ea 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -67,7 +67,7 @@ You can also use `pacman -Su` to upgrade all packages that are out of date. See to determine which packages need upgrading. This behavior operates as follows: Alphanumeric: - 1.0 < 1.0a < 1.0alpha < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc + 1.0a < 1.0alpha < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc < 1.0 Numeric: 1 < 1.0 < 1.1 < 1.1.1 < 1.2 < 2.0 < 3.0.0 diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 124d89a..13d0ae3 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -691,17 +691,13 @@ int SYMEXPORT alpm_pkg_vercmp(const char *a, const char *b) if(*ptr1 == '-' && *ptr2 == '-') { /* no-op, continue comparing since we are equivalent throughout */ } else if(*ptr1 == '-') { - /* ptr1 has hit the pkgrel and ptr2 has not. - * version 2 is newer iff we are not at the end of ptr2; - * if we are at end then one version had pkgrel and one did not */ - ret = *ptr2 ? -1 : 0; - goto cleanup; + /* ptr1 has hit the pkgrel and ptr2 has not. continue version + * comparison after stripping the pkgrel from ptr1. */ + *ptr1 = '\0'; } else if(*ptr2 == '-') { - /* ptr2 has hit the pkgrel and ptr1 has not. - * version 1 is newer iff we are not at the end of ptr1; - * if we are at end then one version had pkgrel and one did not */ - ret = *ptr1 ? 1 : 0; - goto cleanup; + /* ptr2 has hit the pkgrel and ptr1 has not. continue version + * comparison after stripping the pkgrel from ptr2. */ + *ptr2 = '\0'; } } @@ -713,8 +709,14 @@ int SYMEXPORT alpm_pkg_vercmp(const char *a, const char *b) goto cleanup; } - /* whichever version still has characters left over wins */ - if (!*one) { + /* the final showdown. we never want a remaining alpha string to + * beat an empty string. the logic is a bit weird, but: + * - if one is empty and two is not an alpha, two is newer. + * - if one is an alpha, two is newer. + * - otherwise one is newer. + * */ + if ( ( !*one && !isalpha((int)*two) ) + || isalpha((int)*one) ) { ret = -1; } else { ret = 1; diff --git a/pactest/vercmptest.sh b/pactest/vercmptest.sh index f8d457e..e36bf4b 100755 --- a/pactest/vercmptest.sh +++ b/pactest/vercmptest.sh @@ -26,8 +26,9 @@ failure=0 # args: # pass ver1 ver2 ret expected pass() { - echo "test: ver1: $1 ver2: $2 ret: $3 expected: $4" - echo " --> pass" + #echo "test: ver1: $1 ver2: $2 ret: $3 expected: $4" + #echo " --> pass" + echo -n } # args: @@ -54,13 +55,20 @@ runtest() { ret=$($bin $2 $1) func='pass' [ $ret -eq $reverse ] || func='fail' - $func $1 $2 $ret $reverse + $func $2 $1 $ret $reverse total=$(expr $total + 1) } # use first arg as our binary if specified [ -n "$1" ] && bin="$1" +if [ ! -x "$bin" ]; then + echo "vercmp binary ($bin) could not be located" + exit 1 +fi + +echo "Beginning vercmp tests" + # BEGIN TESTS # all similar length, no pkgrel @@ -88,6 +96,28 @@ runtest 1.1-1 1.1 0 runtest 1.0-1 1.1 -1 runtest 1.1-1 1.0 1 +# alphanumeric versions +runtest 1.5b-1 1.5-1 -1 +runtest 1.5b 1.5 -1 +runtest 1.5b-1 1.5 -1 +runtest 1.5b 1.5.1 -1 + +# from the manpage +runtest 1.0a 1.0alpha -1 +runtest 1.0alpha 1.0b -1 +runtest 1.0b 1.0beta -1 +runtest 1.0beta 1.0rc -1 +runtest 1.0rc 1.0 -1 + +# going crazy? alpha-dotted versions +runtest 1.5.a 1.5 1 +runtest 1.5.b 1.5.a 1 +runtest 1.5.1 1.5.b 1 + +# alpha dots and dashes +runtest 1.5.b-1 1.5.b 0 +runtest 1.5-1 1.5.b -1 + #END TESTS echo -- 1.5.6.4