Implement a package number comparison routine for pkg_version(1)

that actually conforms to the Porters Handbook.

Add a -t option to pkg_version(1) for doing comparison testing.

Add a script (and make test target) to do some regression tests on
the package number comparison routine, to help debug future revisions.

Submitted by:		knu
Procrastination by:	bmah

4 files changed, 201 insertions, 31 deletions

diff --git a/usr.sbin/pkg_install/version/Makefile b/usr.sbin/pkg_install/version/Makefile
index 0fb044a..b9c4d1c 100644
--- a/usr.sbin/pkg_install/version/Makefile
+++ b/usr.sbin/pkg_install/version/Makefile
@@ -7,4 +7,7 @@ beforeinstall:
 	${INSTALL} -c -o ${BINOWN} -g ${BINGRP} -m ${BINMODE} \
 	    ${.CURDIR}/pkg_version.pl ${DESTDIR}${BINDIR}/pkg_version
 
+test:
+	./test-pkg_version.sh
+
 .include <bsd.prog.mk>
diff --git a/usr.sbin/pkg_install/version/pkg_version.1 b/usr.sbin/pkg_install/version/pkg_version.1
index e3a787f..5dfb500 100644
--- a/usr.sbin/pkg_install/version/pkg_version.1
+++ b/usr.sbin/pkg_install/version/pkg_version.1
@@ -36,6 +36,8 @@
 .Op Fl l Ar limchar
 .Op Fl L Ar limchar
 .Op Ar index
+.Nm pkg_version
+.Op Fl t Ar version1 version2
 .Sh DESCRIPTION
 The
 .Nm
@@ -144,6 +146,16 @@ Note that because some of the status flag characters are also special
 to the shell, it is best to quote
 .Ar limchar
 with single quotes.
+.It Fl t
+Test a pair of version number strings and exit.
+The output consists of one of the single characters
+.Li =
+(equal),
+.Li \&<
+(right-hand number greater), or
+.Li \&>
+(left-hand number greater) on standard output.
+This flag is mostly useful for scripts or for testing.
 .It Fl v
 Enable verbose output.  Verbose output includes some English-text
 interpretations of the version number comparisons, as well as the
@@ -198,18 +210,20 @@ Blindly running the output of this command may leave a system in an
 unusable state.
 .Pp
 .Dl % pkg_version -c > do_update
+.Pp
+The following command compares two package version strings:
+.Pp
+.Dl % pkg_version -t 1.5 1.5.1
 .Sh AUTHORS
 .An Bruce A. Mah Aq bmah@FreeBSD.org
 .Sh CONTRIBUTORS
 .An Nik Clayton Aq nik@FreeBSD.org ,
 .An Dominic Mitchell Aq dom@palmerharvey.co.uk ,
 .An Mark Ovens Aq marko@FreeBSD.org ,
-.An Doug Barton Aq DougB@gorean.org
+.An Doug Barton Aq DougB@gorean.org ,
+.An Akinori MUSHA Aq knu@FreeBSD.org
 .Sh BUGS
 .Pp
-Patch levels aren't handled
-very well (i.e. version numbers of the form 1.2p3 or 1.2pl3).
-.Pp
 The commands output feature is 
 .Bf Em 
 not
diff --git a/usr.sbin/pkg_install/version/pkg_version.pl b/usr.sbin/pkg_install/version/pkg_version.pl
index 3a57510..8b48090 100755
--- a/usr.sbin/pkg_install/version/pkg_version.pl
+++ b/usr.sbin/pkg_install/version/pkg_version.pl
@@ -37,7 +37,6 @@ use Getopt::Std;
 #
 # Configuration global variables
 #
-$Version = '0.1';
 $CurrentPackagesCommand = '/usr/sbin/pkg_info -aI';
 $CatProgram = "cat ";
 $FetchProgram = "fetch -o - ";
@@ -62,9 +61,7 @@ $PreventFlag = "";
 # This function returns -1, 0, or 1, in the same manner as <=> or cmp.
 #
 sub CompareNumbers {
-    local($v1, $v2);
-    $v1 = $_[0];
-    $v2 = $_[1];
+    my ($v1, $v2) = @_;
 
     # Short-cut in case of equality
     if ($v1 eq $v2) {
@@ -73,27 +70,88 @@ sub CompareNumbers {
 
     # Loop over different components (the parts separated by dots).
     # If any component differs, we have the basis for an inequality.
-    while (1) {
-	($p1, $v1) = split(/\./, $v1, 2);
-	($p2, $v2) = split(/\./, $v2, 2);
-
-	# If we\'re out of components, they\'re equal (this probably won\'t
-	# happen, since the short-cut case above should get this).
-	if (($p1 eq "") && ($p2 eq "")) {
-	    return 0;
-	}
-	# Check for numeric inequality.  We assume here that (for example)
-	# 3.09 < 3.10.
-	elsif ($p1 != $p2) {
-	    return $p1 <=> $p2;
+    my @s1 = split(/\./, $v1);
+    my @s2 = split(/\./, $v2);
+    my ($c1, $c2);
+    do {
+	last unless @s1 || @s2;
+	$c1 = shift @s1;
+	$c2 = shift @s2;
+    } while ($c1 eq $c2);
+
+    # Look at the first components of the arrays that are left.
+    # These will determine the result of the comparison.
+    # Note that if either version doesn't have any components left,
+    # it's implicitly treated as a "0".
+
+    # Our next set of checks looks to see if either component has a
+    # leading letter (there should be at most one leading letter per
+    # component, so that "4.0b1" is allowed, but "4.0beta1" is not).
+    if ($c1 =~ /^\D/) {
+	if ($c2 =~ /^\D/) {
+
+	    # Both have a leading letter, so do an alpha comparison
+	    # on the letters.  This isn't ideal, since we're assuming
+	    # that "1.0.b4" > "1.0.a2".  But it's about the best we can do, 
+	    # without encoding some explicit policy.
+	    my ($letter1, $letter2);
+	    $letter1 = substr($c1, 0, 1);
+	    $letter2 = substr($c2, 0, 1);
+
+	    if ($letter1 ne $letter2) {
+		return $letter1 cmp $letter2;
+	    }
+	    else {
+		# The letters matched equally.  Delete the leading
+		# letters and invoke ourselves on the remainining
+		# characters, which according to the Porters Handbook
+		# must be digits, so for example, "1.0.a9" < "1.0.a10".
+		substr($c1, 0, 1) = "";
+		substr($c2, 0, 1) = "";
+		return &CompareNumbers($c1, $c2);		
+	    }
+
 	}
-	# Check for string inequality, given numeric equality.  This
-	# handles version numbers of the form 3.4j < 3.4k.
-	elsif ($p1 ne $p2) {
-	    return $p1 cmp $p2;
+	else {
+	    # $c1 begins with a letter, but $c2 doesn't.  Let $c2
+	    # win the comparison, so that "1.0.b1" < "1.0.1".
+	    return -1;
 	}
     }
+    else {
+	if ($c2 =~ /^\D/) {
+	    # $c2 begins with a letter but $c1 doesn't.  Let $c1
+	    # win the comparison, as above.
+	    return 1;
+	}
+	else {
+	    # Neither component begins with a leading letter.
+	    # Check for numeric inequality.  We assume here that (for example)
+	    # "3.09" < "3.10", and that we aren't going to be asked to
+	    # decide between "3.010" and "3.10".
+	    if ($c1 != $c2) {
+		return $c1 <=> $c2;
+	    }
 
+	    # String comparison, given numeric equality.  This
+	    # handles comparisons of the form "3.4j" < "3.4k".  This form
+	    # technically isn't allowed by the Porter's Handbook, but a
+	    # number of ports in the FreeBSD Ports Collection as of this
+	    # writing use it (graphics/jpeg and graphics/xv).  So we need
+	    # to support it.
+	    #
+	    # What we actually do is to strip off the leading digits and
+	    # invoke ourselves on the remainder.  This allows us to handle
+	    # comparisons of the form "1.1p1" < "1.1p2".  Again, not
+	    # technically allowed by the Porters Handbook, but lots of ports
+	    # use it.
+	    else {
+		$c1 =~ s/\d+//;
+		$c2 =~ s/\d+//;
+		return &CompareNumbers($c1, $c2);
+	    }
+	}
+    }
 }
 
 #
@@ -194,10 +252,8 @@ sub GetNameAndVersion {
 #
 sub PrintHelp {
     print <<"EOF"
-pkg_version $Version
-Bruce A. Mah <bmah\@freebsd.org>
-
-Usage: pkg_version [-c] [-d debug] [-h] [-v] [index]
+Usage:	pkg_version [-c] [-d debug] [-h] [-l limchar] [-L limchar] [-v] [index]
+	pkg_version [-d debug] -t v1 v2
 -c              Show commands to update installed packages
 -d debug	Debugging output (debug controls level of output)
 -h		Help (this message)
@@ -206,13 +262,15 @@ Usage: pkg_version [-c] [-d debug] [-h] [-v] [index]
 -v		Verbose output
 index		URL or filename of index file
 		(Default is $IndexFile)
+
+-t v1 v2	Test two version strings
 EOF
 }
 
 #
 # Parse command-line arguments, deal with them
 #
-if (!getopts('cdhl:L:v') || ($opt_h)) {
+if (!getopts('cdhl:L:tv') || ($opt_h)) {
     &PrintHelp();
     exit;
 }
@@ -231,11 +289,34 @@ if ($opt_l) {
 if ($opt_L) {
     $PreventFlag = $opt_L;
 }
+if ($opt_t) {
+    $TestFlag = 1;
+}
 if ($opt_v) {
     $VerboseFlag = 1;
 }
 if ($#ARGV >= 0) {
-    $IndexFile = $ARGV[0];
+    if ($TestFlag) {
+	($test1, $test2) = @ARGV;
+    }
+    else {
+	$IndexFile = $ARGV[0];
+    }
+}
+
+# Handle test flag now
+if ($TestFlag) {
+    my $cmp = CompareVersions($test1, $test2);
+    if ($cmp < 0) {
+	print "<\n";
+    }
+    elsif ($cmp == 0) {
+	print "=\n";
+    }
+    else {
+	print ">\n";
+    }
+    exit(0);
 }
 
 # Determine what command to use to retrieve the index file.
diff --git a/usr.sbin/pkg_install/version/test-pkg_version.sh b/usr.sbin/pkg_install/version/test-pkg_version.sh
new file mode 100755
index 0000000..6c14c71
--- /dev/null
+++ b/usr.sbin/pkg_install/version/test-pkg_version.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright 2001 Bruce A. Mah
+#
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+# 1. Redistributions of source code must retain the above copyright
+#    notice, this list of conditions and the following disclaimer.
+# 2. Redistributions in binary form must reproduce the above copyright
+#    notice, this list of conditions and the following disclaimer in the
+#    documentation and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE DEVELOPERS ``AS IS'' AND ANY EXPRESS OR
+# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+# OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+# IN NO EVENT SHALL THE DEVELOPERS BE LIABLE FOR ANY DIRECT, INDIRECT,
+# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+# NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+# test-pkg_version.sh
+#
+# Regression testing for pkg_version
+# Originally from an idea by "Akinori MUSHA" <knu@iDaemons.org>
+#
+# $FreeBSD$
+#
+
+ECHO=echo
+PKG_VERSION=./pkg_version.pl
+
+test-pv ( ) { \
+    setvar v1 $1
+    setvar answer $2
+    setvar v2 $3
+    setvar type $4
+    res=`${PKG_VERSION} -t ${v1} ${v2}`
+    if [ ${res} != ${answer} ]; then \
+	${ECHO} "${type} test failed (${v1} ${res} ${v2}, should have been ${answer})"; \
+    fi
+}
+
+# Test coercion of default PORTREVISION and PORTEPOCH
+test-pv 0.10 "=" 0.10_0 coercion
+test-pv 0.10 "=" 0.10,0 coercion
+test-pv 0.10 "=" 0.10_0,0 coercion
+  
+# Test various comparisons
+test-pv 1.0 "=" 1.0 equality
+test-pv 2.15a "=" 2.15a equality
+
+test-pv 0.10 ">" 0.9 inequality
+test-pv 0.9 "<" 0.10 inequality
+
+test-pv 2.3p10 ">" 2.3p9 number/letter
+test-pv 1.6.0 ">" 1.6.0.p3 number/letter
+test-pv 1.0.b ">" 1.0.a3 number/letter
+test-pv 1.0a "<" 1.0 number/letter
+test-pv 5.0a "<" 5.0.b number/letter
+
+test-pv 1.5_1 ">" 1.5 portrevision
+test-pv 1.5_2 ">" 1.5_1 portrevision
+
+test-pv 00.01.01,1 ">" 99.12.31 portepoch
+test-pv 0.0.1_1,2 ">" 0.0.1,2 portrevision/portepoch
+test-pv 0.0.1_1,3 ">" 0.0.1_2,2 portrevision/portepoch