Merge topic 'unlimited-version-compare'

aa86e8ddfd Remove component size limit for version comparisons
7e730d8f7f Tests: Add cases for cmSystemTools::VersionCompare

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: scivision <michael@scivision.dev>
Merge-request: !8231

2 files changed, 75 insertions, 7 deletions

diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index f34e35f..3d61270 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -3197,20 +3197,41 @@ bool VersionCompare(cmSystemTools::CompareOp op, const char* lhss,
 {
   const char* endl = lhss;
   const char* endr = rhss;
-  unsigned long lhs;
-  unsigned long rhs;
 
   while (((*endl >= '0') && (*endl <= '9')) ||
          ((*endr >= '0') && (*endr <= '9'))) {
-    // Do component-wise comparison.
-    lhs = strtoul(endl, const_cast<char**>(&endl), 10);
-    rhs = strtoul(endr, const_cast<char**>(&endr), 10);
+    // Do component-wise comparison, ignoring leading zeros
+    // (components are treated as integers, not as mantissas)
+    while (*endl == '0') {
+      endl++;
+    }
+    while (*endr == '0') {
+      endr++;
+    }
+
+    const char* beginl = endl;
+    const char* beginr = endr;
+
+    // count significant digits
+    while ((*endl >= '0') && (*endl <= '9')) {
+      endl++;
+    }
+    while ((*endr >= '0') && (*endr <= '9')) {
+      endr++;
+    }
+
+    // compare number of digits first
+    ptrdiff_t r = ((endl - beginl) - (endr - beginr));
+    if (r == 0) {
+      // compare the digits if number of digits is equal
+      r = strncmp(beginl, beginr, endl - beginl);
+    }
 
-    if (lhs < rhs) {
+    if (r < 0) {
       // lhs < rhs, so true if operation is LESS
       return (op & cmSystemTools::OP_LESS) != 0;
     }
-    if (lhs > rhs) {
+    if (r > 0) {
       // lhs > rhs, so true if operation is GREATER
       return (op & cmSystemTools::OP_GREATER) != 0;
     }
diff --git a/Tests/CMakeLib/testSystemTools.cxx b/Tests/CMakeLib/testSystemTools.cxx
index 754205e..0f0c025 100644
--- a/Tests/CMakeLib/testSystemTools.cxx
+++ b/Tests/CMakeLib/testSystemTools.cxx
@@ -36,6 +36,53 @@ int testSystemTools(int /*unused*/, char* /*unused*/[])
            "cmSystemTools::UpperCase");
 
   // ----------------------------------------------------------------------
+  // Test cmSystemTools::VersionCompare
+  cmAssert(cmSystemTools::VersionCompareEqual("", ""),
+           "VersionCompareEqual empty string");
+  cmAssert(!cmSystemTools::VersionCompareGreater("", ""),
+           "VersionCompareGreater empty string");
+  cmAssert(cmSystemTools::VersionCompareEqual("1", "1a"),
+           "VersionCompareEqual letters");
+  cmAssert(!cmSystemTools::VersionCompareGreater("1", "1a"),
+           "VersionCompareGreater letters");
+  cmAssert(cmSystemTools::VersionCompareEqual("001", "1"),
+           "VersionCompareEqual leading zeros equal");
+  cmAssert(!cmSystemTools::VersionCompareGreater("001", "1"),
+           "VersionCompareGreater leading zeros equal");
+  cmAssert(!cmSystemTools::VersionCompareEqual("002", "1"),
+           "VersionCompareEqual leading zeros greater");
+  cmAssert(cmSystemTools::VersionCompareGreater("002", "1"),
+           "VersionCompareGreater leading zeros greater");
+  cmAssert(!cmSystemTools::VersionCompareEqual("6.2.1", "6.3.1"),
+           "VersionCompareEqual components less");
+  cmAssert(!cmSystemTools::VersionCompareGreater("6.2.1", "6.3.1"),
+           "VersionCompareGreater components less");
+  cmAssert(!cmSystemTools::VersionCompareEqual("6.2.1", "6.2"),
+           "VersionCompareEqual different length");
+  cmAssert(cmSystemTools::VersionCompareGreater("6.2.1", "6.2"),
+           "VersionCompareGreater different length");
+  cmAssert(
+    !cmSystemTools::VersionCompareEqual(
+      "3.14159265358979323846264338327950288419716939937510582097494459230",
+      "3.14159265358979323846264338327950288419716939937510582097494459231"),
+    "VersionCompareEqual long number");
+  cmAssert(
+    !cmSystemTools::VersionCompareGreater(
+      "3.14159265358979323846264338327950288419716939937510582097494459230",
+      "3.14159265358979323846264338327950288419716939937510582097494459231"),
+    "VersionCompareGreater long number");
+  cmAssert(
+    !cmSystemTools::VersionCompareEqual(
+      "3.141592653589793238462643383279502884197169399375105820974944592307",
+      "3.14159265358979323846264338327950288419716939937510582097494459231"),
+    "VersionCompareEqual more digits");
+  cmAssert(
+    cmSystemTools::VersionCompareGreater(
+      "3.141592653589793238462643383279502884197169399375105820974944592307",
+      "3.14159265358979323846264338327950288419716939937510582097494459231"),
+    "VersionCompareGreater more digits");
+
+  // ----------------------------------------------------------------------
   // Test cmSystemTools::strverscmp
   cmAssert(cmSystemTools::strverscmp("", "") == 0, "strverscmp empty string");
   cmAssert(cmSystemTools::strverscmp("abc", "") > 0,