Skip to content

fix: refine stability precedence to support standard prerelease sorting#6

Open
sameerforge wants to merge 1 commit intocarvel-dev:masterfrom
sameerforge:topic/sameerforge/follow-up-semver-fix
Open

fix: refine stability precedence to support standard prerelease sorting#6
sameerforge wants to merge 1 commit intocarvel-dev:masterfrom
sameerforge:topic/sameerforge/follow-up-semver-fix

Conversation

@sameerforge
Copy link
Copy Markdown

@sameerforge sameerforge commented Mar 31, 2026

Refinement of Metadata Precedence Logic
This follow-up PR adjusts the build metadata comparison logic to ensure stability-aware precedence (GA > RC) only applies when comparing numeric segments to alphanumeric ones.

Verified Improvements:
Natural Sorting: vmware.10-fips now correctly ranks higher than vmware.9-fips.

Stability: 3.4.0+v1.33 (Stable) now correctly ranks higher than 3.4.0+v1.33-rc.2.

Compatibility: Standard pre-release ordering (e.g., 0.0.1-pre.1 < 0.0.1-rc.0) is preserved, fixing a regression found in vendir.

Validation :
All 38+ existing unit tests, alongside new edge-case coverage Passed ✅

Current changeset Validation : 
➜  v4 git:(topic/sameerforge/follow-up-semver-fix) go test -v .                                                        
=== RUN   TestJSONMarshal
--- PASS: TestJSONMarshal (0.00s)
=== RUN   TestJSONUnmarshal
--- PASS: TestJSONUnmarshal (0.00s)
=== RUN   TestParseComparator
--- PASS: TestParseComparator (0.00s)
=== RUN   TestSplitAndTrim
--- PASS: TestSplitAndTrim (0.00s)
=== RUN   TestSplitComparatorVersion
--- PASS: TestSplitComparatorVersion (0.00s)
=== RUN   TestBuildVersionRange
--- PASS: TestBuildVersionRange (0.00s)
=== RUN   TestSplitORParts
--- PASS: TestSplitORParts (0.00s)
=== RUN   TestGetWildcardType
--- PASS: TestGetWildcardType (0.00s)
=== RUN   TestCreateVersionFromWildcard
--- PASS: TestCreateVersionFromWildcard (0.00s)
=== RUN   TestIncrementMajorVersion
--- PASS: TestIncrementMajorVersion (0.00s)
=== RUN   TestIncrementMinorVersion
--- PASS: TestIncrementMinorVersion (0.00s)
=== RUN   TestExpandWildcardVersion
--- PASS: TestExpandWildcardVersion (0.00s)
=== RUN   TestVersionRangeToRange
--- PASS: TestVersionRangeToRange (0.00s)
=== RUN   TestRangeAND
--- PASS: TestRangeAND (0.00s)
=== RUN   TestRangeOR
--- PASS: TestRangeOR (0.00s)
=== RUN   TestParseRange
--- PASS: TestParseRange (0.00s)
=== RUN   TestMustParseRange
--- PASS: TestMustParseRange (0.00s)
=== RUN   TestMustParseRange_panic
--- PASS: TestMustParseRange_panic (0.00s)
=== RUN   TestStringer
--- PASS: TestStringer (0.00s)
=== RUN   TestParse
--- PASS: TestParse (0.00s)
=== RUN   TestParseTolerant
--- PASS: TestParseTolerant (0.00s)
=== RUN   TestMustParse
--- PASS: TestMustParse (0.00s)
=== RUN   TestMustParse_panic
--- PASS: TestMustParse_panic (0.00s)
=== RUN   TestValidate
--- PASS: TestValidate (0.00s)
=== RUN   TestFinalizeVersionMethod
--- PASS: TestFinalizeVersionMethod (0.00s)
=== RUN   TestCompare
--- PASS: TestCompare (0.00s)
=== RUN   TestWrongFormat
--- PASS: TestWrongFormat (0.00s)
=== RUN   TestWrongTolerantFormat
--- PASS: TestWrongTolerantFormat (0.00s)
=== RUN   TestCompareHelper
--- PASS: TestCompareHelper (0.00s)
=== RUN   TestIncrements
--- PASS: TestIncrements (0.00s)
=== RUN   TestPreReleaseVersions
--- PASS: TestPreReleaseVersions (0.00s)
=== RUN   TestBuildMetaDataVersions
--- PASS: TestBuildMetaDataVersions (0.00s)
=== RUN   TestNewHelper
--- PASS: TestNewHelper (0.00s)
=== RUN   TestMakeHelper
--- PASS: TestMakeHelper (0.00s)
=== RUN   TestFinalizeVersion
--- PASS: TestFinalizeVersion (0.00s)
=== RUN   TestEdgeCases_NaturalSortAndMetadata
--- PASS: TestEdgeCases_NaturalSortAndMetadata (0.00s)
=== RUN   TestSort
--- PASS: TestSort (0.00s)
=== RUN   TestScanString
--- PASS: TestScanString (0.00s)
PASS
ok      github.com/carvel-dev/semver/v4 0.502s

Validation with Vendir Test :
Tests Passed ✅

➜  vendir git:(develop) ✗ go test -v ./pkg/vendir/versions/                                   

=== RUN   TestSemverOrder
--- PASS: TestSemverOrder (0.00s)
=== RUN   TestSemverFilter
--- PASS: TestSemverFilter (0.00s)
=== RUN   TestSemverWithoutPrereleases
--- PASS: TestSemverWithoutPrereleases (0.00s)
=== RUN   TestSemverWithPrereleases
--- PASS: TestSemverWithPrereleases (0.00s)
=== RUN   TestSemverWithPrereleaseIdentifiers
--- PASS: TestSemverWithPrereleaseIdentifiers (0.00s)
=== RUN   TestSemverWithBuildMetadata
--- PASS: TestSemverWithBuildMetadata (0.00s)
=== RUN   TestSemverWithBuildMetadataAndConstraint
--- PASS: TestSemverWithBuildMetadataAndConstraint (0.00s)
=== RUN   TestHighestVersionWithConstraints
--- PASS: TestHighestVersionWithConstraints (0.00s)
PASS
ok      carvel.dev/vendir/pkg/vendir/versions   (cached)

@sameerforge
Copy link
Copy Markdown
Author

@joaopapereira Please review this follow-up change for #5

1 similar comment
@sameerforge
Copy link
Copy Markdown
Author

@joaopapereira Please review this follow-up change for #5

v4/semver.go Outdated
if strings.Contains(sl, "rc") {
return true
}
return strings.Contains(sl, "-") && (strings.Contains(sl, "alpha") || strings.Contains(sl, "beta"))
Copy link
Copy Markdown

@ramineni ramineni Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. we are only checking for '-' for alpha only, not for beta or rc . We have to check for '-' as pre-requisite for all the pre release labels.

  2. why are we restricting to check only rc , alpha, beta ? there can be other labels (pre etc) which can exist? we can do string comparison to evaluate them?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Logic now generically uses hyphens to detect stability label.

{"0.0.1-pre.1", "0.0.1-rc.0", -1, "Vendir: pre < rc alphabetically"},

// 5. Standard SemVer: Numeric segments are always less than alphanumeric ones
{"1.0.0-alpha.1", "1.0.0-alpha.beta", -1, "Standard: numeric < alphanumeric"},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the comparison for alphanumeric vs alpha numeric .

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected. Updated the description to clarify this compares a numeric segment (1) vs an alphanumeric segment (beta) per SemVer precedence.

{"1.0.0-alpha.1", "1.0.0-alpha.beta", -1, "Standard: numeric < alphanumeric"},

// 6. Stability Check: Ensure alpha/beta without hyphens follow standard rules
{"1.0.0-beta", "1.0.0-rc", -1, "Standard: beta < rc alphabetically"},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this example shows with '-' ?

Copy link
Copy Markdown
Author

@sameerforge sameerforge Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated Description. The separator hyphen is not part of the segment itself

Signed-off-by: Sameer <sameer.khan@broadcom.com>
@sameerforge sameerforge force-pushed the topic/sameerforge/follow-up-semver-fix branch from 1c0319b to ab3899f Compare April 2, 2026 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants