- complete rewrite of the version number parsing code, restoring compatibiliy of 5.x with 4.x and portupgrade

- parse version numbers of ports containing an underscore followed by a number correctly

- handle special strings pl, alpha, beta, pre and rc

PR:		56961

4 files changed, 265 insertions, 87 deletions

diff --git a/usr.sbin/pkg_install/lib/lib.h b/usr.sbin/pkg_install/lib/lib.h
index afa68af..5690314 100644
--- a/usr.sbin/pkg_install/lib/lib.h
+++ b/usr.sbin/pkg_install/lib/lib.h
@@ -214,7 +214,6 @@ int		requiredby(const char *, struct reqr_by_head **, Boolean, Boolean);
 
 /* Version */
 int		verscmp(Package *, int, int);
-const char	*version_of(const char *, int *, int *);
 int		version_cmp(const char *, const char *);
 
 /* Externs */
diff --git a/usr.sbin/pkg_install/lib/version.c b/usr.sbin/pkg_install/lib/version.c
index cdc3039..d9c4fe7 100644
--- a/usr.sbin/pkg_install/lib/version.c
+++ b/usr.sbin/pkg_install/lib/version.c
@@ -48,35 +48,205 @@ verscmp(Package *pkg, int major, int minor)
 }
 
 /*
- * version_of(pkgname, epoch, revision) returns a pointer to the version
- * portion of a package name and the two special components.
+ * split_version(pkgname, endname, epoch, revision) returns a pointer to
+ * the version portion of a package name and the two special components.
  *
- * Jeremy D. Lea.
+ * Syntax is:  ${PORTNAME}-${PORTVERSION}[_${PORTREVISION}][,${PORTEPOCH}]
+ *
+ * Written by Oliver Eikemeier
+ * Based on work of Jeremy D. Lea.
  */
-const char *
-version_of(const char *pkgname, int *epoch, int *revision)
+static const char *
+split_version(const char *pkgname, const char **endname, unsigned long *epoch, unsigned long *revision)
 {
     char *ch;
+    const char *versionstr;
+    const char *endversionstr;
 
     if (pkgname == NULL)
 	errx(2, "%s: Passed NULL pkgname.", __func__);
+
+    /* Look for the last '-' the the pkgname */
+    ch = strrchr(pkgname, '-');
+    /* Cheat if we are just passed a version, not a valid package name */
+    versionstr = ch ? ch + 1 : pkgname;
+
+    /* Look for the last '_' in the version string, advancing the end pointer */
+    ch = strrchr(versionstr, '_');
+    if (revision != NULL) {
+	*revision = ch ? strtoul(ch + 1, NULL, 10) : 0;
+    }
+    endversionstr = ch;
+
+    /* Look for the last ',' in the remaining version string */
+    ch = strrchr(endversionstr ? endversionstr + 1 : versionstr, ',');
     if (epoch != NULL) {
-	if ((ch = strrchr(pkgname, ',')) == NULL)
-	    *epoch = 0;
-	else
-	    *epoch = atoi(&ch[1]);
+	*epoch = ch ? strtoul(ch + 1, NULL, 10) : 0;
     }
-    if (revision != NULL) {
-	if ((ch = strrchr(pkgname, '_')) == NULL)
-	    *revision = 0;
-	else
-	    *revision = atoi(&ch[1]);
+    if (ch && !endversionstr)
+	endversionstr = ch;
+
+    /* set the pointer behind the last character of the version without revision or epoch */
+    if (endname)
+	*endname = endversionstr ? endversionstr : strrchr(versionstr, '\0');
+
+    return versionstr;
+}
+
+/*
+ * PORTVERSIONs are composed of components separated by dots. A component
+ * consists of a version number, a letter and a patchlevel number. This does
+ * not conform to the porter's handbook, but let us formulate rules that
+ * fit the current practice and are far simpler than to make decisions
+ * based on the order of netters and lumbers. Besides, people use versions
+ * like 10b2 in the ports...
+ */
+
+typedef struct {
+#ifdef __LONG_LONG_SUPPORTED
+    long long n;
+    long long pl;
+#else
+    long n;
+    long pl;
+#endif
+    int a;
+} version_component;
+
+/*
+ * get_component(position, component) gets the value of the next component
+ * (number - letter - number triple) and returns a pointer to the next character
+ * after any leading separators
+ *
+ * - components are separated by dots
+ * - characters !~ [a-zA-Z0-9.+*] are treated as separators
+ *   (1.0:2003.09.16 = 1.0.2003.09.16), this may not be what you expect:
+ *   1.0.1:2003.09.16 < 1.0:2003.09.16
+ * - consecutive separators are collapsed (10..1 = 10.1)
+ * - missing separators are inserted, essentially
+ *   letter number letter => letter number . letter (10a1b2 = 10a1.b2)
+ * - missing components are assumed to be equal to 0 (10 = 10.0 = 10.0.0)
+ * - the letter sort order is: [none], a, b, ..., z; numbers without letters
+ *   sort first (10 < 10a < 10b)
+ * - missing version numbers (in components starting with a letter) sort as -1
+ *   (a < 0, 10.a < 10)
+ * - a separator is inserted before the special strings "pl", "alpha", "beta", 
+ *   "pre" and "rc".
+ * - "pl" sorts before every other letter, "alpha", "beta", "pre" and "rc"
+ *   sort as a, b, p and r. (10alpha = 10.a < 10, but 10 < 10a; pl11 < alpha3
+ *   < 0.1beta2 = 0.1.b2 < 0.1)
+ * - other strings use only the first letter for sorting, case is ignored
+ *   (1.d2 = 1.dev2 = 1.Development2)
+ * - The special component `*' is guaranteed to be the smallest possible
+ *   component (2.* < 2pl1 < 2alpha3 < 2.9f7 < 3.*)
+ * - components separated by `+' are handled by version_cmp below
+ *
+ * Oliver Eikemeier
+ */
+
+static const struct {
+    const char *name;
+    size_t namelen;
+    int value;
+} stage[] = {
+    { "pl",    2,  0        },
+    { "alpha", 5, 'a'-'a'+1 },
+    { "beta",  4, 'b'-'a'+1 },
+    { "pre",   3, 'p'-'a'+1 },
+    { "rc",    2, 'r'-'a'+1 },
+    { NULL,    0,  -1       }
+};
+
+static const char *
+get_component(const char *position, version_component *component)
+{
+    const char *pos = position;
+    int hasstage = 0, haspatchlevel = 0;
+
+    if (!pos)
+	errx(2, "%s: Passed NULL position.", __func__);
+
+    /* handle version number */
+    if (isdigit(*pos)) {
+	char *endptr;
+#ifdef __LONG_LONG_SUPPORTED
+	component->n = strtoll(pos, &endptr, 10);
+#else
+	component->n = strtol(pos, &endptr, 10);
+#endif
+	/* should we test for errno == ERANGE? */
+	pos = endptr;
+    } else if (*pos == '*') {
+	component->n = -2;
+	do {
+	    pos++;
+	} while(*pos && *pos != '+');
+    } else {
+	component->n = -1;
+	hasstage = 1;
     }
-    /* Cheat if we are just passed a version, not a valid package name */
-    if ((ch = strrchr(pkgname, '-')) == NULL)
-	return pkgname;
-    else
-	return &ch[1];
+
+    /* handle letter */
+    if (isalpha(*pos)) {
+	int c = tolower(*pos);
+	haspatchlevel = 1;
+	/* handle special suffixes */
+	if (isalpha(pos[1])) {
+	    int i;
+	    for (i = 0; stage[i].name; i++) {
+		if (strncasecmp(pos, stage[i].name, stage[i].namelen) == 0
+                    && !isalpha(pos[stage[i].namelen])) {
+		    if (hasstage) {
+			/* stage to value */
+			component->a = stage[i].value;
+			pos += stage[i].namelen;
+		    } else {
+			/* insert dot */
+			component->a = 0;
+			haspatchlevel = 0;
+		    }
+		    c = 0;
+		    break;
+		}
+	    }
+	}
+	/* unhandled above */
+	if (c) {
+	    /* use the first letter and skip following */
+	    component->a = c - 'a' + 1;
+	    do {
+		++pos;
+	    } while (isalpha(*pos));
+	}
+    } else {
+	component->a = 0;
+	haspatchlevel = 0;
+    }
+
+    if (haspatchlevel) {
+	/* handle patch number */
+	if (isdigit(*pos)) {
+	    char *endptr;
+#ifdef __LONG_LONG_SUPPORTED
+	    component->pl = strtoll(pos, &endptr, 10);
+#else
+	    component->pl = strtol(pos, &endptr, 10);
+#endif
+	    /* should we test for errno == ERANGE? */
+	    pos = endptr;
+	} else {
+	    component->pl = -1;
+	}
+    } else {
+	component->pl = 0;
+    }
+
+    /* skip trailing separators */
+    while (*pos && !isdigit(*pos) && !isalpha(*pos) && *pos != '+' && *pos != '*') {
+	pos++;
+    }
+
+    return pos;
 }
 
 /*
@@ -94,78 +264,65 @@ version_of(const char *pkgname, int *epoch, int *revision)
  * of the version should conform to the porting guidelines. It can contain
  * multiple components, separated by a period, including letters.
  *
- * The tests below allow for significantly more latitude in the version
- * numbers than is allowed in the guidelines. No point in wasting user's
- * time enforcing them here. That's what flamewars are for.
+ * The tests allow for significantly more latitude in the version numbers
+ * than is allowed in the guidelines. No point in enforcing them here.
+ * That's what portlint is for.
  *
  * Jeremy D. Lea.
+ * reimplemented by Oliver Eikemeier
  */
 int
 version_cmp(const char *pkg1, const char *pkg2)
 {
-    const char *c1, *c2, *v1, *v2;
-    char *t1, *t2;
-    int e1, e2, r1, r2, n1, n2;
-
-    v1 = version_of(pkg1, &e1, &r1);
-    v2 = version_of(pkg2, &e2, &r2);
-    /* Minor optimisation. */
-    if (strcmp(v1, v2) == 0)
-	return 0;
-    /* First compare epoch. */
-    if (e1 != e2)
-	return (e1 < e2 ? -1 : 1);
-    else {
-	/*
-	 * We walk down the versions, trying to convert to numbers.
-	 * We terminate when we reach an underscore, a comma or the
-	 * string terminator, thanks to a nasty trick with strchr().
-	 * strtol() conveniently gobbles up the chars it converts.
-	 */
-	c1 = strchr("_,", v1[0]);
-	c2 = strchr("_,", v2[0]);
-	while (c1 == NULL && c2 == NULL) {
-	    n1 = strtol(v1, &t1, 10);
-	    n2 = strtol(v2, &t2, 10);
-	    if (n1 != n2)
-		return (n1 < n2 ? -1 : 1);
-	    /*
-	     * The numbers are equal, check for letters. Assume they're
-	     * letters purely because strtol() didn't chomp them.
-	     */
-	    c1 = strchr("_,.", t1[0]);
-	    c2 = strchr("_,.", t2[0]);
-	    if (c1 == NULL && c2 == NULL) {
-		/* Both have letters. Compare them. */
-		if (t1[0] != t2[0])
-		    return (t1[0] < t2[0] ? -1 : 1);
-		/* Boring. The letters are equal. Carry on. */
-		v1 = &t1[1], v2 = &t2[1];
-	    } else if (c1 == NULL) {
-		/*
-		 * Letters are strange. After a number, a letter counts
-		 * as greater, but after a period it's less.
-		 */
-		return (isdigit(v1[0]) ? 1 : -1);
-	    } else if (c2 == NULL) {
-		return (isdigit(v2[0]) ? -1 : 1);
+    const char *v1, *v2, *ve1, *ve2;
+    unsigned long e1, e2, r1, r2;
+    int result = 0;
+
+    v1 = split_version(pkg1, &ve1, &e1, &r1);
+    v2 = split_version(pkg2, &ve2, &e2, &r2);
+
+    /* Check epoch, port version, and port revision, in that order. */
+    if (e1 != e2) {
+	result = (e1 < e2 ? -1 : 1);
+    }
+
+    /* Shortcut check for equality before invoking the parsing routines. */
+    if (result == 0 && (ve1 - v1 != ve2 - v2 || strncasecmp(v1, v2, ve1 - v1) != 0)) {
+	/* Loop over different components (the parts separated by dots).
+	 * If any component differs, we have the basis for an inequality. */
+	while(result == 0 && (v1 < ve1 || v2 < ve2)) {
+	    int block_v1 = 0;
+	    int block_v2 = 0;
+	    version_component vc1 = {0, 0, 0};
+	    version_component vc2 = {0, 0, 0};
+	    if (v1 < ve1 && *v1 != '+') {
+		v1 = get_component(v1, &vc1);
 	    } else {
-		/* Neither were letters.  Advance over the period. */
-		v1 = (t1[0] == '.' ? &t1[1] : t1);
-		v2 = (t2[0] == '.' ? &t2[1] : t2);
+		block_v1 = 1;
+	    }
+	    if (v2 < ve2 && *v2 != '+') {
+		v2 = get_component(v2, &vc2);
+	    } else {
+		block_v2 = 1;
+	    }
+	    if (block_v1 && block_v2) {
+		if (v1 < ve1)
+		    v1++;
+		if (v2 < ve2)
+		    v2++;
+	    } else if (vc1.n != vc2.n) {
+		result = (vc1.n < vc2.n ? -1 : 1);
+	    } else if (vc1.a != vc2.a) {
+		result = (vc1.a < vc2.a ? -1 : 1);
+	    } else if (vc1.pl != vc2.pl) {
+		result = (vc1.pl < vc2.pl ? -1 : 1);
 	    }
-	    c1 = strchr("_,", v1[0]);
-	    c2 = strchr("_,", v2[0]);
 	}
-	/* If we got here, check if one version has something left. */
-	if (c1 == NULL)
-	    return (isdigit(v1[0]) ? 1 : -1);
-	if (c2 == NULL)
-	    return (isdigit(v2[0]) ? -1 : 1);
-	/* We've run out of version. Try the revision... */
-	if (r1 != r2)
-	    return (r1 < r2 ? -1 : 1);
-	else
-	    return 0;
     }
+
+    /* Compare FreeBSD revision numbers. */
+    if (result == 0 && r1 != r2) {
+	result = (r1 < r2 ? -1 : 1);
+    }
+    return result;
 }
diff --git a/usr.sbin/pkg_install/version/perform.c b/usr.sbin/pkg_install/version/perform.c
index 6c345b9..8643dd2 100644
--- a/usr.sbin/pkg_install/version/perform.c
+++ b/usr.sbin/pkg_install/version/perform.c
@@ -271,10 +271,12 @@ show_version(const char *installed, const char *latest, const char *source)
 		ch = strchr(tmp, '|');
 		ch[0] = '\0';
 
-		ver = version_of(tmp, NULL, NULL);
+		ver = strrchr(tmp, '-');
+		ver = ver ? &ver[1] : tmp;
 		printf("   multiple versions (index has %s", ver);
 		do {
-		    ver = version_of(&ch[1], NULL, NULL);
+		    ver = strrchr(&ch[1], '-');
+		    ver = ver ? &ver[1] : &ch[1];
 		    if ((ch = strchr(&ch[1], '|')) != NULL)
 			    ch[0] = '\0';
 		    printf(", %s", ver);
@@ -285,7 +287,8 @@ show_version(const char *installed, const char *latest, const char *source)
 	}
     } else {
 	cmp = version_cmp(installed, latest);
-	ver = version_of(latest, NULL, NULL);
+	ver = strrchr(latest, '-');
+	ver = ver ? &ver[1] : latest;
 	if (cmp < 0 && OUTPUT('<')) {
 	    printf("%-34s  <", tmp);
 	    if (Verbose)
diff --git a/usr.sbin/pkg_install/version/test-pkg_version.sh b/usr.sbin/pkg_install/version/test-pkg_version.sh
index 0651047..5c3d98d 100755
--- a/usr.sbin/pkg_install/version/test-pkg_version.sh
+++ b/usr.sbin/pkg_install/version/test-pkg_version.sh
@@ -73,3 +73,22 @@ test-pv 1.5 "<" 1.5.0.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
+
+test-pv 2.0 ">" 2.a2 number/letter
+test-pv 3 "=" 3.0 equality
+test-pv 4a "<" 4a0 letter/zero
+test-pv 10a1b2 "=" 10a1.b2 separator
+
+test-pv 7pl "=" 7.pl patchevel
+test-pv 8.0.a "=" 8.0alpha alpha
+test-pv 9.b3.0 "=" 9beta3 beta
+test-pv 10.pre7 "=" 10pre7.0 pre
+test-pv 11.r "=" 11.rc rc
+
+test-pv 12pl "<" 12alpha alpha/patchevel
+test-pv 13.* "<" 13.pl star/patchevel
+
+test-pv 1.0.0+2003.09.06 "=" 1.0+2003.09.06 plus/multiple
+test-pv 1.0.1+2003.09.06 ">" 1.0+2003.09.06 plus/multiple
+test-pv 1.0.0+2003.09.06 "<" 1.0+2003.09.06_1 plus/portrevision
+test-pv 1.0.1+2003.09.06 ">" 1.0+2003.09.06_1 plus/portrevision