fixed version comparisons, previously done lexicographically

slsSupportLib/include/sls/Version.h

@@ -16,6 +16,10 @@ class Version {
   public:
     explicit Version(const std::string &s);
 
+    static bool parseSemanticVersion(const std::string &version, int &major,
+                                     int &minor, int &patch,
+                                     std::string &extra);
+
     bool hasSemanticVersioning() const;
     std::string getVersion() const;
     std::string getDate() const;

slsSupportLib/src/Version.cpp

@@ -1,12 +1,32 @@
 // SPDX-License-Identifier: LGPL-3.0-or-other
 // Copyright (C) 2021 Contributors to the SLS Detector Package
 #include "sls/Version.h"
+#include "sls/logger.h"
 #include "sls/sls_detector_exceptions.h"
 #include "sls/string_utils.h"
+#include <cstdio>
 #include <sstream>
 
 namespace sls {
 
+bool Version::parseSemanticVersion(const std::string &version, int &major,
+                                   int &minor, int &patch, std::string &extra) {
+    char extraBuffer[256] = {'\0'};
+    // Parse major.minor.patch and capture any optional trailing suffix.
+    const int scanned = sscanf(version.c_str(), "%d.%d.%d%255s", &major, &minor,
+                               &patch, extraBuffer);
+
+    if (scanned == 3) {
+        extra.clear();
+        return true;
+    }
+    if (scanned == 4) {
+        extra = extraBuffer;
+        return true;
+    }
+    return false;
+}
+
 Version::Version(const std::string &s) {
     auto list = split(s, ' ');
     // only date from previous releases
@@ -62,14 +82,27 @@ bool Version::operator==(const Version &other) const {
 bool Version::operator<=(const Version &other) const {
     // both have semantic versioning
     if (hasSemanticVersioning() && other.hasSemanticVersioning()) {
-        // release version
-        std::string otherVersion = other.getVersion();
-        if (version_ == otherVersion)
-            return true;
-        // less than
-        return (std::lexicographical_compare(version_.begin(), version_.end(),
-                                             otherVersion.begin(),
-                                             otherVersion.end()));
+        int major = 0, minor = 0, patch = 0;
+        int otherMajor = 0, otherMinor = 0, otherPatch = 0;
+        std::string extra;
+        std::string otherExtra;
+
+        if (Version::parseSemanticVersion(version_, major, minor, patch,
+                                          extra) &&
+            Version::parseSemanticVersion(other.getVersion(), otherMajor,
+                                          otherMinor, otherPatch, otherExtra)) {
+            if (major != otherMajor)
+                return major < otherMajor;
+            if (minor != otherMinor)
+                return minor < otherMinor;
+            if (patch != otherPatch)
+                return patch < otherPatch;
+            return extra <= otherExtra;
+        }
+
+        LOG(logWARNING) << "Could not compare semantic versions: " << version_
+                        << " and " << other.getVersion()
+                        << ". Comparing dates instead.";
     }
     // compare dates
     return date_ <= other.getDate();

slsSupportLib/tests/test-Version.cpp

@@ -16,4 +16,39 @@ TEST_CASE("check if version is semantic", "[version]") {
     CHECK(version.hasSemanticVersioning() == has_semantic_version);
 }
 
+TEST_CASE("semantic version ordering compares numerically", "[version]") {
+    Version v9("9.1.1 0x250512");
+    Version v10("10.0.1 0x250512");
+
+    CHECK(v9 <= v10);
+    CHECK_FALSE(v10 <= v9);
+}
+
+TEST_CASE("semantic version compares extra suffix lexicographically",
+          "[version]") {
+    Version rc("10.1.1.rc 0x250512");
+    Version zz("10.1.1.zz 0x250512");
+
+    CHECK(rc <= zz);
+    CHECK_FALSE(zz <= rc);
+}
+
+TEST_CASE(
+    "semantic comparison falls back to date on malformed semantic version",
+    "[version]") {
+    Version malformed("10.1 0x250512");
+    Version valid("10.1.1 0x250513");
+
+    CHECK(malformed <= valid);
+    CHECK_FALSE(valid <= malformed);
+}
+
+TEST_CASE("semantic version compares extra suffix and without", "[version]") {
+    Version v1011rc("10.1.1.rc 0x250512");
+    Version v1010("10.1.0 0x250512");
+
+    CHECK_FALSE(v1011rc <= v1010);
+    CHECK(v1010 <= v1011rc);
+}
+
 } // namespace sls