diff --git a/internal/git/git.go b/internal/git/git.go index 26fca36..397f10f 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -5,10 +5,27 @@ import ( "errors" "fmt" "os/exec" + "regexp" "strings" "time" ) +// versionTagRegex matches cascade's canonical version tags: vX.Y.Z, optionally +// with an -rc.N prerelease and a nested .hotfix.M segment. It is kept in lockstep +// with the parser in internal/version (see version.Parse). The git package cannot +// import internal/version directly because that package depends, transitively +// through internal/changelog, on this one; TestIsValidVersionTag_InSyncWithVersionParse +// asserts the two stay in agreement. +var versionTagRegex = regexp.MustCompile(`^([a-zA-Z]*)(\d+)\.(\d+)\.(\d+)(?:-rc\.(\d+)(?:\.hotfix\.(\d+))?)?$`) + +// IsValidVersionTag reports whether tag is a well-formed cascade version tag. +// Tags that do not match (for example a vX.Y.Z-dryrun.N exercise tag, a foreign +// "nightly" or "latest" tag, or a typo) are invisible to version discovery so +// they can never be mistaken for the latest released or prereleased version. +func IsValidVersionTag(tag string) bool { + return versionTagRegex.MatchString(tag) +} + // GetChangedFiles returns the list of files changed between two commits func GetChangedFiles(baseSHA, headSHA string) ([]string, error) { // Handle null SHA (new branch or first commit) @@ -144,21 +161,25 @@ func GetLatestTag(prefix string) (string, string, error) { } tags := parseLines(output) - if len(tags) == 0 { - return "", "", nil - } - // First tag is the latest (sorted descending) - latestTag := tags[0] + // The prefix glob can still match non-version tags (for example a + // vX.Y.Z-dryrun.N exercise tag or a "vnightly" alias). Skip anything that is + // not a canonical cascade version so it can never be read as the latest one. + for _, tag := range tags { + if !IsValidVersionTag(tag) { + continue + } - // Get the SHA for this tag - cmd = exec.Command("git", "rev-list", "-n", "1", latestTag) - output, err = cmd.Output() - if err != nil { - return latestTag, "", fmt.Errorf("git rev-list for tag: %w", err) + // First valid tag is the latest (git sorted descending by version). + cmd = exec.Command("git", "rev-list", "-n", "1", tag) + output, err = cmd.Output() + if err != nil { + return tag, "", fmt.Errorf("git rev-list for tag: %w", err) + } + return tag, strings.TrimSpace(string(output)), nil } - return latestTag, strings.TrimSpace(string(output)), nil + return "", "", nil } // ListTags returns every tag in the repository. It returns an empty slice when @@ -384,8 +405,14 @@ func GetLatestReleaseTag(prefix string) (string, string, error) { return "", "", nil } - // Find first tag without -rc suffix (published release) + // Find the first published release: a valid cascade version with no -rc + // suffix. Filtering through IsValidVersionTag keeps non-version tags (such as + // a vX.Y.Z-dryrun.N exercise tag, which also lacks an -rc suffix) from being + // mistaken for a release. for _, tag := range tags { + if !IsValidVersionTag(tag) { + continue + } if !strings.Contains(tag, "-rc.") { // Get the SHA for this tag cmd = exec.Command("git", "rev-list", "-n", "1", tag) diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 7331693..38845aa 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -318,3 +318,96 @@ func TestRemoteBranchSHA(t *testing.T) { }) } } + +// tagHead creates a lightweight tag pointing at the current HEAD. +func tagHead(t *testing.T, name string) { + t.Helper() + runGit(t, "tag", name) +} + +func TestGetLatestTag_IgnoresNonVersionTags(t *testing.T) { + newScratchRepo(t) + commitFile(t, "a.txt", "one", "first commit") + + // Valid version tags plus non-version tags that sort newer by base version. + tagHead(t, "v0.5.0") + tagHead(t, "v0.5.1") + tagHead(t, "v0.6.0-dryrun.1") // higher base version, not a cascade version + tagHead(t, "vnightly") // foreign tag matching the prefix glob + + got, sha, err := GetLatestTag("v") + if err != nil { + t.Fatalf("GetLatestTag() unexpected error: %v", err) + } + if got != "v0.5.1" { + t.Errorf("GetLatestTag() = %q, want %q (must ignore -dryrun and foreign tags)", got, "v0.5.1") + } + if sha == "" { + t.Errorf("GetLatestTag() returned empty SHA for %q", got) + } +} + +func TestGetLatestReleaseTag_IgnoresNonVersionTags(t *testing.T) { + newScratchRepo(t) + commitFile(t, "a.txt", "one", "first commit") + + tagHead(t, "v0.5.0") + tagHead(t, "v0.5.1") + tagHead(t, "v0.6.0-dryrun.1") // not an -rc tag, but also not a valid version + tagHead(t, "vnightly") + + got, sha, err := GetLatestReleaseTag("v") + if err != nil { + t.Fatalf("GetLatestReleaseTag() unexpected error: %v", err) + } + if got != "v0.5.1" { + t.Errorf("GetLatestReleaseTag() = %q, want %q (must ignore -dryrun and foreign tags)", got, "v0.5.1") + } + if sha == "" { + t.Errorf("GetLatestReleaseTag() returned empty SHA for %q", got) + } +} + +func TestGetLatestReleaseTag_SkipsRCButKeepsValidRelease(t *testing.T) { + newScratchRepo(t) + commitFile(t, "a.txt", "one", "first commit") + + tagHead(t, "v1.0.0") + tagHead(t, "v1.0.1-rc.0") // valid prerelease, must be skipped for "release" + + got, _, err := GetLatestReleaseTag("v") + if err != nil { + t.Fatalf("GetLatestReleaseTag() unexpected error: %v", err) + } + if got != "v1.0.0" { + t.Errorf("GetLatestReleaseTag() = %q, want %q", got, "v1.0.0") + } +} + +func TestIsValidVersionTag(t *testing.T) { + tests := []struct { + tag string + want bool + }{ + {"v1.2.3", true}, + {"v0.5.1", true}, + {"v1.0.1-rc.0", true}, + {"v1.0.1-rc.4.hotfix.5", true}, + {"1.2.3", true}, // empty prefix is allowed + {"release1.2.3", true}, // alphabetic prefix is allowed + {"v0.6.0-dryrun.1", false}, + {"vnightly", false}, + {"nightly", false}, + {"latest", false}, + {"v1.2", false}, + {"v1.2.3-rc", false}, + {"", false}, + } + for _, tt := range tests { + t.Run(tt.tag, func(t *testing.T) { + if got := IsValidVersionTag(tt.tag); got != tt.want { + t.Errorf("IsValidVersionTag(%q) = %v, want %v", tt.tag, got, tt.want) + } + }) + } +} diff --git a/internal/git/version_tag_sync_test.go b/internal/git/version_tag_sync_test.go new file mode 100644 index 0000000..f9a5c7e --- /dev/null +++ b/internal/git/version_tag_sync_test.go @@ -0,0 +1,51 @@ +package git_test + +import ( + "testing" + + "github.com/stablekernel/cascade/internal/git" + "github.com/stablekernel/cascade/internal/version" +) + +// TestIsValidVersionTag_InSyncWithVersionParse guards against the git package's +// local version predicate drifting from the canonical parser in internal/version. +// The git package cannot import internal/version directly (that would create an +// import cycle through internal/changelog), so this external test asserts the two +// agree across a representative corpus of tag strings. +func TestIsValidVersionTag_InSyncWithVersionParse(t *testing.T) { + corpus := []string{ + "v1.2.3", + "v0.5.1", + "v0.0.0", + "v10.20.30", + "v1.0.1-rc.0", + "v1.0.1-rc.42", + "v1.0.1-rc.4.hotfix.5", + "1.2.3", + "release1.2.3", + "v0.6.0-dryrun.1", + "v0.6.0-dryrun.10", + "vnightly", + "nightly", + "latest", + "v1.2", + "v1.2.3.4", + "v1.2.3-rc", + "v1.2.3-rc.x", + "v1.2.3-hotfix.1", + "v-1.2.3", + "", + "vlatest", + } + + for _, tag := range corpus { + tag := tag + t.Run(tag, func(t *testing.T) { + _, err := version.Parse(tag) + wantValid := err == nil + if got := git.IsValidVersionTag(tag); got != wantValid { + t.Errorf("IsValidVersionTag(%q) = %v, but version.Parse success = %v (predicate drifted from canonical regex)", tag, got, wantValid) + } + }) + } +} diff --git a/internal/orchestrate/nochange_skip_test.go b/internal/orchestrate/nochange_skip_test.go index f5d606e..2a2c5b8 100644 --- a/internal/orchestrate/nochange_skip_test.go +++ b/internal/orchestrate/nochange_skip_test.go @@ -148,6 +148,49 @@ func TestSetup_NewCommit_RunsBuild(t *testing.T) { } } +// TestSetup_DryrunTagPresent_DoesNotBreakVersionCalc reproduces the nightly +// dry-run failure: a vX.Y.Z-dryrun.N exercise tag is cut against the repo and +// sorts newer (by base version) than the real releases. Version discovery must +// ignore it so the no-environment version calculation continues from the latest +// valid release instead of choking on an unparseable tag. +func TestSetup_DryrunTagPresent_DoesNotBreakVersionCalc(t *testing.T) { + repoDir, headSHA := initRepo(t) + manifestPath := writeManifest(t, repoDir, headSHA) + + // Real releases plus a higher-sorting dry-run exercise tag and a foreign tag. + runGit(t, repoDir, "tag", "v0.5.0") + runGit(t, repoDir, "tag", "v0.5.1") + runGit(t, repoDir, "tag", "v0.6.0-dryrun.1") + runGit(t, repoDir, "tag", "vnightly") + + // The git package reads tags from the process working directory, so run from + // inside the fixture repo to exercise its tag set (mirrors how the original + // failure surfaced when a -dryrun tag lived in the checked-out repo). + orig, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + if err := os.Chdir(repoDir); err != nil { + t.Fatalf("chdir repo: %v", err) + } + t.Cleanup(func() { + if err := os.Chdir(orig); err != nil { + t.Fatalf("restore cwd: %v", err) + } + }) + + orch, err := NewOrchestrator(manifestPath, "ci", "prerelease") + if err != nil { + t.Fatalf("NewOrchestrator: %v", err) + } + + // Before the fix this failed with "calculating version: invalid version + // format: v0.6.0-dryrun.1". + if _, err := orch.Setup(headSHA); err != nil { + t.Fatalf("Setup with a -dryrun tag present must not fail version calc: %v", err) + } +} + // TestFinalize_RecordsPerBuildSHA verifies that a successful build's SHA is // recorded into envState.Builds so the build base ladder can consult it on the // next dispatch.