From 4b605a5d39429674e4daa5c5d828c43f14649920 Mon Sep 17 00:00:00 2001 From: Ilyes512 Date: Thu, 16 Jul 2026 19:00:42 +0200 Subject: [PATCH] Ignore -dirty marker when checking local template status --- README.md | 3 +- .../docs/architecture/template-engine.md | 9 ++- docs/content/docs/commands/template.md | 5 +- internal/util/git/check_local_test.go | 69 +++++++++++++++++++ internal/util/git/git.go | 16 ++++- 5 files changed, 96 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index aff6d78..73453d7 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,8 @@ specs template save ./my-template my-template - **Remote templates** (from `download`) are checked against their git remote. - **Local templates** (from `save`) are checked against their **source directory on disk** — `update available` means the source path has moved ahead of what was saved (`source missing` - if that path is gone). + if that path is gone). Uncommitted changes on the saved commit (a "dirty" working tree) are not + treated as an update, so a dirty source is not reported as perpetually out of date. `specs template upgrade [name]` applies available updates: remote templates are re-cloned, local templates are re-copied from their source path. Cached statuses refresh automatically once older diff --git a/docs/content/docs/architecture/template-engine.md b/docs/content/docs/architecture/template-engine.md index 946c8f8..937ff34 100644 --- a/docs/content/docs/architecture/template-engine.md +++ b/docs/content/docs/architecture/template-engine.md @@ -537,9 +537,12 @@ The source of truth depends on how the template was registered: - **Local templates** (`Repository` is `local:`, from `specs template save`) are checked against the **source directory on disk** via `CheckLocalSource` — never a git remote. The template is behind when the source path's current `git describe` (commit + version) differs - from the commit/version recorded in metadata at save time. When the source path no longer - exists (or is no longer a git repository) the status is `source-missing`. A local template - saved from a non-git directory has no recorded commit and is not tracked. + from the commit/version recorded in metadata at save time. The trailing `-dirty` marker is + ignored while the source stays on the saved commit: an uncommitted (dirty) working tree flips + that suffix on the git-describe version even though `HEAD` has not moved, so comparing it + verbatim would report a phantom update to the same commit (issue #97). When the source path no + longer exists (or is no longer a git repository) the status is `source-missing`. A local + template saved from a non-git directory has no recorded commit and is not tracked. ### How a newer version is determined (remote templates) diff --git a/docs/content/docs/commands/template.md b/docs/content/docs/commands/template.md index 08e5487..89d9432 100644 --- a/docs/content/docs/commands/template.md +++ b/docs/content/docs/commands/template.md @@ -36,7 +36,10 @@ The `Status` column reflects where each template's "source of truth" lives: sitting on a released tag the same semver rule applies, otherwise the branch tip is compared. - **Local templates** (registered with `save`) are compared against the **source directory on disk**, not a git remote. `update available` means the source path has moved ahead of the - commit/version recorded when the template was saved. If the source path no longer exists the + commit/version recorded when the template was saved. Uncommitted changes in the source (a + "dirty" working tree) do **not** count as an update on their own: while the source stays on the + saved commit, the transient `-dirty` marker on its git-describe version is ignored so the + template is not perpetually reported as out of date. If the source path no longer exists the status is `source missing`. Possible `Status` values: `up-to-date`, `update: `, `update available`, diff --git a/internal/util/git/check_local_test.go b/internal/util/git/check_local_test.go index 503e198..cacef6c 100644 --- a/internal/util/git/check_local_test.go +++ b/internal/util/git/check_local_test.go @@ -1,6 +1,7 @@ package git_test import ( + "os" "path/filepath" "testing" @@ -73,6 +74,74 @@ func TestCheckLocalSource_SourceAdvanced(t *testing.T) { } } +// A clean source saved earlier that later becomes dirty (uncommitted changes on +// the same commit) must not be reported as an update — the "-dirty" marker is +// transient working-tree state, not a new commit (issue #97). +func TestCheckLocalSource_DirtyAfterSaveIsUpToDate(t *testing.T) { + dir, repo := initRepo(t) + hash := addCommit(t, repo, dir, "init") + tagCommit(t, repo, "v1.0.0", hash, false) + + saved, err := pkggit.Describe(dir) + if err != nil { + t.Fatalf("Describe (saved): %v", err) + } + + // Working tree becomes dirty after the template was saved. + if err := os.WriteFile(filepath.Join(dir, "init.txt"), []byte("modified"), 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + dirtyDesc, err := pkggit.Describe(dir) + if err != nil { + t.Fatalf("Describe (dirty): %v", err) + } + if dirtyDesc.Version == saved.Version { + t.Fatalf("precondition: dirty version %q should differ from saved %q", dirtyDesc.Version, saved.Version) + } + + got := pkggit.CheckLocalSource(dir, saved.Commit, saved.Version) + if got.ErrorKind != pkggit.CheckErrorNone { + t.Fatalf("ErrorKind = %q, want none", got.ErrorKind) + } + if !got.IsUpToDate { + t.Errorf("IsUpToDate = false, want true when only the working tree turned dirty on the saved commit") + } + if got.LatestVersion != "" { + t.Errorf("LatestVersion = %q, want empty when up-to-date", got.LatestVersion) + } +} + +// A template saved while its source was dirty must not be reported as an update +// once the source becomes clean again at the same commit — the reverse of the +// scenario above (issue #97). +func TestCheckLocalSource_SavedDirtyNowCleanIsUpToDate(t *testing.T) { + dir, repo := initRepo(t) + hash := addCommit(t, repo, dir, "init") + tagCommit(t, repo, "v1.0.0", hash, false) + + // Make the tree dirty and capture the version recorded at save time. + if err := os.WriteFile(filepath.Join(dir, "init.txt"), []byte("modified"), 0644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + savedDirty, err := pkggit.Describe(dir) + if err != nil { + t.Fatalf("Describe (dirty): %v", err) + } + + // Source becomes clean again on the same commit. + if err := os.WriteFile(filepath.Join(dir, "init.txt"), []byte("init"), 0644); err != nil { + t.Fatalf("WriteFile (restore): %v", err) + } + + got := pkggit.CheckLocalSource(dir, savedDirty.Commit, savedDirty.Version) + if got.ErrorKind != pkggit.CheckErrorNone { + t.Fatalf("ErrorKind = %q, want none", got.ErrorKind) + } + if !got.IsUpToDate { + t.Errorf("IsUpToDate = false, want true when the source returned clean on the saved commit") + } +} + func TestCheckLocalSource_ErrIsSourceMissing(t *testing.T) { got := pkggit.CheckLocalSource(filepath.Join(t.TempDir(), "nope"), "abc", "v1.0.0") if err := got.Err(); err != pkggit.ErrCheckSourceMissing { diff --git a/internal/util/git/git.go b/internal/util/git/git.go index 433be09..f8b5095 100644 --- a/internal/util/git/git.go +++ b/internal/util/git/git.go @@ -434,6 +434,10 @@ func CheckRemote(dir, url, branch string) RemoteCheckResult { // // - path missing or not a git repository → ErrorKind CheckErrorSourceMissing. // - describe matches the saved commit and version → up-to-date. +// - source still on the saved commit, differing only by the "-dirty" marker → +// up-to-date. A dirty working tree flips the "-dirty" suffix on git-describe +// output even though HEAD has not moved, so comparing it verbatim would report +// a phantom update to the very same commit (issue #97). // - otherwise → not up-to-date, with LatestVersion set to the path's current version. func CheckLocalSource(sourcePath, savedCommit, savedVersion string) (result RemoteCheckResult) { slog.Debug("git check-local start", "source", sourcePath, "saved_commit", savedCommit, "saved_version", savedVersion) @@ -455,12 +459,22 @@ func CheckLocalSource(sourcePath, savedCommit, savedVersion string) (result Remo return RemoteCheckResult{ErrorKind: CheckErrorSourceMissing} } - if desc.Commit == savedCommit && desc.Version == savedVersion { + // When the source still sits on the saved commit, any version-string difference + // can only be the "-dirty" marker (same commit ⇒ same tag, distance and short + // hash). Uncommitted changes are transient working-tree state, not a new commit + // to update to, so treat this as up-to-date rather than a phantom update. + if desc.Commit == savedCommit && stripDirty(desc.Version) == stripDirty(savedVersion) { return RemoteCheckResult{IsUpToDate: true} } return RemoteCheckResult{IsUpToDate: false, LatestVersion: desc.Version} } +// stripDirty removes the trailing "-dirty" marker that Describe appends for a +// working tree with uncommitted changes. +func stripDirty(version string) string { + return strings.TrimSuffix(version, "-dirty") +} + // classifyRemoteError maps a remote.List error to a CheckErrorKind. func classifyRemoteError(err error) CheckErrorKind { var netErr *net.OpError