fix(git-submodules): a bump that would drop a nested checkout's work refuses (plugin-task#54)

candy/plugin-task/nested_work_test.go

@@ -0,0 +1,140 @@
+package task
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/opencharly/spec/spec"
+)
+
+// nestedWorkFixture builds the tree opencharly/plugin-task#54 is about: an umbrella
+// that pins `outer` at O1, `outer` that pins its own `inner` at L1, and BOTH levels
+// initialized — the shape `git clone --recurse-submodules` produces.
+//
+// `outer`'s main is at O2, the commit that records `inner` at L2, so the bump ROLLS
+// outer and the reconcile then moves the nested checkout from L1 to L2. The nested
+// checkout is given L2 locally, the way a real nested remote would serve it.
+//
+// It returns the umbrella, the nested checkout's directory, and the SHAs the
+// assertions speak about.
+func nestedWorkFixture(t *testing.T) (umb, innerCo, o1, l1, l2 string) {
+	t.Helper()
+	base, leafBare, _, L1, L2 := bumpOrigins(t)
+
+	outer := filepath.Join(base, "outer-src")
+	gitIn(t, base, "init", "-q", "-b", "main", "outer-src")
+	writeFile(t, filepath.Join(outer, "o"), "o")
+	gitIn(t, outer, "add", "-A")
+	gitIn(t, outer, "commit", "-qm", "o")
+	gitIn(t, outer, "-c", "protocol.file.allow=always", "submodule", "add", "-q", leafBare, "inner")
+	gitIn(t, outer, "config", "-f", ".gitmodules", "submodule.inner.branch", "main")
+	gitIn(t, outer, "add", ".gitmodules")
+	gitIn(t, outer, "commit", "-qm", "inner=main")
+	gitIn(t, outer, "update-index", "--add", "--cacheinfo", "160000,"+L1+",inner")
+	gitIn(t, outer, "commit", "-qm", "inner=L1")
+	o1 = gitIn(t, outer, "rev-parse", "HEAD")
+	// Advance inner to L2 inside outer: that is the commit the bump rolls outer to.
+	gitIn(t, outer, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "inner")
+	gitIn(t, filepath.Join(outer, "inner"), "fetch", "-q", "origin")
+	gitIn(t, filepath.Join(outer, "inner"), "switch", "--quiet", "--detach", L2)
+	gitIn(t, outer, "add", "inner")
+	gitIn(t, outer, "commit", "-qm", "inner=L2")
+	outerBare := filepath.Join(base, "outer.git")
+	gitIn(t, base, "clone", "-q", "--bare", outer, outerBare)
+
+	umb = filepath.Join(base, "umb-nested-work")
+	gitIn(t, base, "init", "-q", "-b", "main", "umb-nested-work")
+	writeFile(t, filepath.Join(umb, "u"), "u")
+	gitIn(t, umb, "add", "-A")
+	gitIn(t, umb, "commit", "-qm", "u")
+	gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "add", "-q", outerBare, "outer")
+	gitIn(t, umb, "config", "-f", ".gitmodules", "submodule.outer.branch", "main")
+	gitIn(t, umb, "add", ".gitmodules")
+	gitIn(t, umb, "commit", "-qm", "branch=main")
+	gitIn(t, umb, "update-index", "--add", "--cacheinfo", "160000,"+o1+",outer")
+	gitIn(t, umb, "commit", "-qm", "stale pin")
+	gitIn(t, umb, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "outer")
+	outerCo := filepath.Join(umb, "outer")
+	gitIn(t, outerCo, "-c", "protocol.file.allow=always", "submodule", "update", "--init", "inner")
+	innerCo = filepath.Join(outerCo, "inner")
+	gitIn(t, innerCo, "-c", "protocol.file.allow=always", "fetch", "-q", "origin", L2)
+	return umb, innerCo, o1, L1, L2
+}
+
+// TestGitSubmodules_BumpRefusesToDropNestedWork is opencharly/plugin-task#54's gate.
+//
+// THE DEFECT: the reconcile moves the nested checkouts with `git submodule update
+// --recursive`, and that update takes a nested checkout off the commit it is at — even
+// when that commit carries work NOTHING ELSE REFERENCES — with exit 0 and nothing
+// reported. The operator's commit survives only in the reflog. MEASURED against git
+// 2.56: the same update refuses a DIRTY nested checkout loudly, so the loss is not
+// git's doing but the update's blind spot.
+//
+// THE ASSERTION: the bump REFUSES, it NAMES the nested checkout, and it leaves the
+// tree exactly as it was — the nested checkout still holds the operator's commit, the
+// superproject's pin was NOT staged, and the submodule's own checkout was not switched.
+// A bump that refuses is the only outcome that loses nothing.
+//
+// Before the fix this FAILS: the bump PASSES and the nested checkout is moved off the
+// operator's commit, with nothing reported.
+func TestGitSubmodules_BumpRefusesToDropNestedWork(t *testing.T) {
+	umb, innerCo, o1, _, _ := nestedWorkFixture(t)
+
+	// The operator's work: a commit in the nested checkout that NO ref references —
+	// the shape the update drops.
+	writeFile(t, filepath.Join(innerCo, "work"), "work")
+	gitIn(t, innerCo, "add", "-A")
+	gitIn(t, innerCo, "commit", "-qm", "local work")
+	work := gitIn(t, innerCo, "rev-parse", "HEAD")
+	if out := gitIn(t, innerCo, "for-each-ref", "--contains", work, "--format=%(refname)"); out != "" {
+		t.Fatalf("fixture: the local commit must be referenced by no ref, got %q", out)
+	}
+
+	st, msg := runMaintenanceVerbIn(umb, "git-submodules", map[string]any{"mode": "bump"})
+	if st != spec.StatusFail {
+		t.Fatalf("the bump must REFUSE a nested checkout whose commit the update would drop, got %s: %s", st, msg)
+	}
+	if !strings.Contains(msg, "outer/inner") {
+		t.Fatalf("the refusal must NAME the nested checkout, got: %s", msg)
+	}
+	if got := gitIn(t, innerCo, "rev-parse", "HEAD"); got != work {
+		t.Fatalf("the nested checkout must still hold the operator's commit %s, got %s", work, got)
+	}
+	// The tree is untouched: the pin was not staged and the submodule was not switched.
+	if got := gitlink(umb, "outer"); got != o1 {
+		t.Fatalf("the refusal must leave the superproject's pin at %s, got %s", o1, got)
+	}
+	if got := gitIn(t, filepath.Join(umb, "outer"), "rev-parse", "HEAD"); got != o1 {
+		t.Fatalf("the refusal must leave the submodule's checkout at %s, got %s", o1, got)
+	}
+}
+
+// TestGitSubmodules_BumpKeepsNestedWorkThatABranchHolds is the DISCRIMINATING half of
+// the gate: the same work, on a BRANCH, is not lost by the update — the branch still
+// holds the commit after the checkout moved — so the bump must go through.
+//
+// Without this half the gate would also pass a verb that refuses every nested
+// checkout, which is not a fix but a freeze (R7: a test that cannot fail is invalid).
+func TestGitSubmodules_BumpKeepsNestedWorkThatABranchHolds(t *testing.T) {
+	umb, innerCo, _, _, L2 := nestedWorkFixture(t)
+
+	writeFile(t, filepath.Join(innerCo, "work"), "work")
+	gitIn(t, innerCo, "add", "-A")
+	gitIn(t, innerCo, "commit", "-qm", "local work")
+	work := gitIn(t, innerCo, "rev-parse", "HEAD")
+	gitIn(t, innerCo, "branch", "keep")
+
+	st, msg := runMaintenanceVerbIn(umb, "git-submodules", map[string]any{"mode": "bump"})
+	if st != spec.StatusPass {
+		t.Fatalf("the bump must go through when a branch holds the local commit, got %s: %s", st, msg)
+	}
+	// The work survived the move.
+	if got := gitIn(t, innerCo, "rev-parse", "keep"); got != work {
+		t.Fatalf("the branch must still hold the operator's commit %s, got %s", work, got)
+	}
+	// And the nested checkout followed the pin the new commit records.
+	if got := gitIn(t, innerCo, "rev-parse", "HEAD"); got != L2 {
+		t.Fatalf("the nested checkout must be at the recorded pin %s, got %s", L2, got)
+	}
+}

candy/plugin-task/prune.go

@@ -279,7 +279,7 @@ func branchMerged(repoDir, branch, tip, base string, prs map[string][]prInfo, gh
 // strictly smaller, never larger).
 func pruneRepos(projDir string) []string {
 	repos := []string{projDir}
-	subs, err := submodulePaths(projDir)
+	subs, err := gitmodulesPaths(projDir)
 	if err != nil {
 		return repos
 	}