Support -N package-release suffix in Salt version strings

linux/svtminion.sh

@@ -408,14 +408,17 @@ _set_log_level() {
 #
 # _salt_onedir_dir_is_ga
 #
-#   True (status 0) if the onedir directory name is GA numeric CalVer only
-#       (digits and dots). Prerelease names (e.g. 3008.0rc1) are not GA.
+#   True (status 0) if the onedir directory name is GA CalVer, optionally
+#       with a -N package-release suffix (e.g. 3008.1 or 3008.1-1).
+#       Prerelease names (e.g. 3008.0rc1) are not GA. A -N suffix is a
+#       repackage of the same version, not a prerelease, so it counts as GA
+#       and can win latest/major-series selection via sort -V.
 #
 # Results:
 #   0 if GA, 1 otherwise
 #
 _salt_onedir_dir_is_ga() {
-    local _ga_re='^[0-9]+\.[0-9]+(\.[0-9]+)*$'
+    local _ga_re='^[0-9]+\.[0-9]+(\.[0-9]+)*(-[0-9]+)?$'
     [[ -n "$1" && "$1" =~ ${_ga_re} ]]
 }
 

tests/linux/test_version_resolution.sh

@@ -0,0 +1,96 @@
+#!/usr/bin/env bash
+#
+# Lightweight, fixture-free regression test for linux/svtminion.sh's Salt
+# version resolution logic (_salt_onedir_dir_is_ga / _get_desired_salt_version_fn).
+#
+# Unlike test-linux.sh, this does not perform a real install and needs no
+# onedir tarball fixtures -- it extracts the two functions under test
+# directly from the real script and exercises them against a directory of
+# zero-byte stub files, so it only tests directory-name resolution logic
+# (GA classification, latest/major-series selection, exact-name lookup),
+# not extraction or installation.
+#
+# Run directly: bash tests/linux/test_version_resolution.sh
+
+set -o nounset
+set -o errexit
+set -o pipefail
+
+_test_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+_repo_root="$(cd "${_test_dir}/../.." && pwd)"
+_script="${_repo_root}/linux/svtminion.sh"
+
+# Stub the logging functions _get_desired_salt_version_fn depends on so it
+# can run standalone without the rest of svtminion.sh's global state.
+_error_log() { echo "ERROR: $*" 1>&2; }
+_info_log() { :; }
+_debug_log() { :; }
+
+# Extract the two functions under test verbatim from the real script, so
+# this test always exercises the current implementation rather than a
+# hand-copied duplicate that could drift out of sync.
+_extracted="$(mktemp)"
+trap 'rm -f "${_extracted}"' EXIT
+
+sed -n '/^_salt_onedir_dir_is_ga() {/,/^}/p' "${_script}" > "${_extracted}"
+sed -n '/^_get_desired_salt_version_fn() {/,/^}/p' "${_script}" >> "${_extracted}"
+
+# shellcheck disable=SC1090
+source "${_extracted}"
+
+_fixture_dir="$(mktemp -d)"
+trap 'rm -rf "${_fixture_dir}"; rm -f "${_extracted}"' EXIT
+
+for v in "3008.0rc1" "3008.1" "3008.1-1" "3007.9"; do
+    touch "${_fixture_dir}/${v}"
+done
+
+_failed=0
+
+_assert_resolves_to() {
+    local requested="$1"
+    local expected="$2"
+    local label="$3"
+
+    salt_url_version="${requested}"
+    _GENERIC_PKG_VERSION=""
+    salt_specific_version=""
+
+    if ! _get_desired_salt_version_fn "${_fixture_dir}"; then
+        echo "FAILED: ${label} -- _get_desired_salt_version_fn returned non-zero for '${requested}'"
+        _failed=1
+        return
+    fi
+
+    if [ "${salt_specific_version}" != "${expected}" ]; then
+        echo "FAILED: ${label} -- requested '${requested}', expected '${expected}', got '${salt_specific_version}'"
+        _failed=1
+    else
+        echo "OK: ${label} -- '${requested}' resolved to '${expected}'"
+    fi
+}
+
+# latest / major-series resolution must prefer the newest GA build,
+# including a -N repackage of a version that also exists without the suffix.
+_assert_resolves_to "latest" "3008.1-1" "latest picks -N repackage over bare version"
+_assert_resolves_to "3008" "3008.1-1" "major-series picks -N repackage over bare version"
+
+# Exact-name request for a prerelease still works (unchanged RC behavior).
+_assert_resolves_to "3008.0rc1" "3008.0rc1" "exact RC request"
+
+# Exact-name request for a -N build works.
+_assert_resolves_to "3008.1-1" "3008.1-1" "exact -N request"
+
+# Mere-presence check: requesting an unrelated version must not fail/crash
+# just because -N and rc directories exist alongside it in the listing.
+# This is the same failure mode that previously broke RC handling before
+# GA classification distinguished "present" from "requested".
+_assert_resolves_to "3007.9" "3007.9" "mere presence of -N/rc does not break unrelated exact request"
+
+if [ "${_failed}" -ne 0 ]; then
+    echo "test_version_resolution.sh: FAILED"
+    exit 1
+fi
+
+echo "test_version_resolution.sh: All tests passed"
+exit 0

tests/windows/functional/test_input_validation.ps1

@@ -42,12 +42,15 @@ function test_Test-MinionVersionParameter {
         @{ MinionVersion = "3008.0rc1"; Expected = $true }
         @{ MinionVersion = "3009.0rc2"; Expected = $true }
         @{ MinionVersion = "3009.1.2"; Expected = $true }
+        @{ MinionVersion = "3008.1-1"; Expected = $true }
+        @{ MinionVersion = "3004.2-1"; Expected = $true }
         @{ MinionVersion = "bad version"; Expected = $false }
         @{ MinionVersion = "3006/evil"; Expected = $false }
         @{ MinionVersion = "abc"; Expected = $false }
         @{ MinionVersion = '3006;rm -rf /'; Expected = $false }
         @{ MinionVersion = ""; Expected = $false }
         @{ MinionVersion = "3006 .2"; Expected = $false }
+        @{ MinionVersion = "3008.1-1-2"; Expected = $false }
     )
     $failed = 0
     foreach ($c in $cases) {

tests/windows/functional/test_version_helpers.ps1

@@ -16,6 +16,7 @@ function test_Test-SaltOnedirVersionIsGA {
         @{ Version = "3006.24"; Expected = $true }
         @{ Version = "3008.0"; Expected = $true }
         @{ Version = "3006.1"; Expected = $true }
+        @{ Version = "3008.1-1"; Expected = $true }
         @{ Version = "3008.0rc1"; Expected = $false }
         @{ Version = "3007.1dev1"; Expected = $false }
         @{ Version = "notaversion"; Expected = $false }
@@ -42,6 +43,11 @@ function test_Compare-SaltCalVer {
         @{ Left = "3006.24"; Right = "3006.8"; Expected = 1 }
         @{ Left = "3006.8"; Right = "3006.24"; Expected = -1 }
         @{ Left = "3008.0"; Right = "3007.99"; Expected = 1 }
+        @{ Left = "3008.1-1"; Right = "3008.1"; Expected = 1 }
+        @{ Left = "3008.1"; Right = "3008.1-1"; Expected = -1 }
+        @{ Left = "3008.1-2"; Right = "3008.1-1"; Expected = 1 }
+        @{ Left = "3008.2"; Right = "3008.1-1"; Expected = 1 }
+        @{ Left = "3008.1-1"; Right = "3008.1-1"; Expected = 0 }
     )
     $failed = 0
     foreach ( $c in $cases ) {

windows/svtminion.ps1

@@ -1753,13 +1753,14 @@ function Test-MinionVersionParameter {
     )
 
     # Valid: "latest", 4-digit major (3006), major.minor (3006.2),
-    # multi-part (3006.24), rc suffix (3008.0rc1)
-    if ($MinionVersion -notmatch '^(latest|\d{4}(\.\d+(\.\d+)*(rc\d+)?)?)$') {
+    # multi-part (3006.24), rc suffix (3008.0rc1), package-release suffix
+    # (3008.1-1)
+    if ($MinionVersion -notmatch '^(latest|\d{4}(\.\d+(\.\d+)*(rc\d+|-\d+)?)?)$') {
         $msg = "Invalid MinionVersion: $MinionVersion"
         Write-Log $msg -Level error
         Write-Host $msg -ForegroundColor Red
         $msg = "Must be 'latest', a major version (e.g. 3006), " +
-               "or a full version (e.g. 3006.2, 3008.0rc1)"
+               "or a full version (e.g. 3006.2, 3008.0rc1, 3008.1-1)"
         Write-Host $msg -ForegroundColor Yellow
         return $false
     }
@@ -1875,37 +1876,58 @@ function Get-MajorVersion {
 
 
 function Test-SaltOnedirVersionIsGA {
-    # True if the onedir directory name is a GA CalVer (digits and dots only).
-    # Prerelease dirs (e.g. 3008.0rc1) are not GA; install those only via exact
-    # -MinionVersion matching the directory name.
+    # True if the onedir directory name is a GA CalVer, optionally with a -N
+    # package-release suffix (e.g. 3008.1 or 3008.1-1). Prerelease dirs (e.g.
+    # 3008.0rc1) are not GA; install those only via exact -MinionVersion
+    # matching the directory name. A -N suffix is a repackage of the same
+    # version, not a prerelease, so it counts as GA and participates in
+    # latest/major-series selection via Compare-SaltCalVer.
     [CmdletBinding()]
     param(
         [Parameter(Mandatory=$true)]
         [String] $Version
     )
-    return [bool]( $Version -match '^\d+\.\d+(\.\d+)*$' )
+    return [bool]( $Version -match '^\d+\.\d+(\.\d+)*(-\d+)?$' )
 }
 
 
 function Compare-SaltCalVer {
-    # Compare two GA numeric CalVer strings (e.g. 3006.24 vs 3007.0). Returns 1
-    # if Left is greater than Right, -1 if less, 0 if equal.
+    # Compare two GA CalVer strings, each optionally carrying a -N
+    # package-release suffix (e.g. 3006.24, 3008.1-1). Returns 1 if Left is
+    # greater than Right, -1 if less, 0 if equal. The dotted version is
+    # compared first; a missing -N suffix is treated as release 0, so
+    # 3008.1-1 compares greater than 3008.1.
     [CmdletBinding()]
     param(
         [Parameter(Mandatory=$true)]
         [String] $Left,
         [Parameter(Mandatory=$true)]
         [String] $Right
     )
-    $left_parts = @( ($Left -split '\.') | ForEach-Object { [int]$_ } )
-    $right_parts = @( ($Right -split '\.') | ForEach-Object { [int]$_ } )
-    $max_len = [Math]::Max($left_parts.Count, $right_parts.Count)
+    function Get-CalVerParts {
+        param([String] $Version)
+        $release = 0
+        $dotted = $Version
+        if ( $Version -match '^(.+)-(\d+)$' ) {
+            $dotted = $Matches[1]
+            $release = [int]$Matches[2]
+        }
+        return @{
+            Dotted  = @( ($dotted -split '\.') | ForEach-Object { [int]$_ } )
+            Release = $release
+        }
+    }
+    $left_parts = Get-CalVerParts $Left
+    $right_parts = Get-CalVerParts $Right
+    $max_len = [Math]::Max($left_parts.Dotted.Count, $right_parts.Dotted.Count)
     for ( $i = 0; $i -lt $max_len; $i++ ) {
-        $a = if ( $i -lt $left_parts.Count ) { $left_parts[$i] } else { 0 }
-        $b = if ( $i -lt $right_parts.Count ) { $right_parts[$i] } else { 0 }
+        $a = if ( $i -lt $left_parts.Dotted.Count ) { $left_parts.Dotted[$i] } else { 0 }
+        $b = if ( $i -lt $right_parts.Dotted.Count ) { $right_parts.Dotted[$i] } else { 0 }
         if ( $a -gt $b ) { return 1 }
         if ( $a -lt $b ) { return -1 }
     }
+    if ( $left_parts.Release -gt $right_parts.Release ) { return 1 }
+    if ( $left_parts.Release -lt $right_parts.Release ) { return -1 }
     return 0
 }