Compare prerelease versions correctly for updates

install.ps1

@@ -100,8 +100,43 @@ function Release-LatestTagDownloadUrl {
     return $null
 }
 
+function Version-Core($Value) {
+    $base = $Value.TrimStart("v").Split("+")[0]
+    $base.Split("-")[0]
+}
+
+function Version-Prerelease($Value) {
+    $base = $Value.TrimStart("v").Split("+")[0]
+    $dash = $base.IndexOf("-")
+    if ($dash -lt 0) {
+        return ""
+    }
+    $base.Substring($dash + 1).ToLowerInvariant()
+}
+
 function Version-Parts($Value) {
-    [regex]::Matches($Value, "\d+") | ForEach-Object { [int64]$_.Value }
+    [regex]::Matches((Version-Core $Value), "\d+") | ForEach-Object { [int64]$_.Value }
+}
+
+function Compare-Prerelease($Left, $Right) {
+    if (-not $Left -and -not $Right) {
+        return 0
+    }
+    if (-not $Left) {
+        return 1
+    }
+    if (-not $Right) {
+        return -1
+    }
+    if ($Left -eq $Right) {
+        return 0
+    }
+    if ($Left -match "(?i)^rc(\d+)$" -and $Right -match "(?i)^rc(\d+)$") {
+        $leftRc = [int64]([regex]::Match($Left, "(?i)^rc(\d+)$").Groups[1].Value)
+        $rightRc = [int64]([regex]::Match($Right, "(?i)^rc(\d+)$").Groups[1].Value)
+        return $leftRc.CompareTo($rightRc)
+    }
+    return [string]::CompareOrdinal($Left, $Right)
 }
 
 function Compare-DoshVersion($Left, $Right) {
@@ -114,7 +149,7 @@ function Compare-DoshVersion($Left, $Right) {
         if ($l -lt $r) { return -1 }
         if ($l -gt $r) { return 1 }
     }
-    return 0
+    return Compare-Prerelease (Version-Prerelease $Left) (Version-Prerelease $Right)
 }
 
 function Current-SourceVersion {

install.sh

@@ -222,20 +222,60 @@ release_latest_tag_download_url() {
   esac
 }
 
-version_part() {
+version_core() {
+  value="${1#v}"
+  value="${value%%+*}"
+  printf '%s\n' "${value%%-*}"
+}
+
+version_prerelease() {
+  value="${1#v}"
+  value="${value%%+*}"
+  case "$value" in
+    *-*) printf '%s\n' "${value#*-}" ;;
+    *) printf '\n' ;;
+  esac
+}
+
+version_core_part() {
   version="$1"
   index="$2"
-  printf '%s\n' "$version" \
+  version_core "$version" \
     | sed 's/[^0-9][^0-9]*/ /g' \
     | awk -v index="$index" '{ value=$index; if (value == "") value=0; print value }'
 }
 
+prerelease_less_than() {
+  left="$1"
+  right="$2"
+  if [ -z "$left" ] && [ -z "$right" ]; then
+    return 1
+  fi
+  if [ -z "$left" ]; then
+    return 1
+  fi
+  if [ -z "$right" ]; then
+    return 0
+  fi
+  if [ "$left" = "$right" ]; then
+    return 1
+  fi
+  left_rc="$(printf '%s\n' "$left" | sed -n 's/^[Rr][Cc]\([0-9][0-9]*\)$/\1/p')"
+  right_rc="$(printf '%s\n' "$right" | sed -n 's/^[Rr][Cc]\([0-9][0-9]*\)$/\1/p')"
+  if [ -n "$left_rc" ] && [ -n "$right_rc" ]; then
+    [ "$left_rc" -lt "$right_rc" ]
+    return
+  fi
+  first="$(printf '%s\n%s\n' "$left" "$right" | LC_ALL=C sort | sed -n '1p')"
+  [ "$first" = "$left" ]
+}
+
 version_less_than() {
   left="$1"
   right="$2"
   for index in 1 2 3 4 5 6 7 8; do
-    lpart="$(version_part "$left" "$index")"
+    lpart="$(version_core_part "$left" "$index")"
-    rpart="$(version_part "$right" "$index")"
+    rpart="$(version_core_part "$right" "$index")"
     if [ "$lpart" -lt "$rpart" ]; then
       return 0
     fi
@@ -243,7 +283,7 @@ version_less_than() {
       return 1
     fi
   done
-  return 1
+  prerelease_less_than "$(version_prerelease "$left")" "$(version_prerelease "$right")"
 }
 
 current_source_version() {

src/bin/dosh-client.rs

@@ -4135,15 +4135,71 @@ fn update_binary_version_for_installer(local: &str, latest_tag: Option<&str>) ->
 }
 
 fn compare_dotted_versions(left: &str, right: &str) -> std::cmp::Ordering {
-    let left = parse_dotted_version(left);
+    let left = parse_comparable_version(left);
-    let right = parse_dotted_version(right);
+    let right = parse_comparable_version(right);
-    let width = left.len().max(right.len());
+    let width = left.core.len().max(right.core.len());
     for index in 0..width {
         let ordering = left
+            .core
             .get(index)
             .copied()
             .unwrap_or(0)
-            .cmp(&right.get(index).copied().unwrap_or(0));
+            .cmp(&right.core.get(index).copied().unwrap_or(0));
+        if ordering != std::cmp::Ordering::Equal {
+            return ordering;
+        }
+    }
+    compare_prerelease(left.prerelease.as_deref(), right.prerelease.as_deref())
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+struct ComparableVersion {
+    core: Vec<u64>,
+    prerelease: Option<String>,
+}
+
+fn parse_comparable_version(version: &str) -> ComparableVersion {
+    let version = version.strip_prefix('v').unwrap_or(version);
+    let version = version.split_once('+').map_or(version, |(base, _)| base);
+    let (core, prerelease) = version
+        .split_once('-')
+        .map_or((version, None), |(core, pre)| (core, Some(pre)));
+    ComparableVersion {
+        core: parse_dotted_version(core),
+        prerelease: prerelease
+            .filter(|value| !value.is_empty())
+            .map(|value| value.to_ascii_lowercase()),
+    }
+}
+
+fn compare_prerelease(left: Option<&str>, right: Option<&str>) -> std::cmp::Ordering {
+    match (left, right) {
+        (None, None) => std::cmp::Ordering::Equal,
+        (None, Some(_)) => std::cmp::Ordering::Greater,
+        (Some(_), None) => std::cmp::Ordering::Less,
+        (Some(left), Some(right)) => compare_prerelease_identifiers(left, right),
+    }
+}
+
+fn compare_prerelease_identifiers(left: &str, right: &str) -> std::cmp::Ordering {
+    let left_parts = left.split(['.', '-']).collect::<Vec<_>>();
+    let right_parts = right.split(['.', '-']).collect::<Vec<_>>();
+    let width = left_parts.len().max(right_parts.len());
+    for index in 0..width {
+        let Some(left) = left_parts.get(index).copied() else {
+            return std::cmp::Ordering::Less;
+        };
+        let Some(right) = right_parts.get(index).copied() else {
+            return std::cmp::Ordering::Greater;
+        };
+        let left_number = parse_numeric_identifier(left);
+        let right_number = parse_numeric_identifier(right);
+        let ordering = match (left_number, right_number) {
+            (Some(left), Some(right)) => left.cmp(&right),
+            (Some(_), None) => std::cmp::Ordering::Less,
+            (None, Some(_)) => std::cmp::Ordering::Greater,
+            (None, None) => left.cmp(right),
+        };
         if ordering != std::cmp::Ordering::Equal {
             return ordering;
         }
@@ -4151,6 +4207,12 @@ fn compare_dotted_versions(left: &str, right: &str) -> std::cmp::Ordering {
     std::cmp::Ordering::Equal
 }
 
+fn parse_numeric_identifier(value: &str) -> Option<u64> {
+    (!value.is_empty() && value.bytes().all(|byte| byte.is_ascii_digit()))
+        .then(|| value.parse::<u64>().ok())
+        .flatten()
+}
+
 fn parse_dotted_version(version: &str) -> Vec<u64> {
     version
         .split(|byte: char| !byte.is_ascii_digit())
@@ -11226,6 +11288,14 @@ mod tests {
             update_binary_version_for_installer("1.0.0-rc41", Some("v1.0.0-rc42")),
             None
         );
+        assert_eq!(
+            update_binary_version_for_installer("1.0.0", Some("v1.0.0-rc42")).as_deref(),
+            Some("v1.0.0")
+        );
+        assert_eq!(
+            update_binary_version_for_installer("1.0.0-rc42", Some("v1.0.0")),
+            None
+        );
         let config = ClientConfig::default();
         let unix = unix_update_script(
             &config,
@@ -11357,6 +11427,15 @@ mod tests {
             update_version_status("1.0.0-rc40", "1.0.0-rc37"),
             "local newer than latest release"
         );
+        assert_eq!(
+            update_version_status("1.0.0-rc42", "1.0.0"),
+            "update available"
+        );
+        assert_eq!(
+            update_version_status("1.0.0", "1.0.0-rc42"),
+            "local newer than latest release"
+        );
+        assert_eq!(update_version_status("1.0.0", "1.0.0+build.7"), "current");
     }
 
     #[test]

tests/release_scripts.rs

@@ -44,6 +44,8 @@ fn installers_skip_stale_latest_release_prebuilts() {
     assert!(install.contains("latest_release_is_stale"));
     assert!(install.contains("current_source_version"));
     assert!(install.contains("latest release $latest is older than source $current"));
+    assert!(install.contains("version_prerelease"));
+    assert!(install.contains("prerelease_less_than"));
     assert!(
         install.contains("if latest_release_is_stale; then"),
         "unix installer must skip stale latest before downloading a prebuilt"
@@ -53,6 +55,8 @@ fn installers_skip_stale_latest_release_prebuilts() {
     assert!(ps1.contains("Latest-ReleaseIsStale"));
     assert!(ps1.contains("Current-SourceVersion"));
     assert!(ps1.contains("latest release $latestVersion is older than source $current"));
+    assert!(ps1.contains("Version-Prerelease"));
+    assert!(ps1.contains("Compare-Prerelease"));
     assert!(
         ps1.contains("if (Latest-ReleaseIsStale)"),
         "windows installer must skip stale latest before downloading a prebuilt"