diff --git a/README.md b/README.md index cd90435..1ab3616 100644 --- a/README.md +++ b/README.md @@ -341,7 +341,9 @@ git may already have updated the checkout. It never deletes a lock itself. If the branch being deleted is the one you're on, sync switches you to its trunk first, and like `git switch` your uncommitted changes come along. It only keeps the branch when git says the -switch would overwrite them, and then names the files. +switch would overwrite them, and then names the files. A worktree whose directory you deleted +by hand (rather than with `git worktree remove`) is forgotten at the start of the sync, so its +branches can be cleaned up like any other; a locked one is left alone with a notice. Flags: - `-d` / `--delete-all` delete merged or closed branches without asking diff --git a/docs/architecture.md b/docs/architecture.md index c18fe53..e6e9293 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -272,7 +272,11 @@ dropped then, so it heals over two syncs. A candidate checked out in the current trunk is checked out in another worktree leaves HEAD detached at the trunk tip rather than failing. Moving the current checkout off a branch being deleted is a plain `git switch`, so uncommitted changes git can carry come along; only git's "would be overwritten" refusal keeps -the branch (as `dirty`, files named); any other failure stops the sync as before. Consent for deletion is `stack.sync.prune` (`always` by default), `-d`, or `-f`; +the branch (as `dirty`, files named); any other failure stops the sync as before. Before any of +this, sync runs `git worktree prune` when `git worktree list` marks an entry prunable (its +directory was deleted by hand): otherwise git still counts its branch as checked out there and +the branch could never be deleted. Locked worktrees are never pruned, matching git; one whose +directory is gone gets a notice to unlock it. Consent for deletion is `stack.sync.prune` (`always` by default), `-d`, or `-f`; with no terminal and `ask`, branches are kept with a notice. Nothing is pushed; `submit` does that. diff --git a/pkg/app/sync.go b/pkg/app/sync.go index 044216a..e7432d7 100644 --- a/pkg/app/sync.go +++ b/pkg/app/sync.go @@ -193,6 +193,9 @@ func (a *App) Sync(ctx context.Context, repo git.Repo, o SyncOptions) (SyncResul return res, err } res.Remote = st.remote + if err := a.syncPruneWorktrees(ctx, st, &res); err != nil { + return res, err + } // Listing pull requests is a forge round trip that can take longer than // the fetch itself on a busy repository, so it runs alongside the fetch @@ -290,7 +293,7 @@ func (a *App) gatherSync(ctx context.Context, repo git.Repo) (*syncState, error) if err != nil { return nil, err } - st := &syncState{repo: repo, graph: graph, dirty: newDirtyCache(), local: map[string]git.Branch{}} + st := &syncState{repo: repo, graph: graph, dirty: newDirtyCache()} st.trunks = graph.Trunks if len(st.trunks) == 0 { def, ok, err := a.d.Git.DefaultBranch(ctx, repo) @@ -304,17 +307,25 @@ func (a *App) gatherSync(ctx context.Context, repo git.Repo) (*syncState, error) st.trunks = []string{def} } st.remote = a.d.Git.RemoteFor(ctx, repo, st.trunks[0]) - branches, err := a.d.Git.Branches(ctx, repo) - if err != nil { + if err := a.loadCheckouts(ctx, st); err != nil { return nil, err } + return st, nil +} + +// loadCheckouts reads every local branch, with its tip and where it is +// checked out, and the worktrees. +func (a *App) loadCheckouts(ctx context.Context, st *syncState) error { + branches, err := a.d.Git.Branches(ctx, st.repo) + if err != nil { + return err + } + st.local = make(map[string]git.Branch, len(branches)) for _, b := range branches { st.local[b.Name] = b } - if st.worktrees, err = a.d.Git.Worktrees(ctx, repo); err != nil { - return nil, err - } - return st, nil + st.worktrees, err = a.d.Git.Worktrees(ctx, st.repo) + return err } // moveBranch moves a local branch from one tip to another. A branch nobody diff --git a/pkg/app/sync_cleanup_test.go b/pkg/app/sync_cleanup_test.go index 33c44f7..1ad7d7e 100644 --- a/pkg/app/sync_cleanup_test.go +++ b/pkg/app/sync_cleanup_test.go @@ -2,6 +2,7 @@ package app_test import ( "context" + "os" "path/filepath" "slices" "strings" @@ -416,6 +417,52 @@ func TestSyncKeepsDeletedBranchWhenSwitchWouldOverwriteChanges(t *testing.T) { } } +func TestSyncPrunesWorktreeWhoseDirectoryIsGone(t *testing.T) { + f := newSyncFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "main") + wt := filepath.Join(t.TempDir(), "wt") + gittest.Run(t, f.dir, "worktree", "add", "-q", wt, "a") + if err := os.RemoveAll(wt); err != nil { + t.Fatal(err) + } + f.mergeOnRemote(t, "a", 0) + res, err := f.sync(t, app.SyncOptions{NoRestack: true}) + if err != nil { + t.Fatal(err) + } + if _, ok := deleted(res, "a"); !ok || branchExists(t, f.dir, "a") { + t.Errorf("a should be deleted once its dead worktree is pruned: deleted = %+v kept = %+v", res.Deleted, res.Kept) + } + if n := len(strings.Split(gittest.Run(t, f.dir, "worktree", "list"), "\n")); n != 1 { + t.Errorf("%d worktrees left, want just the main one", n) + } + if !strings.Contains(strings.Join(res.Notices, "\n"), "its directory is gone") { + t.Errorf("notices = %v", res.Notices) + } +} + +func TestSyncLeavesLockedWorktreeWhoseDirectoryIsGone(t *testing.T) { + f := newSyncFixture(t) + gittest.Run(t, f.dir, "switch", "-q", "main") + wt := filepath.Join(t.TempDir(), "wt") + gittest.Run(t, f.dir, "worktree", "add", "-q", wt, "a") + gittest.Run(t, f.dir, "worktree", "lock", wt) + if err := os.RemoveAll(wt); err != nil { + t.Fatal(err) + } + f.mergeOnRemote(t, "a", 0) + res, err := f.sync(t, app.SyncOptions{NoRestack: true}) + if err != nil { + t.Fatal(err) + } + if k, ok := kept(res, "a"); !ok || k.Reason != app.KeptOtherWorktree || !branchExists(t, f.dir, "a") { + t.Errorf("kept = %+v", res.Kept) + } + if !strings.Contains(strings.Join(res.Notices, "\n"), "git worktree unlock") { + t.Errorf("notices = %v", res.Notices) + } +} + func TestSyncDropsGoneBranchesFromMetadata(t *testing.T) { f := newSyncFixture(t) f.deps.Config.SyncPrune = config.SyncPruneNever diff --git a/pkg/app/sync_worktrees.go b/pkg/app/sync_worktrees.go new file mode 100644 index 0000000..b4f6dcb --- /dev/null +++ b/pkg/app/sync_worktrees.go @@ -0,0 +1,43 @@ +package app + +import ( + "context" + "errors" + "io/fs" + "os" +) + +// syncPruneWorktrees forgets worktrees whose directory was deleted without +// git worktree remove. git still counts their branches as checked out +// there, so until they are pruned sync would keep those branches and say to +// sync from a directory that no longer exists. A locked worktree is never +// pruned (that's what the lock is for, e.g. a checkout on a drive that isn't +// mounted), so it only gets a notice. +func (a *App) syncPruneWorktrees(ctx context.Context, st *syncState, res *SyncResult) error { + var gone []string + for _, wt := range st.worktrees { + switch { + case wt.Prunable && !wt.Locked: + gone = append(gone, wt.Path) + case wt.Locked && missing(wt.Path): + res.notice("the worktree at %s is locked, so git keeps it although its directory is gone; run git worktree unlock %s and sync again to clean it up", + shortPath(wt.Path), wt.Path) + } + } + if len(gone) == 0 { + return nil + } + if err := a.d.Git.PruneWorktrees(ctx, st.repo); err != nil { + return err + } + for _, p := range gone { + res.notice("forgot the worktree at %s because its directory is gone", shortPath(p)) + } + return a.loadCheckouts(ctx, st) +} + +// missing reports whether nothing exists at path. +func missing(path string) bool { + _, err := os.Stat(path) + return errors.Is(err, fs.ErrNotExist) +} diff --git a/pkg/git/branch.go b/pkg/git/branch.go index 60ed213..3640c1c 100644 --- a/pkg/git/branch.go +++ b/pkg/git/branch.go @@ -137,6 +137,19 @@ func (c *Client) SwitchDetached(ctx context.Context, repo Repo, rev string) erro return nil } +// PruneWorktrees forgets every worktree whose directory is gone, the way +// git worktree prune does. Locked ones are left alone. +func (c *Client) PruneWorktrees(ctx context.Context, repo Repo) error { + _, err := c.gitIn(ctx, repo, "worktree", "prune") + if err != nil { + if ee, ok := errors.AsType[*exec.ExitError](err); ok { + return fmt.Errorf("worktree prune: %s", ee.Result.Err()) + } + return err + } + return nil +} + // Worktree is one entry of `git worktree list`. type Worktree struct { // Path is the working tree root. @@ -146,6 +159,11 @@ type Worktree struct { Branch string Detached bool Bare bool + // Prunable is set when git would prune the entry, usually because its + // directory has been deleted. + Prunable bool + // Locked is set when the entry is locked against pruning. + Locked bool } // Worktrees lists the main checkout and every linked worktree. @@ -180,6 +198,10 @@ func (c *Client) Worktrees(ctx context.Context, repo Repo) ([]Worktree, error) { cur.Detached = true case line == "bare": cur.Bare = true + case line == "prunable" || strings.HasPrefix(line, "prunable "): + cur.Prunable = true + case line == "locked" || strings.HasPrefix(line, "locked "): + cur.Locked = true } } flush() diff --git a/pkg/git/worktree_test.go b/pkg/git/worktree_test.go index d5fbbc4..1cd443d 100644 --- a/pkg/git/worktree_test.go +++ b/pkg/git/worktree_test.go @@ -3,6 +3,7 @@ package git_test import ( "context" "errors" + "os" "path/filepath" "testing" @@ -42,6 +43,47 @@ func TestWorktrees(t *testing.T) { } } +func TestWorktreesMarksMissingDirectoriesPrunable(t *testing.T) { + gittest.Isolate(t) + dir := gittest.InitRepo(t) + c := newClient() + ctx := context.Background() + repo, err := c.Discover(ctx, dir) + if err != nil { + t.Fatal(err) + } + gone := filepath.Join(t.TempDir(), "gone") + gittest.Run(t, dir, "worktree", "add", "-q", "-b", "gone", gone) + locked := filepath.Join(t.TempDir(), "locked") + gittest.Run(t, dir, "worktree", "add", "-q", "-b", "locked", locked) + gittest.Run(t, dir, "worktree", "lock", "--reason", "on a usb stick", locked) + for _, p := range []string{gone, locked} { + if err := os.RemoveAll(p); err != nil { + t.Fatal(err) + } + } + + wts, err := c.Worktrees(ctx, repo) + if err != nil { + t.Fatal(err) + } + // git never offers a locked worktree for pruning, gone or not. + if len(wts) != 3 || wts[0].Prunable || !wts[1].Prunable || wts[1].Locked || wts[2].Prunable || !wts[2].Locked { + t.Fatalf("worktrees = %+v", wts) + } + + if err := c.PruneWorktrees(ctx, repo); err != nil { + t.Fatal(err) + } + wts, err = c.Worktrees(ctx, repo) + if err != nil { + t.Fatal(err) + } + if len(wts) != 2 || wts[1].Branch != "locked" { + t.Errorf("after prune = %+v; want the locked one kept", wts) + } +} + func TestSwitchReportsFilesInTheWay(t *testing.T) { gittest.Isolate(t) dir := gittest.InitRepo(t)