* apt-pkg/deb/debversion.cc:

  - consider absent of debian revision equivalent to 0 (Closes: #573592)

This moves the existing testcase for version comparison to "my" directory,
adds a few more tests (e.g. the tests used in cupt) and rewrites the
testcases runner: The runner does now call dpkg --compare-versions
to check what dpkg thinks about the comparison - all done in less code ;)

apt-pkg/deb/debversion.cc

@@ -190,8 +190,22 @@ int debVersioningSystem::DoCmpVersion(const char *A,const char *AEnd,
       dlhs++;
    if (drhs != rhs)
       drhs++;
-   
-   return CmpFragment(dlhs,AEnd,drhs,BEnd);
+
+   // no debian revision need to be treated like -0
+   if (*(dlhs-1) == '-' && *(drhs-1) == '-')
+      return CmpFragment(dlhs,AEnd,drhs,BEnd);
+   else if (*(dlhs-1) == '-')
+   {
+      const char* null = "0";
+      return CmpFragment(dlhs,AEnd,null, null+1);
+   }
+   else if (*(drhs-1) == '-')
+   {
+      const char* null = "0";
+      return CmpFragment(null, null+1, drhs, BEnd);
+   }
+   else
+      return 0;
 }
 									/*}}}*/
 // debVS::CheckDep - Check a single dependency				/*{{{*/

debian/changelog

@@ -20,6 +20,8 @@ apt (0.7.26) UNRELEASED; urgency=low
     - fix memory leaks in error conditions in DoSource()
   * apt-pkg/contrib/cmndline.cc:
     - save Commandline in Commandline::AsString for logging
+  * apt-pkg/deb/debversion.cc:
+    - consider absent of debian revision equivalent to 0 (Closes: #573592)
 
  -- David Kalnischkies <kalnischkies@gmail.com>  Fri, 19 Feb 2010 21:21:43 +0100
 

test/libapt/compareversion_test.cc

@@ -0,0 +1,123 @@
+// -*- mode: cpp; mode: fold -*-
+// Description								/*{{{*/
+/* ######################################################################
+
+   Version Test - Simple program to run through a file and comare versions.
+   
+   Each version is compared and the result is checked against an expected
+   result in the file. The format of the file is
+       a b Res
+   Where Res is -1, 1, 0. dpkg -D=1 --compare-versions a "<" b can be
+   used to determine what Res should be. # at the start of the line
+   is a comment and blank lines are skipped
+
+   The runner will also call dpkg --compare-versions to check if APT and
+   dpkg have (still) the same idea.
+   
+   ##################################################################### */
+									/*}}}*/
+#include <apt-pkg/macros.h>
+#include <apt-pkg/error.h>
+#include <apt-pkg/version.h>
+#include <apt-pkg/debversion.h>
+#include <apt-pkg/fileutl.h>
+#include <iostream>
+#include <fstream>
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+using namespace std;
+
+bool callDPkg(const char *val, const char *ref, const char &op) {
+   pid_t Process = ExecFork();
+   if (Process == 0)
+   {
+      const char * args[6];
+      args[0] = "/usr/bin/dpkg";
+      args[1] = "--compare-versions";
+      args[2] = val;
+      args[3] = (op == 1) ? ">>" : ( (op == 0) ? "=" : "<<");
+      args[4] = ref;
+      args[5] = 0;
+      execv(args[0], (char**) args);
+      exit(1);
+   }
+   int Ret;
+   waitpid(Process, &Ret, 0);
+   return WIFEXITED(Ret) == true && WEXITSTATUS(Ret) == 0;
+}
+
+void assertVersion(int const &CurLine, string const &A, string const &B, int const &Expected) {
+   int Res = debVS.CmpVersion(A.c_str(), B.c_str());
+   bool const dpkg = callDPkg(A.c_str(),B.c_str(), Expected);
+   Res = (Res < 0) ? -1 : ( (Res > 0) ? 1 : Res);
+
+   if (Res != Expected)
+      _error->Error("Comparison failed on line %u. '%s' '%s' '%s' %i != %i",CurLine,A.c_str(),((Expected == 1) ? "<<" : ( (Expected == 0) ? "=" : ">>")) ,B.c_str(),Res,Expected);
+   if (dpkg == false)
+      _error->Error("DPkg differ with line: %u. '%s' '%s' '%s' == false",CurLine,A.c_str(),((Expected == 1) ? "<<" : ( (Expected == 0) ? "=" : ">>")),B.c_str());
+}
+
+bool RunTest(const char *File)
+{
+   ifstream F(File,ios::in);
+   if (!F != 0)
+      return false;
+
+   char Buffer[300];
+   int CurLine = 0;
+   
+   while (1)
+   {
+      F.getline(Buffer,sizeof(Buffer));
+      CurLine++;
+      if (F.eof() != 0)
+	 return true;
+      if (!F != 0)
+	 return _error->Error("Line %u in %s is too long",CurLine,File);
+
+      // Comment
+      if (Buffer[0] == '#' || Buffer[0] == 0)
+	 continue;
+      
+      // First version
+      char *I;
+      char *Start = Buffer;
+      for (I = Buffer; *I != 0 && *I != ' '; I++);
+      string A(Start, I - Start);
+
+      if (*I == 0)
+	 return _error->Error("Invalid line %u",CurLine);
+      
+      // Second version
+      I++;
+      Start = I;
+      for (I = Start; *I != 0 && *I != ' '; I++);
+      string B(Start,I - Start);
+      
+      if (*I == 0 || I[1] == 0)
+	 return _error->Error("Invalid line %u",CurLine);
+      
+      // Result
+      I++;
+      int const Expected = atoi(I);
+      assertVersion(CurLine, A, B, Expected);
+      // Check the reverse as well
+      assertVersion(CurLine, B, A, Expected*-1);
+   }
+}
+
+int main(int argc, char *argv[])
+{
+   if (argc <= 1)
+      RunTest("../versions.lst");
+   else
+      RunTest(argv[1]);
+
+   // Print any errors or warnings found
+   _error->DumpErrors();
+   return 0;
+}

test/libapt/makefile

@@ -29,3 +29,9 @@ PROGRAM = commandlineasstring${BASENAME}
 SLIBS = -lapt-pkg
 SOURCE = commandlineasstring_test.cc
 include $(PROGRAM_H)
+
+# Program for testing debians version comparing
+PROGRAM = compareversion${BASENAME}
+SLIBS = -lapt-pkg
+SOURCE = compareversion_test.cc
+include $(PROGRAM_H)

test/makefile

@@ -24,13 +24,6 @@ LIB_MAKES = apt-pkg/makefile apt-inst/makefile
 SOURCE = scratch.cc
 include $(PROGRAM_H)
 
-# Version compare tester
-PROGRAM=versiontest
-SLIBS = -lapt-pkg 
-LIB_MAKES = apt-pkg/makefile
-SOURCE = versiontest.cc
-include $(PROGRAM_H)
-
 # Version compare tester
 PROGRAM=testextract
 SLIBS = -lapt-pkg -lapt-inst

test/versions.lst

@@ -10,10 +10,11 @@
 1.3 1.2.2 1
 
 # Important attributes
-- . -1
-p - -1
-a - -1
-z - -1
+# disabled as dpkg --compare-versions doesn't like them…
+#- . -1
+#p - -1
+#a - -1
+#z - -1
 a . -1
 z . -1
 
@@ -22,6 +23,10 @@ z . -1
 1:1.25-4 1:1.25-8 -1
 0:1.18.36 1.18.36 0
 
+# native version
+1.18.36 1.18.35 1
+0:1.18.36 1.18.35 1
+
 # Funky, but allowed, characters in upstream version
 9:1.18.36:5.4-20 10:0.5.1-22 -1
 9:1.18.36:5.4-20 9:1.18.36:5.5-1 -1
@@ -49,3 +54,54 @@ III-alpha9.8 III-alpha9.8-1.5 -1
 
 # #205960
 3.0~rc1-1 3.0-1 -1
+
+# #573592 - debian policy 5.6.12
+1.0 1.0-0 0
+0.2 1.0-0 -1
+1.0 1.0-0+b1 -1
+1.0 1.0-0~ 1
+
+# if a version includes a dash
+# it should be the debrev dash - policy says so…
+0:0-0-0 0-0 1
+
+# do we like strange versions? Yes we like strange versions…
+0 0 0
+0 00 0
+
+# "steal" the testcases from cupt
+1.2.3 1.2.3 0 # identical
+4.4.3-2 4.4.3-2 0 # identical
+1:2ab:5 1:2ab:5 0 # this is correct...
+7:1-a:b-5 7:1-a:b-5 0 # and this
+57:1.2.3abYZ+~-4-5 57:1.2.3abYZ+~-4-5 0 # and those too
+1.2.3 0:1.2.3 0 # zero epoch
+1.2.3 1.2.3-0 0 # zero revision
+009 9 0 # zeroes...
+009ab5 9ab5 0 # there as well
+1.2.3 1.2.3-1 -1 # added non-zero revision
+1.2.3 1.2.4 -1 # just bigger
+1.2.4 1.2.3 1 # order doesn't matter
+1.2.24 1.2.3 1 # bigger, eh?
+0.10.0 0.8.7 1 # bigger, eh?
+3.2 2.3 1 # major number rocks
+1.3.2a 1.3.2 1 # letters rock
+0.5.0~git 0.5.0~git2 -1 # numbers rock
+2a 21 -1 # but not in all places
+1.3.2a 1.3.2b -1 # but there is another letter
+1:1.2.3 1.2.4 1 # epoch rocks
+1:1.2.3 1:1.2.4 -1 # bigger anyway
+1.2a+~bCd3 1.2a++ -1 # tilde doesn't rock
+1.2a+~bCd3 1.2a+~ 1 # but first is longer!
+5:2 304-2 1 # epoch rocks
+5:2 304:2 -1 # so big epoch?
+25:2 3:2 1 # 25 > 3, obviously
+1:2:123 1:12:3 -1 # 12 > 2
+1.2-5 1.2-3-5 -1 # 1.2 < 1.2-3
+5.10.0 5.005 1 # preceding zeroes don't matters
+3a9.8 3.10.2 -1 # letters are before all letter symbols
+3a9.8 3~10 1 # but after the tilde
+1.4+OOo3.0.0~ 1.4+OOo3.0.0-4 -1 # another tilde check
+2.4.7-1 2.4.7-z -1 # revision comparing
+1.002-1+b2 1.00 1 # whatever...
+2.2.4-47978_Debian_lenny 2.2.4-47978_Debian_lenny 0 # and underscore...

test/versiontest.cc

@@ -1,233 +0,0 @@
-// -*- mode: cpp; mode: fold -*-
-// Description								/*{{{*/
-// $Id: versiontest.cc,v 1.5 2003/08/18 15:55:19 mdz Exp $
-/* ######################################################################
-
-   Version Test - Simple program to run through a file and comare versions.
-   
-   Each version is compared and the result is checked against an expected
-   result in the file. The format of the file is
-       a b Res
-   Where Res is -1, 1, 0. dpkg -D=1 --compare-versions a "<" b can be
-   used to determine what Res should be. # at the start of the line
-   is a comment and blank lines are skipped
-   
-   ##################################################################### */
-									/*}}}*/
-#include <apt-pkg/macros.h>
-#include <apt-pkg/error.h>
-#include <apt-pkg/version.h>
-#include <apt-pkg/debversion.h>
-#include <iostream>
-#include <fstream>
-
-using namespace std;
-
-  static int verrevcmp(const char *val, const char *ref) 
-{
-   int vc, rc;
-   long vl, rl;
-   const char *vp, *rp;
-   
-   if (!val) 
-      val = "";
-   if (!ref) 
-      ref = "";
-   for (;;) 
-   {
-      vp = val;  
-      while (*vp && !isdigit(*vp)) 
-	 vp++;
-      rp = ref;  
-      while (*rp && !isdigit(*rp)) 
-	 rp++;
-      for (;;) 
-      {
-	 vc= val == vp ? 0 : *val++;
-	 rc= ref == rp ? 0 : *ref++;
-	 if (!rc && !vc)
-	    break;
-	 if (vc && !isalpha(vc)) 
-	    vc += 256; /* assumes ASCII character set */
-	 if (rc && !isalpha(rc)) 
-	    rc += 256;
-	 if (vc != rc) 
-	    return vc - rc;
-      }
-      val = vp;
-      ref = rp;
-      vl = 0;
-      if (isdigit(*vp)) 
-	 vl = strtol(val,(char**)&val,10);
-      rl = 0;
-      if (isdigit(*rp)) 
-	 rl = strtol(ref,(char**)&ref,10);
-      if (vl != rl) 
-	 return vl - rl;
-      if (!*val && !*ref) 
-	 return 0;
-      if (!*val) 
-	 return -1;
-      if (!*ref) 
-	 return +1;
-   }
-}
-
-#if 0
-static int verrevcmp(const char *val, const char *ref) 
-{   
-   int vc, rc;
-   long vl, rl;
-   const char *vp, *rp;
-   
-   if (!val) val= "";
-   if (!ref) ref= "";
-   for (;;) 
-   {
-      vp= val;  while (*vp && !isdigit(*vp) && *vp != '~') vp++;
-      rp= ref;  while (*rp && !isdigit(*rp) && *rp != '~') rp++;
-      for (;;)
-      {	 
-	 vc= val == vp ? 0 : *val++;
-	 rc= ref == rp ? 0 : *ref++;
-	 if (!rc && !vc) break;
-	 if (vc && !isalpha(vc)) vc += 256; /* assumes ASCII character set */
-	 if (rc && !isalpha(rc)) rc += 256;
-	 if (vc != rc) return vc - rc;
-      }
-      
-      val= vp;
-      ref= rp;
-      if (*vp == '~') val++;
-      if (*rp == '~') ref++;
-      vl=0;  if (isdigit(*val)) vl= strtol(val,(char**)&val,10);
-      rl=0;  if (isdigit(*ref)) rl= strtol(ref,(char**)&ref,10);
-      if (vl == 0 && rl == 0)
-      {
-	 if (*vp == '~' && *rp != '~') return -1;
-	 if (*vp != '~' && *rp == '~') return +1;
-      }
-      if (*vp == '~')
-	 vl *= -1;
-      if (*rp == '~')
-	 rl *= -1;
-      if (vl != rl) return vl - rl;
-      if (!*val && !*ref) return 0;
-      if (!*val)
-      {
-	 if (*ref == '~')
-	    return +1;
-         else
-	    return -1;
-      }
-      
-      if (!*ref)
-      {
-	 if (*val == '~')
-	    return -1;
-	 else
-	    return +1;
-      }      
-   }   
-}
-#endif
-    
-bool RunTest(const char *File)
-{
-   ifstream F(File,ios::in);
-   if (!F != 0)
-      return false;
-
-   char Buffer[300];
-   int CurLine = 0;
-   
-   while (1)
-   {
-      F.getline(Buffer,sizeof(Buffer));
-      CurLine++;
-      if (F.eof() != 0)
-	 return true;
-      if (!F != 0)
-	 return _error->Error("Line %u in %s is too long",CurLine,File);
-
-      // Comment
-      if (Buffer[0] == '#' || Buffer[0] == 0)
-	 continue;
-      
-      // First version
-      char *I;
-      char *Start = Buffer;
-      for (I = Buffer; *I != 0 && *I != ' '; I++);
-      string A(Start, I - Start);
-
-      if (*I == 0)
-	 return _error->Error("Invalid line %u",CurLine);
-      
-      // Second version
-      I++;
-      Start = I;
-      for (I = Start; *I != 0 && *I != ' '; I++);
-      string B(Start,I - Start);
-      
-      if (*I == 0 || I[1] == 0)
-	 return _error->Error("Invalid line %u",CurLine);
-      
-      // Result
-      I++;
-      int Expected = atoi(I);
-      int Res = debVS.CmpVersion(A.c_str(), B.c_str());
-      int Res2 = verrevcmp(A.c_str(),B.c_str());
-      cout << "'" << A << "' ? '" << B << "' = " << Res << " (= " << Expected << ") " << Res2 << endl;
-
-      if (Res < 0)
-           Res = -1;
-      else if (Res > 0)
-           Res = 1;
-
-      if (Res != Expected)
-	 _error->Error("Comparison failed on line %u. '%s' ? '%s' %i != %i",CurLine,A.c_str(),B.c_str(),Res,Expected);
-
-      // Check the reverse as well
-      Expected = -1*Expected;
-      Res = debVS.CmpVersion(B.c_str(), A.c_str());
-      Res2 = verrevcmp(B.c_str(),A.c_str());
-
-      cout << "'" << B << "' ? '" << A << "' = " << Res << " (= " << Expected << ") " << Res2 << endl;
-
-      if (Res < 0)
-           Res = -1;
-      else if (Res > 0)
-           Res = 1;
-
-      if (Res != Expected)
-	 _error->Error("Comparison failed on line %u. '%s' ? '%s' %i != %i",CurLine,B.c_str(),A.c_str(),Res,Expected);
-   }
-}
-
-int main(int argc, char *argv[])
-{
-   if (argc <= 1)
-   {
-      cerr << "You must specify a test file" << endl;
-      return 0;
-   }
-   
-   RunTest(argv[1]);
-
-   // Print any errors or warnings found
-   if (_error->empty() == false)
-   {
-      string Err;
-      while (_error->empty() == false)
-      {
-	 
-	 bool Type = _error->PopMessage(Err);
-	 if (Type == true)
-	    cout << "E: " << Err << endl;
-	 else
-	    cout << "W: " << Err << endl;
-      }
-      
-      return 0;
-   }
-}