fix(arcup): honor SemVer prerelease precedence in version_gt (fixes #205)

[danilaverbena]

Problem (fixes #205)

version_gt() strips the prerelease suffix from both operands before comparing:

ver1="${ver1%%-*}"
ver2="${ver2%%-*}"

So any prerelease information is discarded and versions that differ only in their prerelease compare as equal:

• version_gt "0.3.0" "0.3.0-rc.1" returns non-zero → the stable release is not seen as newer than its release candidate.
• version_gt "0.3.0-rc.2" "0.3.0-rc.1" also returns non-zero → a newer RC is not detected.

Because version_gt drives the installer's self-update checks (arcup/arcup lines ~413-430 and ~454-462), arcup fails to recognise a stable release as an upgrade over a previously installed prerelease.

Fix

Implement SemVer precedence (semver.org §11) in version_gt:

1. Split each version into its major.minor.patch core and its prerelease, and drop build metadata (+...), which is ignored for precedence.
2. Compare the core numerically (unchanged behaviour).
3. When cores are equal, apply prerelease precedence:
   • a version without a prerelease outranks the same version with one;
   • otherwise compare dot-separated identifiers left to right — numeric identifiers compared numerically, numeric ranked lower than alphanumeric, alphanumeric compared in ASCII order, and a larger set of identifiers outranks a smaller one when all preceding identifiers are equal.

Testing

Verified with a local harness covering the reported cases plus the full precedence chain from the SemVer spec (§11.4):

1.0.0-alpha < 1.0.0-alpha.1 < 1.0.0-alpha.beta < 1.0.0-beta
            < 1.0.0-beta.2 < 1.0.0-beta.11 < 1.0.0-rc.1 < 1.0.0

All 18 assertions pass, including the two bug reproductions from the issue, existing core-version comparisons, and build-metadata being ignored. bash -n clean; behaviour for plain major.minor.patch inputs is unchanged.

Scope: single self-contained function in arcup/arcup; no other changes.

[osr21]

Good fix — the bug is real and the implementation is substantially correct. A few observations:

Correctness check

The two reported cases from #205 both work correctly with the new code:

• version_gt "0.3.0" "0.3.0-rc.1": cores equal, pre1="", pre2="rc.1" → hits [ -z "$pre1" ] && return 0 ✓
• version_gt "0.3.0-rc.2" "0.3.0-rc.1": cores equal, both have prereleases → identifier arrays ["rc","2"] vs ["rc","1"] → rc=rc (continue), 2 > 1 numeric → return 0 ✓

The full SemVer §11.4 precedence chain also holds:

Comparison	Expected	Result
alpha < alpha.1	fewer ids → lower	a1 exhausted → return 1 ✓
alpha.1 < alpha.beta	numeric < alphanum	1 is numeric, beta is alphanum → return 1 ✓
alpha.beta < beta	ASCII order	"alpha" < "beta" → return 1 ✓
beta < beta.2	more ids → higher	a1 exhausted → return 1 ✓
beta.2 < beta.11	numeric, not lexicographic	2 < 11 integer comparison → return 1 ✓
rc.1 < 1.0.0	stable > any prerelease	pre2="" → [ -z "$pre2" ] && return 1 ✓

One subtle issue: %% vs % on the prerelease strip

local core1="${v1%%-*}" pre1=""

%% removes the longest suffix matching -*, which is correct — for 1.0.0-rc.1+build, the longest -* suffix is -rc.1+build, leaving core1="1.0.0". If % (shortest) had been used instead, it would only strip the last -something segment and produce a wrong core. This is correct as written, just worth calling out explicitly since it's the load-bearing operator choice.

Minor: glob expansion in the array split

local LC_ALL=C IFS=.
local -a a1=($pre1) a2=($pre2)

($pre1) is subject to pathname expansion before word-splitting. If the current working directory happened to contain files matching a prerelease identifier (unlikely for rc, alpha, beta, but not impossible in a dev environment running arcup from a dirty tree), an identifier could expand unexpectedly. Defensive fix:

local LC_ALL=C IFS=. -
set -f
local -a a1=($pre1) a2=($pre2)
set +f

Or equivalently wrap in a subshell with noglob. For Arc version strings in practice this will never trigger, so it's a minor nit rather than a blocking issue.

Build metadata with embedded hyphens (edge case, not a blocker)

For a version string like 1.0.0+build-123 (build metadata containing a hyphen — unusual but valid per spec):

• "${v1%%-*}" strips the longest -* suffix → core1="1.0.0+build", then %%+* → "1.0.0" ✓
• But "$v1" ≠ "$core1" is true (they differ), so pre1="${v1#*-}" → "123"
• After %%+* on pre1: no + present → pre1="123" — incorrectly treated as a prerelease

This only fires when build metadata itself contains a hyphen. Arc release tags (v0.7.3, v0.7.3-rc.1) never produce this pattern, so it has no practical impact. Flagging for completeness.

Test harness

The PR description mentions a local harness with 18 assertions. If there's any path to committing that harness alongside arcup (even as arcup/version_gt_test.sh with a # NOT installed comment), it would let CI or a maintainer reproduce the verification without re-deriving it from the spec. Understandable if the project doesn't have shell test infrastructure today.

Bash dependency

local -a, for (( )), and [[ ]] are bash extensions. The existing function already uses local (not POSIX sh), so this is an established assumption throughout arcup. No regression here, just confirming the script should always be invoked via bash rather than sh.

[danilaverbena]

@osr21 thank you for the thorough review — much appreciated. Pushed a follow-up addressing both points:

Build metadata containing a hyphen — fixed. Build metadata is now stripped up front, before the prerelease split:

local v1="${1#v}"; v1="${v1%%+*}"

So 1.0.0+build-123 yields core=1.0.0, pre="" and is no longer misread as having a 123 prerelease. Added regression tests for it.

Glob expansion in the array split — fixed by splitting with read -ra instead of an unquoted array assignment:

local -a a1 a2
IFS=. read -ra a1 <<< "$pre1"
IFS=. read -ra a2 <<< "$pre2"

read -ra performs word-splitting on IFS without pathname expansion, so no set -f/set +f is needed and the function doesn't mutate shell options. Verified by running the new test suite from a working directory containing files named after prerelease identifiers (1, 2, 11, rc, beta.2, ...) — all pass.

%% vs % — good catch, kept the longest-match strip; stripping build metadata first also keeps core extraction robust.

Test harness — added arcup/version_gt_test.sh (sources arcup with ARCUP_SKIP_MAIN=1). 22 assertions: the two #205 reproductions, the full SemVer §11.4 chain, build-metadata handling, and the hyphen edge case. Run with bash arcup/version_gt_test.sh → 22 passed, 0 failed.

Bash dependency — confirmed; arcup is already #!/usr/bin/env bash under set -euo pipefail, so no new assumption.

[osr21]

Both fixes are correct — the implementation is now clean.

Build metadata strip — v1="${v1%%+*}" before the prerelease split is the right order. For 1.0.0+build-123: strips to 1.0.0, then core1="1.0.0", pre1="". No false prerelease. ✓

Glob expansion — IFS=. read -ra a1 <<< "$pre1" avoids pathname expansion entirely without touching shell options. Cleaner than the set -f approach I suggested. ✓

LC_ALL=C — confirmed present in the updated code (local LC_ALL=C before the array declarations), so the [[ "$id1" > "$id2" ]] alphanumeric comparisons use ASCII ordering as SemVer requires. ✓

One small test gap

The test suite covers build-only versions (assert_le "1.0.0+build.9" "1.0.0+build.1") and build-with-hyphen on a stable core (assert_le "1.0.0+build-123" "1.0.0"), but not the combined prerelease + build metadata case:

# 1.0.0-rc.1+build-123 should equal 1.0.0-rc.1 for precedence purposes
assert_le "1.0.0-rc.1+build-123" "1.0.0-rc.1"
assert_le "1.0.0-rc.1" "1.0.0-rc.1+build-123"
# rc.2 still beats rc.1 regardless of build metadata on either side
assert_gt "1.0.0-rc.2+build-1" "1.0.0-rc.1+build-99"

The logic handles these correctly (strip metadata first, then compare prereleases), but the regression suite doesn't explicitly exercise the path where both a prerelease and build metadata are present simultaneously. Worth adding the three assertions above before merge.

Everything else is solid. LGTM once those three assertions land.

[danilaverbena]

@osr21 added the three assertions — thanks for catching the gap.

# Prerelease combined with build metadata: build metadata is ignored, so
# precedence is decided purely by the prerelease identifiers.
assert_le "1.0.0-rc.1+build-123" "1.0.0-rc.1"
assert_le "1.0.0-rc.1" "1.0.0-rc.1+build-123"
assert_gt "1.0.0-rc.2+build-1" "1.0.0-rc.1+build-99"

The suite now runs 25 passed, 0 failed. Verified locally against the branch's arcup, including from a working directory containing files named after the prerelease/build identifiers (rc.1, rc.2, build-1, build-99, ...) to double-check the read -ra split stays glob-safe. Ready for another look.

[osr21]

Traced all three new assertions against the implementation — all correct.

assert_le "1.0.0-rc.1+build-123" "1.0.0-rc.1" and its reverse

Both operands strip to 1.0.0-rc.1 via %%+*. After that: core1=core2=1.0.0, pre1=pre2=rc.1. The identifier loop runs rc=rc, 1=1, exhausts both arrays, falls through to return 1 (not gt). assert_le passes on both directions. ✓

The key ordering — %%+* before %%-* — is what makes this correct. If build metadata were stripped after the prerelease split, 1.0.0-rc.1+build-123 would produce pre1="rc.1+build-123", which would split into ["rc", "1+build-123"] and the last identifier would fail the ^[0-9]+$ test as alphanumeric, producing a wrong result. The current order gets it right.

assert_gt "1.0.0-rc.2+build-1" "1.0.0-rc.1+build-99"

1.0.0-rc.2+build-1 → 1.0.0-rc.2; 1.0.0-rc.1+build-99 → 1.0.0-rc.1. Cores equal. i=0: rc=rc (continue); i=1: 2 > 1 numeric → return 0. ✓

Implementation looks good overall

• [ "$1" = "$2" ] && return 1 fast-path covers the equal-string case before any stripping — handles assert_le "1.2.3" "1.2.3" correctly.
• ${a1[i]-} (default to empty on out-of-bounds) is the right idiom for the identifier-exhaustion check; the subsequent [ -z "$id1" ] && return 1 / [ -z "$id2" ] && return 0 correctly implements §11.4.4.
• local LC_ALL=C scoped to the function, so the ASCII ordering guarantee on [[ "$id1" > "$id2" ]] doesn't leak into the caller's environment.

25 passed, 0 failed. LGTM.