[pacman-dev] [PATCH] Add simple vercmp test script
Commit 84283672853350a84d2a71b72dc06e180cad1587 updated the versioncmp code in libalpm. Unfortunately for us, it also introduced the regression that becomes apparant with the following upgrade: warning: sonata: local (1.5-2) is newer than extra (1.5.1-2) Add a vercmptest.sh test script that is run during the make check phase which now points out three regressions in the version comparison function that will need fixing. All current tests in this script pass with the old versioncmp code. Signed-off-by: Dan McGee <dan@archlinux.org> --- Makefile.am | 6 ++- pactest/vercmptest.sh | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100755 pactest/vercmptest.sh diff --git a/Makefile.am b/Makefile.am index dc08542..ea3b0e0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -11,12 +11,14 @@ EXTRA_DIST = HACKING pkgdatadir = ${datadir}/${PACKAGE} dist_pkgdata_DATA = PKGBUILD.proto proto.install ChangeLog.proto -# run the pactest test suite -check-local: src/pacman +# run the pactest test suite and vercmp tests +check-local: pactest src/pacman src/util $(PYTHON) $(top_srcdir)/pactest/pactest.py --debug=1 \ --test $(top_srcdir)/pactest/tests/*.py \ -p $(top_builddir)/src/pacman/pacman rm -rf $(top_builddir)/root + $(SH) $(top_srcdir)/pactest/vercmptest.sh \ + $(top_builddir)/src/util/vercmp # create the pacman DB and cache directories upon install install-data-local: diff --git a/pactest/vercmptest.sh b/pactest/vercmptest.sh new file mode 100755 index 0000000..62a3e32 --- /dev/null +++ b/pactest/vercmptest.sh @@ -0,0 +1,77 @@ +#!/bin/sh +# +# vercmptest - a test suite for the vercmp/libalpm program +# +# Copyright (c) 2008 by Dan McGee <dan@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# default binary if one was not specified as $1 +bin='vercmp' +# holds count of failed tests +failure=0 + +# args: +# fail ver1 ver2 ret expected +fail() { + echo "unexpected failure:" + echo " ver1: $1 ver2: $2 ret: $3 expected: $4" + failure=$(expr $failure + 1) +} + +# args: +# runtest ver1 ver2 expected +runtest() { + ret=$($bin $1 $2) + [ $ret -eq $3 ] || fail $1 $2 $ret $3 +} + +# use first arg as our binary if specified +[ -n "$1" ] && bin="$1" + +# BEGIN TESTS + +# all similar length, no pkgrel +runtest 1.5.0 1.5.0 0 +runtest 1.5.0 1.5.1 -1 +runtest 1.5.1 1.5.0 1 + +# mixed length +runtest 1.5 1.5.1 -1 +runtest 1.5.1 1.5 1 + +# with pkgrel, simple +runtest 1.5.0-1 1.5.0-1 0 +runtest 1.5.0-1 1.5.0-2 -1 +runtest 1.5.0-1 1.5.1-1 -1 +runtest 1.5.0-2 1.5.1-1 -1 + +# with pkgrel, mixed lengths +runtest 1.5-1 1.5.1-1 -1 +runtest 1.5-2 1.5.1-1 -1 +runtest 1.5-2 1.5.1-2 -1 + +# mixed pkgrel inclusion +runtest 1.5 1.5-1 0 +runtest 1.5-1 1.5 0 + +#END TESTS + +if [ $failure -eq 0 ]; then + echo "All tests successful" + exit 0 +fi + +echo "$failure failed tests" +exit 1 -- 1.5.5.3
Commit 84283672853350a84d2a71b72dc06e180cad1587 introduced the regression, and a previous commit introduced the vercmptest.sh test script to track down these issues. This commit solves the problem by removing the previous attempt at locating the pkgrel portions and replacing it with something that performs the correct logic. While tracking down everything I needed to, I also found a mistake in one of the pactests which is fixed here as well as increased the functionality and verbosity of the vercmptest script to both print out each test it is running as well as automatically run the mirror of each test case. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/package.c | 36 +++++++++++++++++++++++++++--------- pactest/tests/upgrade058.py | 2 +- pactest/vercmptest.sh | 39 +++++++++++++++++++++++++++++++-------- 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 07b5fa3..685a411 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -566,8 +566,6 @@ alpm_list_t SYMEXPORT *alpm_pkg_compute_requiredby(pmpkg_t *pkg) return(reqs); } -/** @} */ - /** Compare two version strings and determine which one is 'newer'. * Returns a value comparable to the way strcmp works. Returns 1 * if a is newer than b, 0 if a and b are the same version, or -1 @@ -577,6 +575,12 @@ alpm_list_t SYMEXPORT *alpm_pkg_compute_requiredby(pmpkg_t *pkg) * at lib/rpmvercmp.c, and was most recently updated against rpm * version 4.4.2.3. Small modifications have been made to make it more * consistent with the libalpm coding style. + * + * Keep in mind that the pkgrel is only compared if it is available + * on both versions handed to this function. For example, comparing + * 1.5-1 and 1.5 will yield 0; comparing 1.5-1 and 1.5-2 will yield + * -1 as expected. This is mainly for supporting versioned dependencies + * that do not include the pkgrel. */ int SYMEXPORT alpm_pkg_vercmp(const char *a, const char *b) { @@ -688,6 +692,26 @@ int SYMEXPORT alpm_pkg_vercmp(const char *a, const char *b) one = ptr1; *ptr2 = oldch2; two = ptr2; + + /* libalpm added code. check if version strings have hit the pkgrel + * portion. depending on which strings have hit, take correct action. + * this is all based on the premise that we only have one dash in + * the version string, and it separates pkgver from pkgrel. */ + 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; + } 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; + } } /* this catches the case where all numeric and alpha segments have */ @@ -698,13 +722,6 @@ int SYMEXPORT alpm_pkg_vercmp(const char *a, const char *b) goto cleanup; } - /* libalpm added code. one version string may have a pkgrel number, the - * other may not. unless both have them, we ignore it and return 0. */ - if( (*one && *one == '-') || (*two && *two == '-') ) { - ret = 0; - goto cleanup; - } - /* whichever version still has characters left over wins */ if (!*one) { ret = -1; @@ -718,6 +735,7 @@ cleanup: return(ret); } +/** @} */ pmpkg_t *_alpm_pkg_new(void) { diff --git a/pactest/tests/upgrade058.py b/pactest/tests/upgrade058.py index 7641680..109289e 100644 --- a/pactest/tests/upgrade058.py +++ b/pactest/tests/upgrade058.py @@ -14,4 +14,4 @@ self.addpkg(p) self.args = "-U %s" % p.filename() self.addrule("PACMAN_RETCODE=1") -self.addrule("!PKG_VERSION=pkg2|1.1") +self.addrule("!PKG_VERSION=pkg2|1.1-1") diff --git a/pactest/vercmptest.sh b/pactest/vercmptest.sh index 62a3e32..f8d457e 100755 --- a/pactest/vercmptest.sh +++ b/pactest/vercmptest.sh @@ -19,22 +19,43 @@ # default binary if one was not specified as $1 bin='vercmp' -# holds count of failed tests +# holds counts of tests +total=0 failure=0 # args: +# pass ver1 ver2 ret expected +pass() { + echo "test: ver1: $1 ver2: $2 ret: $3 expected: $4" + echo " --> pass" +} + +# args: # fail ver1 ver2 ret expected fail() { - echo "unexpected failure:" - echo " ver1: $1 ver2: $2 ret: $3 expected: $4" + echo "test: ver1: $1 ver2: $2 ret: $3 expected: $4" + echo " ==> FAILURE" failure=$(expr $failure + 1) } # args: # runtest ver1 ver2 expected runtest() { + # run the test ret=$($bin $1 $2) - [ $ret -eq $3 ] || fail $1 $2 $ret $3 + func='pass' + [ $ret -eq $3 ] || func='fail' + $func $1 $2 $ret $3 + total=$(expr $total + 1) + # and run its mirror case just to be sure + reverse=0 + [ $3 -eq 1 ] && reverse=-1 + [ $3 -eq -1 ] && reverse=1 + ret=$($bin $2 $1) + func='pass' + [ $ret -eq $reverse ] || func='fail' + $func $1 $2 $ret $reverse + total=$(expr $total + 1) } # use first arg as our binary if specified @@ -44,11 +65,9 @@ runtest() { # all similar length, no pkgrel runtest 1.5.0 1.5.0 0 -runtest 1.5.0 1.5.1 -1 runtest 1.5.1 1.5.0 1 # mixed length -runtest 1.5 1.5.1 -1 runtest 1.5.1 1.5 1 # with pkgrel, simple @@ -65,13 +84,17 @@ runtest 1.5-2 1.5.1-2 -1 # mixed pkgrel inclusion runtest 1.5 1.5-1 0 runtest 1.5-1 1.5 0 +runtest 1.1-1 1.1 0 +runtest 1.0-1 1.1 -1 +runtest 1.1-1 1.0 1 #END TESTS +echo if [ $failure -eq 0 ]; then - echo "All tests successful" + echo "All $total tests successful" exit 0 fi -echo "$failure failed tests" +echo "$failure of $total tests failed" exit 1 -- 1.5.5.3
participants (1)
-
Dan McGee