diff --git a/docs/actions.md b/docs/actions.md index 4e5fdeb..5c7ee15 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -86,7 +86,8 @@ After a successful restore of repository R with target T: any depth. So the subtree shows `in sync` everywhere, except for states restore does -not change: `unpushed`, behind/ahead of the ref, and `detached HEAD on pin`. +not change: `unpushed`, behind/ahead of the ref, and `detached HEAD on pin` +where no branch is at the pin (see [Branches](#branches)). The recursion follows **T**, not the tree loaded in the UI. A submodule that is only in T (for example, after switching branches) is initialized. A @@ -100,8 +101,25 @@ submodule that is only on disk is deleted as an untracked directory. the branch stays checked out. - **Submodule off the pin:** HEAD is detached at the target. The branch that was checked out is **not moved**, so its commits stay on it. +- **Submodule detached at the target** (after the step above, or because it + was already so, also when it was in sync): HEAD is put on a branch whose + tip is the target, the first that fits of ([D28](decisions.md)): + 1. a local branch tracking the ref the UI shows (or named like it), e.g. + local `main` tracking `origin/main`, also when the pin is behind or + ahead of the ref; + 2. the ref itself, if its tip is the target and no local branch has its + name: the local branch is created, tracking it; + 3. any other local branch, the first alphabetically; + 4. any other remote branch whose name no local branch has, created as a + tracking branch; the ref's remote first, then alphabetically. -Restore never moves, creates or deletes a branch. + If git refuses one (checked out in another worktree, a name conflict), + the next is tried. If none fits, HEAD stays detached. + +Restore never moves or deletes a branch. A local branch that is behind or +ahead of its remote branch is no candidate and is not fast-forwarded. The +only branches restore creates are tracking branches for remote branches at +the target, in steps 2 and 4. ### What can be lost @@ -187,6 +205,14 @@ submodule update --init --checkout --force -- `--checkout` overrides `submodule..update`, and the explicit paths override `submodule..active`, so no child module is skipped. +For a submodule, after `submodule sync`, `attachBranch` puts a detached HEAD +on a branch: `symbolic-ref -q HEAD` (on a branch: done), `for-each-ref +--points-at=T refs/heads refs/remotes` (no branch at T: done), then the ref +is resolved like `Status` does, and the first candidate (see +[Branches](#branches)) that `switch` / `switch -c --track ` +accepts wins. The branch checked out before the restore is passed to the +ref resolution. A child's `.gitmodules` branch is read once per parent. + With `c` the parent's index gets the committed pin first (`update-index --cacheinfo` in the parent). @@ -261,7 +287,9 @@ restore or switch runs at a time. `git.Provider.Switch` (`internal/submodule/git/switch.go`): verify the ref is still `T`; `update-index --cacheinfo` in the parent; `restoreRepo` (see -restore); then `switch` / `switch -c --track `. +restore); then `switch` / `switch -c --track `. The restore leaves HEAD of the +switched repository alone (Switch puts it on the chosen branch); the +submodules below it are attached as in a restore. `SwitchLoss` is `lossOf` with `T` as the target. `Branches` is one `for-each-ref`, without the `origin/HEAD` symref. @@ -337,7 +365,12 @@ action, hence `x`. not moved, ignored files survive, a merge and a rebase in progress end, a standalone clone and a removed submodule's directory are deleted, a missing submodule is cloned again, `update = none` and inactive submodules are - restored, an unavailable commit is reported. The loss is empty for off pin, + restored, an unavailable commit is reported. A detached submodule ends on + the local branch tracking the ref, else on the ref created as a tracking + branch (before other branches at the pin), else on the first local + branch; a local branch elsewhere is not moved; a checked-out branch stays; + a branch of another worktree is skipped; nested submodules are attached + too. The loss is empty for off pin, missing and unpushed submodules, counts files, nested changes, merges, detached commits (not those on a branch) and standalone repositories. - UI (`internal/tui/restore_test.go`): restores at once without a loss, asks diff --git a/docs/decisions.md b/docs/decisions.md index fc812d4..6d8d7dd 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -327,7 +327,8 @@ the same for every submodule of the target tree, initializing missing ones. - *Ignored files stay* (`clean` without `-x`): `.env`, `node_modules` and IDE settings are not "additional files" worth deleting in every submodule. - *Branches never move.* A checkout that is not on the target is detached - first, so commits stay on their branch. + first, so commits stay on their branch. (A submodule is then put on a + branch at the target if there is one, see D28.) - *It asks only if something is lost* (changed or untracked files, operations in progress, commits on no ref, standalone repositories). Never asking would delete repositories that exist nowhere else; always @@ -499,3 +500,33 @@ timeout, failures are silent), which supersedes the once-a-day rule of D21. N1 still holds: the TUI never waits for it. `q` changes meaning while an update is available, so muscle memory that exits with `q` now updates; the bar says so. Typing `q` in the filter or a popup is unchanged. + +## D28 — Restore puts a submodule on a branch at its pin + +**Context.** After a restore (for example after the superproject's branch +changed) every submodule was a detached HEAD, so IDEs showed only hashes. +**Decision.** Amends D23. After restoring a submodule, at any depth, a +detached HEAD is put on a branch whose tip is the pin, in this order: (1) a +local branch tracking the shown ref (or named like it), (2) the ref itself +as a new local tracking branch, if no local branch has its name, (3) any +other local branch, alphabetically, (4) any other remote branch whose name +no local branch has, as a new tracking branch, the ref's remote first. A +HEAD already on a branch stays there. If git refuses a branch (checked out +in another worktree, a name conflict), the next one is tried; if none fits, +HEAD stays detached and the restore still succeeds. Submodules that were in +sync are included. The superproject is not affected. +**Rationale.** Attaching a branch whose tip is the pin changes no commit and +no file, so nothing can be lost and the loss question stays the same. The +ref comes first because it is what the UI shows; a local branch beats a new +one so restore adds as few branches as possible. +**Alternatives.** Fast-forwarding a local branch that is behind its remote +branch at the pin: rejected, restore still never moves a branch. Staying +detached when several branches fit: rejected, the order above always gives +one answer, the same on every run. +**Consequences.** Restore now may create local branches (only for remote +branches at the pin whose name has none). Per detached submodule it costs a +`symbolic-ref` and one `for-each-ref --points-at`; only if a branch is at +the pin, the ref is resolved (as for `Status`) and one `switch` runs. No +network. `detached HEAD on pin` is now left only where no branch is at the +pin. Switch keeps its own rule for the repository it switches; the +submodules below it follow this one. diff --git a/docs/requirements.md b/docs/requirements.md index eb9d662..29168f2 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -145,7 +145,8 @@ commit hashes. It doesn't answer the questions users actually have: the header is selected): its worktree becomes exactly what git records, and so do all submodules below it, recursively. Staged pins are kept at every depth. For a submodule with a staged pin it asks whether to restore to the - staged or the committed pin (the latter unstages it). It asks for + staged or the committed pin (the latter unstages it). A restored submodule + whose pin is a branch tip ends on that branch, not detached. It asks for confirmation only if something would be lost. See [actions.md](actions.md#restore). - **F57** `b` opens a popup in the middle of the screen with a search line diff --git a/internal/submodule/git/provider.go b/internal/submodule/git/provider.go index 9766efa..fa7d266 100644 --- a/internal/submodule/git/provider.go +++ b/internal/submodule/git/provider.go @@ -327,17 +327,10 @@ func pinState(ctx context.Context, in info, st *submodule.Status, pin, head, bra return fmt.Errorf("pinned commit %.7s not fetched", pin) } - // remote branches containing the pin, oldest tip first - out, err := run(ctx, in.dir, "for-each-ref", "--contains", pin, "--sort=committerdate", "--format=%(refname)", "refs/remotes") + containing, err := remoteContaining(ctx, in.dir, pin) if err != nil { return err } - var containing []string - for _, r := range strings.Fields(out) { - if !strings.HasSuffix(r, "/HEAD") { - containing = append(containing, strings.TrimPrefix(r, "refs/remotes/")) - } - } st.Pin.Pushed = len(containing) > 0 var ref string @@ -359,6 +352,7 @@ func pinState(ctx context.Context, in info, st *submodule.Status, pin, head, bra return err } st.CheckedOut = st.Pinned + var out string if head != pin { if st.CheckedOut, err = commit(ctx, in.dir, head); err != nil { return err @@ -417,6 +411,22 @@ func pinState(ctx context.Context, in info, st *submodule.Status, pin, head, bra return nil } +// remoteContaining lists the remote branches containing pin, oldest tip +// first, without the origin/HEAD symref. +func remoteContaining(ctx context.Context, dir, pin string) ([]string, error) { + out, err := run(ctx, dir, "for-each-ref", "--contains", pin, "--sort=committerdate", "--format=%(refname)", "refs/remotes") + if err != nil { + return nil, err + } + var containing []string + for _, r := range strings.Fields(out) { + if !strings.HasSuffix(r, "/HEAD") { + containing = append(containing, strings.TrimPrefix(r, "refs/remotes/")) + } + } + return containing, nil +} + // resolve finds the ref the pin is compared with, following the order in // docs/state-model.md: the target (see targetRef) if the pin is on it, else the // branch containing the pin that is closest to it, else the target anyway, diff --git a/internal/submodule/git/restore.go b/internal/submodule/git/restore.go index cd6e17d..548a3e4 100644 --- a/internal/submodule/git/restore.go +++ b/internal/submodule/git/restore.go @@ -6,7 +6,9 @@ import ( "fmt" "io/fs" "os" + "path" "path/filepath" + "slices" "strings" "sync" "time" @@ -45,7 +47,7 @@ func (p *Provider) RestoreLoss(ctx context.Context, path string, to submodule.Re // 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") + return p.restoreRepo(ctx, p.root, "HEAD", nil) } p.mu.RLock() in, ok := p.mods[path] @@ -67,7 +69,7 @@ func (p *Provider) Restore(ctx context.Context, path string, to submodule.Restor return fmt.Errorf("%s: %w", path, err) } } - return p.restoreRepo(ctx, in.dir, pin) + return p.restoreRepo(ctx, in.dir, pin, &in) } // restoreTarget returns the directory and the commit path is restored to: @@ -123,7 +125,9 @@ func pinOf(ctx context.Context, in info, to submodule.RestoreTarget) (string, er // 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 { +// For a submodule (in is not nil) a detached HEAD is then put on a branch at +// target if there is one, see attachBranch; the submodules below always are. +func (p *Provider) restoreRepo(ctx context.Context, dir, target string, in *info) error { env := p.networkEnv() g := func(args ...string) (string, error) { return p.exec(ctx, dir, env, append(append([]string{}, restoreArgs...), args...)...) @@ -138,6 +142,7 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error { if gitDir, _, err := gitDirs(dir); err == nil { p.clearOperations(ctx, dir, gitDir) } + prev, _, _ := try(ctx, dir, "symbolic-ref", "--short", "-q", "HEAD") // for the ref, see attachBranch 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 { @@ -161,6 +166,11 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error { if _, err := g("submodule", "sync", "-q"); err != nil { return err } + if in != nil { + if err := p.attachBranch(ctx, *in, sha, prev); err != nil { + return err + } + } links, err := indexGitlinks(ctx, dir) if err != nil { @@ -178,6 +188,16 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error { } } + // the .gitmodules branch of each child, for its ref + branches := map[string]string{} + if out, ok, err := try(ctx, dir, "config", "-f", ".gitmodules", "--get-regexp", `^submodule\..*\.(path|branch)$`); err != nil { + return err + } else if ok { + for _, m := range parseGitmodules(out) { + branches[path.Clean(m.path)] = m.branch + } + } + // the children are independent repositories var wg sync.WaitGroup errs := make([]error, len(links)) @@ -185,7 +205,8 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error { wg.Add(1) go func() { defer wg.Done() - if err := p.restoreRepo(ctx, filepath.Join(dir, filepath.FromSlash(l.path)), l.sha); err != nil { + child := info{superDir: dir, dir: filepath.Join(dir, filepath.FromSlash(l.path)), relPath: l.path, branch: branches[l.path]} + if err := p.restoreRepo(ctx, child.dir, l.sha, &child); err != nil { errs[i] = err } }() @@ -194,6 +215,135 @@ func (p *Provider) restoreRepo(ctx context.Context, dir, target string) error { return errors.Join(errs...) } +// attachBranch puts the detached HEAD of the restored submodule in on a +// branch whose tip is sha, the first that git accepts of, in this order: +// +// 1. a local branch tracking the ref the UI shows (see resolve), or named +// like it; +// 2. the ref itself, as a new local branch tracking it, if no local branch +// has its name; +// 3. any other local branch, alphabetically; +// 4. any other remote branch whose name no local branch has, as a new +// tracking branch; the ref's remote first, then alphabetically. +// +// A HEAD that is on a branch stays there. No branch is moved: a local branch +// elsewhere is no candidate, and its remote branch neither. If nothing fits, +// HEAD stays detached; only a context error is returned. prev is the branch +// checked out before the restore; the ref depends on it. +func (p *Provider) attachBranch(ctx context.Context, in info, sha, prev string) error { + if _, onBranch, err := try(ctx, in.dir, "symbolic-ref", "-q", "HEAD"); err != nil || onBranch { + return err + } + out, ok, err := try(ctx, in.dir, "for-each-ref", "--points-at="+sha, + "--format=%(refname)%00%(upstream)%00%(symref)", "refs/heads", "refs/remotes") + if err != nil || !ok { + return err + } + var locals, remotes []string // alphabetical, as for-each-ref sorts them + upstreams := map[string]string{} + for _, line := range strings.Split(out, "\n") { + f := strings.Split(line, "\x00") + if len(f) != 3 || f[2] != "" { // a symref is origin/HEAD + continue + } + if name, ok := strings.CutPrefix(f[0], "refs/heads/"); ok { + locals = append(locals, name) + upstreams[name] = strings.TrimPrefix(f[1], "refs/remotes/") + } else if name, ok := strings.CutPrefix(f[0], "refs/remotes/"); ok { + remotes = append(remotes, name) + } + } + if len(locals)+len(remotes) == 0 { + return nil + } + + // the ref, if it is a remote branch + var ref string + containing, err := remoteContaining(ctx, in.dir, sha) + if err != nil { + return err + } + if r, source, _, err := resolve(ctx, in, sha, prev, containing); err != nil { + return err + } else if source != submodule.RefFromTag && source != submodule.RefFromSHA { + ref = r + } + + localOf := func(remote string) string { _, l, _ := strings.Cut(remote, "/"); return l } + hasLocal := func(name string) (bool, error) { + _, ok, err := try(ctx, in.dir, "rev-parse", "--verify", "-q", "refs/heads/"+name) + return ok, err + } + type candidate struct{ local, remote string } // remote: create local, tracking it + var cands []candidate + addRemote := func(r string) error { + l := localOf(r) + if l == "" { + return nil + } + exists, err := hasLocal(l) + if err == nil && !exists { + cands = append(cands, candidate{l, r}) + } + return err + } + if ref != "" { + for _, l := range locals { + if upstreams[l] == ref { + cands = append(cands, candidate{local: l}) + } + } + if l := localOf(ref); slices.Contains(locals, l) { + cands = append(cands, candidate{local: l}) + } + if slices.Contains(remotes, ref) { + if err := addRemote(ref); err != nil { + return err + } + } + } + for _, l := range locals { + cands = append(cands, candidate{local: l}) + } + preferred, _, _ := strings.Cut(ref, "/") + if preferred == "" { + if preferred, err = remoteName(ctx, in.dir); err != nil { + return err + } + } + slices.SortStableFunc(remotes, func(a, b string) int { + ap, bp := strings.HasPrefix(a, preferred+"/"), strings.HasPrefix(b, preferred+"/") + switch { + case ap && !bp: + return -1 + case bp && !ap: + return 1 + } + return 0 + }) + for _, r := range remotes { + if r != ref { + if err := addRemote(r); err != nil { + return err + } + } + } + + for _, c := range cands { + args := append(append([]string{}, restoreArgs...), "switch", "-q", c.local) + if c.remote != "" { + args = append(append([]string{}, restoreArgs...), "switch", "-q", "-c", c.local, "--track", c.remote) + } + if _, err := p.exec(ctx, in.dir, nil, args...); err == nil { + return nil + } else if ctx.Err() != nil { + return ctx.Err() + } + // e.g. checked out in another worktree: try the next one + } + return nil +} + // networkEnv is the environment of commands that may clone or fetch. func (p *Provider) networkEnv() []string { return append(append([]string{}, p.sshEnv...), p.extraEnv...) diff --git a/internal/submodule/git/restore_test.go b/internal/submodule/git/restore_test.go index 0c84340..e9d6cc3 100644 --- a/internal/submodule/git/restore_test.go +++ b/internal/submodule/git/restore_test.go @@ -332,3 +332,130 @@ func TestRestoreLossCountsStagedPinsOfAMovingRepository(t *testing.T) { t.Errorf("loss %+v, want 1 staged pin and the 2 detached nested commits", l) } } + +// restoreSub restores the submodule path and returns the branch its HEAD is +// on ("" if detached). +func restoreSub(t *testing.T, p *Provider, super, path string) string { + t.Helper() + if err := p.Restore(context.Background(), path, submodule.RestoreStaged); err != nil { + t.Fatal(err) + } + return out(t, filepath.Join(super, filepath.FromSlash(path)), "branch", "--show-current") +} + +func TestRestoreAttachesTheLocalBranchTrackingTheRef(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + // src/ui: in sync, the pin is behind origin/main, local main is at the pin + git(t, filepath.Join(super, "src", "ui"), "checkout", "-q", "--detach") + git(t, filepath.Join(super, "src", "ui"), "branch", "aaa") // alphabetically first, but no ref + + if got := restoreSub(t, p, super, "src/ui"); got != "main" { + t.Errorf("branch = %q, want main", got) + } +} + +func TestRestoreCreatesTheRefBranchBeforeOtherBranches(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + log := filepath.Join(super, "libs", "log") + // libs/log: the pin is the tip of origin/main and origin/archive + git(t, log, "checkout", "-q", "--detach") + git(t, log, "branch", "-D", "main") + + if got := restoreSub(t, p, super, "libs/log"); got != "main" { + t.Errorf("branch = %q, want main", got) + } + if up := out(t, log, "rev-parse", "--abbrev-ref", "main@{upstream}"); up != "origin/main" { + t.Errorf("upstream = %q, want origin/main", up) + } + if out(t, log, "branch", "--list", "archive") != "" { + t.Error("archive must not be created") + } +} + +func TestRestoreCreatesARemoteFeatureBranch(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + topic := filepath.Join(super, "libs", "topic") + mainTip := out(t, topic, "rev-parse", "main") + // libs/topic: detached at the tip of origin/feat/search, local main elsewhere + + if got := restoreSub(t, p, super, "libs/topic"); got != "feat/search" { + t.Errorf("branch = %q, want feat/search", got) + } + if got := out(t, topic, "rev-parse", "main"); got != mainTip { + t.Errorf("main moved: %s -> %s", mainTip, got) + } +} + +func TestRestorePrefersLocalBranchesAlphabetically(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + feature := filepath.Join(super, "libs", "feature") + // libs/feature: the pin is on no branch tip + git(t, feature, "branch", "zz") + git(t, feature, "branch", "aa") + + if got := restoreSub(t, p, super, "libs/feature"); got != "aa" { + t.Errorf("branch = %q, want aa", got) + } +} + +func TestRestoreNeverMovesALocalBranchToAttach(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + topic := filepath.Join(super, "libs", "topic") + git(t, topic, "branch", "feat/search", "origin/main") // same name, elsewhere + before := out(t, topic, "rev-parse", "feat/search") + + if got := restoreSub(t, p, super, "libs/topic"); got != "" { + t.Errorf("branch = %q, want detached", got) + } + if after := out(t, topic, "rev-parse", "feat/search"); after != before { + t.Errorf("feat/search moved: %s -> %s", before, after) + } +} + +func TestRestoreKeepsTheCheckedOutBranch(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + log := filepath.Join(super, "libs", "log") + git(t, log, "switch", "-q", "-c", "zz") // at the pin, like main + + if got := restoreSub(t, p, super, "libs/log"); got != "zz" { + t.Errorf("branch = %q, want zz kept", got) + } +} + +func TestRestoreSkipsABranchOfAnotherWorktree(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + log := filepath.Join(super, "libs", "log") + git(t, log, "checkout", "-q", "--detach") + git(t, log, "worktree", "add", "-q", filepath.Join(root, "wt"), "main") + + if got := restoreSub(t, p, super, "libs/log"); got == "main" { + t.Error("main is checked out in another worktree") + } +} + +func TestRestoreSuperprojectAttachesNestedSubmodules(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + nested := filepath.Join(super, "libs", "log", "modules", "nested") + git(t, nested, "checkout", "-q", "--detach") + + if err := p.Restore(context.Background(), "", submodule.RestoreStaged); err != nil { + t.Fatal(err) + } + if got := out(t, nested, "branch", "--show-current"); got != "main" { + t.Errorf("nested branch = %q, want main", got) + } + if got := out(t, filepath.Join(super, "libs", "topic"), "branch", "--show-current"); got != "feat/search" { + t.Errorf("libs/topic branch = %q, want feat/search", got) + } + if got := out(t, super, "branch", "--show-current"); got != "main" { + t.Errorf("superproject branch = %q, want main", got) + } +} diff --git a/internal/submodule/git/switch.go b/internal/submodule/git/switch.go index c863c23..0621ab3 100644 --- a/internal/submodule/git/switch.go +++ b/internal/submodule/git/switch.go @@ -107,7 +107,7 @@ func (p *Provider) Switch(ctx context.Context, path string, b submodule.Branch) return "", err } } - if err := p.restoreRepo(ctx, dir, b.SHA); err != nil { + if err := p.restoreRepo(ctx, dir, b.SHA, nil); err != nil { // HEAD goes on b below return "", err }