Restore keeps staged pins; offer the committed pin for a staged one

Staged pins survive a restore at every depth and are the targets of the submodules below. For a submodule with a staged pin, x asks whether to restore to the staged or the committed pin, the latter unstaging it. Staged pins of a repository that moves count as a loss. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-22 12:42:20 +02:00
parent 7980d3356a
commit b9f6055759
10 changed files with 359 additions and 72 deletions
@@ -28,8 +28,8 @@ type gitlink struct{ path, sha string }

 // RestoreLoss reports what Restore(path) would discard. It walks the
 // repositories on disk, not the loaded tree.
-func (p *Provider) RestoreLoss(ctx context.Context, path string) (submodule.Loss, error) {
-	dir, target, err := p.restoreTarget(ctx, path)
+func (p *Provider) RestoreLoss(ctx context.Context, path string, to submodule.RestoreTarget) (submodule.Loss, error) {
+	dir, target, err := p.restoreTarget(ctx, path, to)
 	if err != nil {
 		return submodule.Loss{}, err
 	}
@@ -40,9 +40,10 @@ func (p *Provider) RestoreLoss(ctx context.Context, path string) (submodule.Loss
 }

 // Restore makes path ("" is the superproject) and all submodules below it
-// exactly what git records, see docs/actions.md. A missing submodule is
-// cloned and missing commits are fetched; git never prompts.
-func (p *Provider) Restore(ctx context.Context, path string) error {
+// exactly what git records, see docs/actions.md. Staged pins are kept. With
+// RestoreCommitted, a submodule's own staged pin is unstaged first. A missing
+// submodule is cloned and missing commits are fetched; git never prompts.
+func (p *Provider) Restore(ctx context.Context, path string, to submodule.RestoreTarget) error {
 	if path == "" {
 		return p.restoreRepo(ctx, p.root, "HEAD")
 	}
@@ -52,10 +53,15 @@ func (p *Provider) Restore(ctx context.Context, path string) error {
 	if !ok {
 		return fmt.Errorf("unknown submodule %q", path)
 	}
-	pin, err := indexGitlink(ctx, in)
+	pin, err := pinOf(ctx, in, to)
 	if err != nil {
 		return err
 	}
+	if to == submodule.RestoreCommitted {
+		if _, err := p.exec(ctx, in.superDir, nil, "update-index", "--cacheinfo", "160000,"+pin+","+in.relPath); err != nil {
+			return err
+		}
+	}
 	if !initialized(in.dir) {
 		if err := p.initSubmodules(ctx, in.superDir, []string{in.relPath}); err != nil {
 			return fmt.Errorf("%s: %w", path, err)
@@ -65,9 +71,9 @@ func (p *Provider) Restore(ctx context.Context, path string) error {
 }

 // restoreTarget returns the directory and the commit path is restored to:
-// HEAD for the superproject, else the gitlink in its parent's index. The
-// directory is "" if the submodule is not initialized.
-func (p *Provider) restoreTarget(ctx context.Context, path string) (dir, target string, err error) {
+// HEAD for the superproject, else the pin to. The directory is "" if the
+// submodule is not initialized.
+func (p *Provider) restoreTarget(ctx context.Context, path string, to submodule.RestoreTarget) (dir, target string, err error) {
 	if path == "" {
 		return p.root, "HEAD", nil
 	}
@@ -77,7 +83,7 @@ func (p *Provider) restoreTarget(ctx context.Context, path string) (dir, target
 	if !ok {
 		return "", "", fmt.Errorf("unknown submodule %q", path)
 	}
-	pin, err := indexGitlink(ctx, in)
+	pin, err := pinOf(ctx, in, to)
 	if err != nil {
 		return "", "", err
 	}
@@ -87,7 +93,19 @@ func (p *Provider) restoreTarget(ctx context.Context, path string) (dir, target
 	return in.dir, pin, nil
 }

-func indexGitlink(ctx context.Context, in info) (string, error) {
+// pinOf returns the gitlink of a submodule in its parent's index (staged)
+// or HEAD (committed).
+func pinOf(ctx context.Context, in info, to submodule.RestoreTarget) (string, error) {
+	if to == submodule.RestoreCommitted {
+		out, _, err := try(ctx, in.superDir, "ls-tree", "HEAD", "--", in.relPath)
+		if err != nil {
+			return "", err
+		}
+		if pin := parseGitlink(out); pin != "" {
+			return pin, nil
+		}
+		return "", errors.New("has no committed pin (added, but not committed yet)")
+	}
 	out, err := run(ctx, in.superDir, "ls-files", "-s", "--", in.relPath)
 	if err != nil {
 		return "", err
@@ -102,8 +120,9 @@ func indexGitlink(ctx context.Context, in info) (string, error) {
 // --- restore -----------------------------------------------------------------

 // restoreRepo restores the initialized repository at dir to target (a commit
-// or "HEAD") and then every submodule that target records. Branches are never
-// moved: a checkout that is not on target is detached first.
+// or "HEAD") and then every submodule its index records. Branches are never
+// moved: a checkout that is not on target is detached first. If the checkout
+// stays on target, its staged pins are kept and are the submodules' targets.
 func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error {
 	env := p.networkEnv()
 	g := func(args ...string) (string, error) {
@@ -119,14 +138,22 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error {
 	if gitDir, _, err := gitDirs(dir); err == nil {
 		p.clearOperations(ctx, dir, gitDir)
 	}
+	var keep []gitlink
 	if head, err := g("rev-parse", "--verify", "-q", "HEAD"); err != nil || head != sha {
 		if _, err := g("checkout", "--force", "--detach", sha); err != nil {
 			return err
 		}
+	} else if keep, err = stagedPins(ctx, dir); err != nil {
+		return err
 	}
 	if _, err := g("reset", "--hard", "-q"); err != nil {
 		return err
 	}
+	for _, l := range keep {
+		if _, err := g("update-index", "--cacheinfo", "160000,"+l.sha+","+l.path); err != nil {
+			return err
+		}
+	}
 	// -ff also removes untracked repositories; ignored files stay
 	if _, err := g("clean", "-ffdq"); err != nil {
 		return err
@@ -135,7 +162,7 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error {
 		return err
 	}

-	links, err := treeGitlinks(ctx, dir, sha)
+	links, err := indexGitlinks(ctx, dir)
 	if err != nil {
 		return err
 	}
@@ -276,6 +303,31 @@ func indexGitlinks(ctx context.Context, dir string) ([]gitlink, error) {
 	return links, nil
 }

+// stagedPins lists the gitlinks of the index of dir that differ from HEAD's,
+// i.e. staged pins of submodules that HEAD has. A submodule that was added
+// but not committed is not one: its .gitmodules entry is not kept either.
+func stagedPins(ctx context.Context, dir string) ([]gitlink, error) {
+	index, err := indexGitlinks(ctx, dir)
+	if err != nil {
+		return nil, err
+	}
+	tree, err := treeGitlinks(ctx, dir, "HEAD")
+	if err != nil {
+		return nil, err
+	}
+	committed := map[string]string{}
+	for _, l := range tree {
+		committed[l.path] = l.sha
+	}
+	var staged []gitlink
+	for _, l := range index {
+		if c, ok := committed[l.path]; ok && c != l.sha {
+			staged = append(staged, l)
+		}
+	}
+	return staged, nil
+}
+
 // --- loss --------------------------------------------------------------------

 // lossOf adds up what a restore of the repository at dir to target
@@ -341,22 +393,38 @@ func (p *Provider) lossOf(ctx context.Context, dir, target string) submodule.Los
 					unknown()
 				}
 			}
-		}

-		if children, err = indexGitlinks(ctx, dir); err != nil {
-			unknown()
-		}
-		if target != "" {
-			if links, err := treeGitlinks(ctx, dir, target); err == nil {
+			// staged pins are kept, unless the checkout moves away from
+			// the commit they were staged on; then the submodules follow
+			// the target's tree
+			staged, err := stagedPins(ctx, dir)
+			if err != nil {
+				unknown()
+			}
+			var links []gitlink
+			switch {
+			case !moves:
+				links, err = indexGitlinks(ctx, dir)
+			case target != "":
+				own.StagedPins = len(staged)
+				links, err = treeGitlinks(ctx, dir, target)
+			default:
+				own.StagedPins = len(staged)
+			}
+			if err == nil && links != nil {
 				targets = map[string]string{}
 				for _, l := range links {
 					targets[l.path] = l.sha
 				}
 			}
 		}
+
+		if children, err = indexGitlinks(ctx, dir); err != nil {
+			unknown()
+		}
 	}()

-	if own.Files+own.Operations+own.Commits+own.Repositories+own.Unknown > 0 {
+	if own.Files+own.Operations+own.Commits+own.Repositories+own.StagedPins+own.Unknown > 0 {
 		own.Modules = 1
 	}

@@ -24,12 +24,16 @@ func out(t *testing.T, dir string, args ...string) string {
 func exists(path string) bool { _, err := os.Stat(path); return err == nil }

 // assertExact checks that the superproject at dir and every submodule below
-// it is initialized, on its pin and has no change at all.
+// it is initialized, on its pin (staged ones included) and has no change
+// besides staged pins.
 func assertExact(t *testing.T, dir string) {
 	t.Helper()
-	if s := out(t, dir, "status", "--porcelain"); s != "" {
+	if s := out(t, dir, "diff", "--name-only", "--ignore-submodules=dirty"); s != "" { // worktree vs index
 		t.Errorf("%s not clean:\n%s", dir, s)
 	}
+	if s := out(t, dir, "ls-files", "--others", "--exclude-standard"); s != "" {
+		t.Errorf("%s has untracked files:\n%s", dir, s)
+	}
 	raw, err := exec.Command("git", "-C", dir, "submodule", "status", "--recursive").Output() // the prefix is a leading space
 	if err != nil {
 		t.Fatal(err)
@@ -43,9 +47,13 @@ func assertExact(t *testing.T, dir string) {
 			continue
 		}
 		f := strings.Fields(line)
-		if s := out(t, filepath.Join(dir, f[1]), "status", "--porcelain"); s != "" {
+		sub := filepath.Join(dir, f[1])
+		if s := out(t, sub, "diff", "--name-only", "--ignore-submodules=dirty"); s != "" {
 			t.Errorf("%s not clean:\n%s", f[1], s)
 		}
+		if s := out(t, sub, "ls-files", "--others", "--exclude-standard"); s != "" {
+			t.Errorf("%s has untracked files:\n%s", f[1], s)
+		}
 	}
 }

@@ -72,7 +80,7 @@ func TestRestoreSuperprojectMakesEverythingExact(t *testing.T) {
 	conflict := filepath.Join(super, "libs", "conflict")
 	mainTip := out(t, conflict, "rev-parse", "main")

-	if err := p.Restore(ctx, ""); err != nil {
+	if err := p.Restore(ctx, "", submodule.RestoreStaged); err != nil {
 		t.Fatal(err)
 	}
 	assertExact(t, super)
@@ -88,8 +96,9 @@ func TestRestoreSuperprojectMakesEverythingExact(t *testing.T) {
 	if !exists(filepath.Join(super, "libs", "missing", ".git")) {
 		t.Error("libs/missing was not initialized")
 	}
-	if out(t, super, "diff", "--cached", "--name-only") != "" {
-		t.Error("staged pin survived a superproject restore")
+	// the fixture stages libs/staged and libs/conflict
+	if got := out(t, super, "diff", "--cached", "--name-only"); got != "libs/conflict\nlibs/staged" {
+		t.Errorf("staged = %q, want the staged pins kept", got)
 	}
 }

@@ -100,7 +109,7 @@ func TestRestoreSubmoduleKeepsStagedPinAsTarget(t *testing.T) {
 	// the fixture staged the checked-out commit; move the checkout away
 	git(t, filepath.Join(super, "libs", "staged"), "checkout", "-q", "--detach", "HEAD~1")

-	if err := p.Restore(context.Background(), "libs/staged"); err != nil {
+	if err := p.Restore(context.Background(), "libs/staged", submodule.RestoreStaged); err != nil {
 		t.Fatal(err)
 	}
 	if got := out(t, filepath.Join(super, "libs", "staged"), "rev-parse", "HEAD"); got != staged {
@@ -118,7 +127,7 @@ func TestRestoreLoss(t *testing.T) {

 	loss := func(path string) submodule.Loss {
 		t.Helper()
-		l, err := p.RestoreLoss(ctx, path)
+		l, err := p.RestoreLoss(ctx, path, submodule.RestoreStaged)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -191,7 +200,7 @@ func TestRestoreRemovesLeftoversAndEndsRebase(t *testing.T) {
 		t.Fatal("setup: no rebase in progress")
 	}

-	if err := p.Restore(ctx, ""); err != nil {
+	if err := p.Restore(ctx, "", submodule.RestoreStaged); err != nil {
 		t.Fatal(err)
 	}
 	for _, gone := range []string{"stray", "libs/topic"} {
@@ -205,9 +214,7 @@ func TestRestoreRemovesLeftoversAndEndsRebase(t *testing.T) {
 	if got := out(t, v, "rev-parse", "y"); got != yTip {
 		t.Errorf("branch y moved: %s -> %s", yTip, got)
 	}
-	if out(t, super, "status", "--porcelain") != "" {
-		t.Errorf("superproject not clean:\n%s", out(t, super, "status", "--porcelain"))
-	}
+	assertExact(t, super)
 }

 func TestRestoreClonesAMissingSubmodule(t *testing.T) {
@@ -219,7 +226,7 @@ func TestRestoreClonesAMissingSubmodule(t *testing.T) {
 	if err := os.RemoveAll(filepath.Join(super, ".git", "modules", "libs", "missing")); err != nil {
 		t.Fatal(err)
 	}
-	if err := p.Restore(context.Background(), "libs/missing"); err != nil {
+	if err := p.Restore(context.Background(), "libs/missing", submodule.RestoreStaged); err != nil {
 		t.Fatal(err)
 	}
 	if !exists(filepath.Join(super, "libs", "missing", ".git")) {
@@ -238,7 +245,7 @@ func TestRestoreReportsAnUnavailablePin(t *testing.T) {
 	bogus := "0123456789abcdef0123456789abcdef01234567"
 	git(t, super, "update-index", "--cacheinfo", "160000,"+bogus+",src/core")
 	_ = core
-	err := p.Restore(context.Background(), "src/core")
+	err := p.Restore(context.Background(), "src/core", submodule.RestoreStaged)
 	if err == nil || !strings.Contains(err.Error(), "not available") {
 		t.Fatalf("err = %v, want a message about the unavailable commit", err)
 	}
@@ -251,7 +258,7 @@ func TestRestoreIgnoresUpdateNoneAndInactive(t *testing.T) {
 	git(t, super, "config", "submodule.libs/missing.active", "false")
 	git(t, super, "config", "submodule.active", "src")

-	if err := p.Restore(context.Background(), ""); err != nil {
+	if err := p.Restore(context.Background(), "", submodule.RestoreStaged); err != nil {
 		t.Fatal(err)
 	}
 	if !exists(filepath.Join(super, "libs", "missing", ".git")) {
@@ -259,3 +266,69 @@ func TestRestoreIgnoresUpdateNoneAndInactive(t *testing.T) {
 	}
 	assertExact(t, super)
 }
+
+func TestRestoreKeepsNestedStagedPinAndResetsToIt(t *testing.T) {
+	p, root := ownFixture(t)
+	super := filepath.Join(root, "super")
+	log := filepath.Join(super, "libs", "log")
+	nested := filepath.Join(log, "modules", "nested")
+
+	// stage a new pin of nested in libs/log, then change nested again
+	git(t, nested, "commit", "-q", "-am", "Nested work")
+	staged := out(t, nested, "rev-parse", "HEAD")
+	git(t, log, "add", "modules/nested")
+	os.WriteFile(filepath.Join(nested, "README"), []byte("more\n"), 0o644)
+
+	if l, err := p.RestoreLoss(context.Background(), "", submodule.RestoreStaged); err != nil || l.StagedPins != 0 || l.Files == 0 {
+		t.Fatalf("loss %+v, %v: want the change, not the staged pin", l, err)
+	}
+	if err := p.Restore(context.Background(), "", submodule.RestoreStaged); err != nil {
+		t.Fatal(err)
+	}
+	if got := out(t, log, "diff", "--cached", "--name-only"); got != "modules/nested" {
+		t.Errorf("libs/log staged = %q, want the nested pin", got)
+	}
+	if got := out(t, nested, "rev-parse", "HEAD"); got != staged {
+		t.Errorf("nested HEAD = %s, want the staged pin %s", got, staged)
+	}
+	assertExact(t, super)
+}
+
+func TestRestoreCommittedUnstagesThePin(t *testing.T) {
+	p, root := ownFixture(t)
+	super := filepath.Join(root, "super")
+	committed := strings.Fields(out(t, super, "ls-tree", "HEAD", "libs/staged"))[2]
+
+	if err := p.Restore(context.Background(), "libs/staged", submodule.RestoreCommitted); err != nil {
+		t.Fatal(err)
+	}
+	if got := out(t, super, "diff", "--cached", "--name-only"); strings.Contains(got, "libs/staged") {
+		t.Errorf("still staged: %q", got)
+	}
+	if got := out(t, filepath.Join(super, "libs", "staged"), "rev-parse", "HEAD"); got != committed {
+		t.Errorf("HEAD = %s, want the committed pin %s", got, committed)
+	}
+}
+
+func TestRestoreLossCountsStagedPinsOfAMovingRepository(t *testing.T) {
+	p, root := ownFixture(t)
+	super := filepath.Join(root, "super")
+	log := filepath.Join(super, "libs", "log")
+	nested := filepath.Join(log, "modules", "nested")
+
+	// libs/log moves on (off pin, on its branch) and stages a nested pin
+	git(t, nested, "commit", "-q", "-am", "Nested work")
+	git(t, log, "add", "modules/nested")
+	git(t, log, "commit", "-q", "--allow-empty", "-m", "Log work")
+	git(t, nested, "commit", "-q", "--allow-empty", "-m", "More nested work")
+	git(t, log, "add", "modules/nested")
+
+	l, err := p.RestoreLoss(context.Background(), "libs/log", submodule.RestoreStaged)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// the nested commits are on a detached HEAD that moves back: lost too
+	if l.StagedPins != 1 || l.Commits != 2 {
+		t.Errorf("loss %+v, want 1 staged pin and the 2 detached nested commits", l)
+	}
+}