Add revision number comparison to test versions

require-ess/src/require.c

@@ -617,8 +617,7 @@ static int compareDigit(int found, int requested, const char *name) {
   return MATCH;
 }
 
-static int compareNumericVersion(semver_t *sv_found, semver_t *sv_request,
-                                 int already_matched) {
+static int compareNumericVersion(semver_t *sv_found, semver_t *sv_request) {
   int match;
 
   match = compareDigit(sv_found->major, sv_request->major, "major");
@@ -629,25 +628,7 @@ static int compareNumericVersion(semver_t *sv_found, semver_t *sv_request,
   if (match != MATCH) {
     return match;
   }
-  match = compareDigit(sv_found->patch, sv_request->patch, "patch");
-  if (match != MATCH) {
-    return match;
-  }
-
-  if (sv_request->revision == -1) {
-    if (already_matched) {
-      debug(
-          "require: compareVersions: No revision number requested. Returning "
-          "HIGHER\n");
-      return HIGHER;
-    } else {
-      debug(
-          "require: compareVersions: No revision number requested. Returning "
-          "MATCH\n");
-      return MATCH;
-    }
-  }
-  return compareDigit(sv_found->revision, sv_request->revision, "revision");
+  return compareDigit(sv_found->patch, sv_request->patch, "patch");
 }
 
 /*
@@ -672,16 +653,20 @@ static int compareVersions(const char *found, const char *request,
   sv_found = (semver_t *)calloc(1, sizeof(semver_t));
   sv_request = (semver_t *)calloc(1, sizeof(semver_t));
 
+  // TODO: maybe don't do this. This is only in the case that
+  // we have found an installed version with no revision number.
+  if (already_matched && sv_request->revision == -1) sv_request->revision = 0;
+
   parse_semver(found, sv_found);
   parse_semver(request, sv_request);
 
   // test version, look for exact.
   if (strlen(sv_request->test_str) > 0) {
-    if (strcmp(found, request) == 0) {
+    if (strcmp(sv_found->test_str, sv_request->test_str) == 0) {
       debug(
           "require: compareVersions: Test version requested and found, "
-          "matches exactly\n");
-      match = EXACT;
+          "matches\n");
+      match = MATCH;
     } else if (strlen(sv_found->test_str) > 0) {
       debug(
           "require: compareVersions: Test versions requested and found, no "
@@ -699,7 +684,27 @@ static int compareVersions(const char *found, const char *request,
         "found\n");
     match = MISMATCH;
   } else {
-    match = compareNumericVersion(sv_found, sv_request, already_matched);
+    match = compareNumericVersion(sv_found, sv_request);
+  }
+
+  // Finally, check revision numbers
+  if (match == MATCH) {
+    if (sv_request->revision == -1) {
+      if (already_matched) {
+        debug(
+            "require: compareVersions: No revision number for already found "
+            "version. Returning HIGHER\n");
+        match = HIGHER;
+      } else {
+        debug(
+            "require: compareVersions: No revision number requested. Returning "
+            "MATCH\n");
+        match = MATCH;
+      }
+    } else {
+      match =
+          compareDigit(sv_found->revision, sv_request->revision, "revision");
+    }
   }
   cleanup_semver(sv_found);
   cleanup_semver(sv_request);

require-ess/src/version.c

@@ -35,13 +35,13 @@ static void fetch_part(char *version_str, const regmatch_t *groups,
 
 int parse_semver(const char *version, semver_t *s) {
   static const char *version_regex =
-      "^(([0-9]+)\\.([0-9]+)\\.([0-9]+)(\\+([0-9]+))?)?(.*)$";
+      "^(([0-9]+)\\.([0-9]+)\\.([0-9]+))?([^+]*)(\\+([0-9]+))?$";
   static const unsigned int max_regex_groups = 7 + 1;
   static const unsigned int major_ix = 2;
   static const unsigned int minor_ix = 3;
   static const unsigned int patch_ix = 4;
-  static const unsigned int revision_ix = 6;
-  static const unsigned int test_ix = 7;
+  static const unsigned int test_label_ix = 5;
+  static const unsigned int revision_ix = 7;
 
   regex_t compiled;
   regmatch_t groups[max_regex_groups];
@@ -60,10 +60,13 @@ int parse_semver(const char *version, semver_t *s) {
     fetch_part(s->version_str, groups, major_ix, &s->major);
     fetch_part(s->version_str, groups, minor_ix, &s->minor);
     fetch_part(s->version_str, groups, patch_ix, &s->patch);
+    if (groups[test_label_ix].rm_so != groups[test_label_ix].rm_eo) {
+      s->test_str = s->version_str;
+    }
     if (groups[revision_ix].rm_so != (regoff_t)-1) {
       fetch_part(s->version_str, groups, revision_ix, &s->revision);
+      s->version_str[groups[revision_ix].rm_so - 1] = 0;
     }
-    s->test_str = s->version_str + groups[test_ix].rm_so;
   }
   return 0;
 }

tests/test_versions.py

@@ -32,6 +32,12 @@ RE_MODULE_NOT_LOADED = "Module {module} (not available|version {required} not av
         ("", "0.0.1+0", ["0.0.1+0", "1.0"]),
         # Numeric version should be prioritised over "higher" test version
         ("", "0.1.0+0", ["0.1.0+0", "1.0.0-rc1"]),
+        # Pick the latest test version based on revision number
+        ("test", "test+2", ["test+0", "test+2"]),
+        # Don't install an incorrect test version
+        ("foo", "", ["bar"]),
+        # Mix of test and numeric
+        ("1.2.3-test", "1.2.3-test+2", ["1.2.3-test", "1.2.3-test+0", "1.2.3-test+2"]),
     ],
 )
 def test_version(wrapper: Wrapper, requested, expected, installed):