diff --git a/README.md b/README.md index 1f4f043..69e622d 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ git_pruner version # also --version, -v | `tab` | Switch between local branches and [remote-only branches](#remote-only-branches) | | `c` | Checkout the branch under the cursor (`git switch`); on a remote-only row, create a local branch that tracks it | | `/` | Filter branches by name; `enter` keeps it, `esc` clears it | -| `r` | Toggle "also delete remote" for the row (needs an upstream) | +| `r` | Toggle "also delete remote" for the row (needs an upstream on a remote that is not [protected](#protected-branches)) | | `v` | View the branch's diff (through [delta](https://github.com/dandavison/delta) when installed) | | `x` | Select gone branches that hold no unique work (no fetch) | | `p` | Fetch `--all --prune`, then select gone branches that hold no unique work | @@ -102,7 +102,8 @@ precedence, so `y`, `R` and `n` still work while a list is scrolled. - `>` cursor, `[x]` selected, `R` remote deletion armed - `*` current branch and `P` protected are locked; `+` checked out in another worktree (see - [Branches checked out in a worktree](#branches-checked-out-in-a-worktree)) + [Branches checked out in a worktree](#branches-checked-out-in-a-worktree)); `~` held by a + rebase or bisect in progress (see [Branches held by a rebase or bisect](#branches-held-by-a-rebase-or-bisect)) - ahead/behind shown as `↑N↓M` (`=` when in sync, `gone` in red when the upstream was deleted) - a green `✓` after the track column means the upstream is merged into the remote default branch — i.e. the remote is safe to delete @@ -166,8 +167,11 @@ claiming the work is unrecoverable. the confirmation screen flags any commits that would be discarded (see above). When a `-d` delete is refused for being unmerged, a follow-up prompt lets you retry those branches with `-D` without leaving the results — no need to back out and re-select. -- Remote: when armed with `r`, runs `git push --delete `, where the remote is - derived from the branch's upstream. Because this affects shared history, remote deletion +- Remote: when armed with `r`, runs `git push --delete ` on the branch's + upstream, as git records it. That upstream need not share the branch's name: a branch made + with `git switch -c feat origin/main` tracks `main`. So `r` refuses when the upstream is the + default branch or protected, and when the upstream is a local branch (`--track main`), which + has no remote branch to delete. Because this affects shared history, remote deletion requires the explicit `R` key on the confirmation screen — plain `y` deletes locals only. The confirmation screen also shows, per branch, whether the upstream is merged into the remote default (`✓ merged` / `⚠ not merged`) to help you judge whether the remote is safe to delete. @@ -191,6 +195,13 @@ branch. The confirmation screen names each worktree it will remove. This happens when you run git_pruner from a linked worktree. - Script mode removes worktrees the same way. +## Branches held by a rebase or bisect + +git also refuses to delete a branch that a rebase or a bisect started from, in any worktree, +even with `-D`. A `git rebase --update-refs` holds every branch in its stack. git_pruner marks +such a branch with `~`, says `rebase in progress` or `bisect in progress` on its row, and locks +it. Finish or abort the operation (`git rebase --abort`, `git bisect reset`) to free it. + ## Selecting merged, old branches Press `m`, type an age in days (it starts at 90, or at `pruner.staleDays`), and press `enter`. @@ -209,6 +220,11 @@ confirmation screen counts the commits of any branch that is not. Deleting one is a push, so only `R` does it; `y` never touches a remote. The exit summary prints the `git push` command that puts a deleted remote branch back. +Each row is mapped back to its branch on the remote through the remote's fetch refspecs +(`remote..fetch`), so a refspec that renames branches deletes the right one. A row that +maps to something other than one remote branch is not listed: pull request refs fetched with +`+refs/pull/*/head:refs/remotes/origin/pr/*` are not branches a push can delete. + ## Protected branches The default branch is always protected. Add your own with name globs: @@ -219,7 +235,9 @@ git config --add pruner.protect develop ``` A protected branch shows `P` and cannot be selected by any key. Remote rows are matched by the -branch part of their name, so `release/*` covers `origin/release/1.0` too. +branch part of their name, so `release/*` covers `origin/release/1.0` too. The same check +applies to the remote branch a local branch tracks: `r` cannot arm the delete of +`origin/release/1.0` from a local `hotfix` made from it. ## Script mode @@ -242,6 +260,11 @@ stop after 60 seconds. A remote that asks for a password or does not answer fail instead of freezing the screen. Use a credential helper or an ssh agent for remotes that need a login. +In a partial clone (`git clone --filter=blob:none`), git fetches file contents when it needs +them, so `git cherry` (the commit count) and `git diff` (`v`) can also reach the remote. They get +the same limits there. A commit count that fails is shown as unknown, never as zero, so `x` and +`p` leave that branch unselected. + ## Settings The sort field and order are saved in `~/Library/Application Support/git_pruner/settings` diff --git a/bugs.md b/bugs.md index 604ac8d..383cd8b 100644 --- a/bugs.md +++ b/bugs.md @@ -1,2 +1,8 @@ ### October 5 - [x] Git worktrees checked out to other locations are not able to be cleaned up + +### October 9 +- [x] Branches held by a rebase, bisect or `rebase --update-refs` showed as free, and their delete failed +- [x] `r` could delete the wrong remote branch: a protected or default upstream, or `origin`'s branch for a local upstream +- [x] Remote-only rows ignored fetch refspecs and could delete a teammate's branch of the same short name +- [x] Partial clones: `git cherry` failing offline counted as zero risky commits, so `x` selected unmerged gone branches diff --git a/docs/test-coverage-gaps.md b/docs/test-coverage-gaps.md index d50d499..000af8a 100644 --- a/docs/test-coverage-gaps.md +++ b/docs/test-coverage-gaps.md @@ -176,6 +176,66 @@ Combined with a multibyte branch name, pinned end to end through parse, render, --- +## Tier 4 — git state outside the branch list — **DONE 2026-10-09** + +The linked-worktree bug (`bugs.md`, October 5) had one cause: git keeps state outside +`refs/heads` that changes what a delete does, and the tool did not read it. This sweep looked +for more of that shape. Every claim was reproduced in a throwaway repo first. + +### 15. Rebase, bisect and `rebase --update-refs` hold branches *(confirmed — fixed)* + +git refuses `branch -d` and `-D` on a branch a rebase or bisect started from, in any worktree, +and on every branch a `rebase --update-refs` will move. The worktree's HEAD is detached, so +`%(HEAD)` and `%(worktreepath)` are both empty and the tool showed the branch as free. Fixed by +`loadHeldBranches`, which reads each worktree's `rebase-merge/head-name`, +`rebase-apply/head-name`, `rebase-merge/update-refs` and `BISECT_START`, following git's +`prepare_checked_out_branches`. Held branches show `~` and are locked. +*(`TestHeldBranchesAreLocked`)* + +### 16. `r` armed the delete of the wrong remote branch *(confirmed — fixed)* + +`r` targets the upstream, which need not share the branch's name. Reproduced: + +- `hotfix` made from `origin/release/1.0` with `pruner.protect 'release/*'`: the push deleted + `release/1.0`. The protect check only looked at the local name. +- `git switch -c feat origin/main`: the push tried to delete `main`. Only the remote's own + `receive.denyDeleteCurrent` stopped it. +- `--track main` (remote `.`): `remoteName` fell back to `origin`, so the push tried to delete + `origin`'s `main`, a remote the branch never used. + +Fixed by `remoteProtected` (trunk and protect checks on `remoteBranch()`) and +`remoteDeletable()`, which also refuses a local upstream. *(`TestRemoteDeleteRefusesWrongTargets`)* + +### 17. Remote rows ignored fetch refspecs *(confirmed — fixed)* + +With `+refs/heads/jb/*:refs/remotes/origin/*`, the row `origin/old` is `jb/old` on the remote, +but the delete pushed `refs/heads/old` and removed a teammate's branch. `mapRemoteRows` now maps +each row back through `remote..fetch`. Rows that map to no branch, or to more than one +source (the usual pull request refspec next to the default one), are dropped. Every remote's +trunk is now protected in the remote view, not only the default remote's. +*(`TestRemoteRowsFollowFetchRefspecs`)* + +### 18. Partial clones reach the network from `cherry` and `diff` *(confirmed — fixed)* + +In a `--filter=blob:none` clone, `git cherry` fetches blobs when two commits touch the same +files, and `git diff` fetches them for the `v` view. `runGit` treated both as local: no +timeout and no ssh prompt guard. Worse, with the remote unreachable `cherry` exits 128, and +`riskCommitCountRef` read that as **zero** commits at risk, so `x` selected a gone branch whose +unique commit `-D` then discarded. Fixed by `lazyFetch` (set from `remote.*.promisor` or +`extensions.partialClone`), which makes both commands network-bound, and by `riskUnknown`, +which `x`, `p` and the confirm screens treat as risky. *(`TestPartialCloneRiskIsNotGuessed`)* + +`git switch` (`c`) also fetches in a partial clone, but stays local: the 60 s timeout would +kill a checkout partway and leave `index.lock` behind. + +### Checked, no change needed + +- Worktree folder deleted by hand: `git worktree remove --force` still succeeds. +- Bare repo with worktrees: its HEAD branch shows `*` and is locked, though git would delete + it. The safe side. +- `git am` in progress: HEAD stays on the branch, so it already shows as current. +- Locked worktree: the remove is refused as designed. + ## Ref handling — the rule this work established **Never use git's `%(refname:short)`, `%(upstream:short)`, or `symbolic-ref --short`.** They diff --git a/git.go b/git.go index 218ac4f..2a06d0f 100644 --- a/git.go +++ b/git.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "regexp" "sort" "strconv" @@ -34,15 +35,19 @@ type branch struct { gone bool // upstream was configured but no longer exists remoteMerged bool // upstream is merged into the remote default branch (safe to delete) headMerged bool // branch tip is merged into HEAD (git's -d criterion when there is no upstream) - riskCommits int // commits whose patch is not in the base branch; -D discards them + riskCommits int // commits whose patch is not in the base branch; -D discards them (riskUnknown: not countable) riskMeasured bool // riskCommits has been computed (0 is a meaningful value) isCurrent bool worktree string // path of another worktree that has the branch checked out; "" when none does mainWorktree bool // that worktree is the main one, which git cannot remove - protected bool // matches the default branch or a pruner.protect pattern - remoteOnly bool // a remote branch that no local branch tracks - selected bool - deleteRemote bool + // heldBy names the operation in progress that holds the branch, "rebase" or + // "bisect"; "" when none does. git refuses to delete a held branch. + heldBy string + protected bool // matches the default branch or a pruner.protect pattern + remoteProtected bool // its remote branch is the trunk or matches a pruner.protect pattern + remoteOnly bool // a remote branch that no local branch tracks + selected bool + deleteRemote bool } // remoteArmed reports whether deleting b includes its remote branch. Selecting @@ -52,7 +57,28 @@ func (b branch) remoteArmed() bool { return (b.deleteRemote || b.remoteOnly) && // locked reports whether b can never be marked for deletion. A branch in a // linked worktree is not: the delete removes the worktree first. The main // worktree cannot be removed, so its branch stays locked. -func (b branch) locked() bool { return b.isCurrent || b.mainWorktree || b.protected } +func (b branch) locked() bool { return b.isCurrent || b.mainWorktree || b.heldBy != "" || b.protected } + +// remoteDeletable reports whether r may arm the delete of b's remote branch. An +// upstream that is a local branch (remote ".") has no remote branch at all, and +// remoteName would guess a remote b never used. +func (b branch) remoteDeletable() bool { + return b.upstream != "" && b.upRemote != "." && !b.remoteOnly && !b.locked() && !b.remoteProtected +} + +// remoteRefusal says why r cannot arm b's remote delete, for the cases the row +// does not already show. +func (b branch) remoteRefusal() string { + switch { + case b.remoteOnly || b.locked() || b.upstream == "": + return "" + case b.upRemote == ".": + return b.name + " tracks local branch " + b.upstream + ": there is no remote branch to delete" + case b.remoteProtected: + return "remote branch " + b.remoteName() + "/" + b.remoteBranch() + " is protected" + } + return "" +} // ref is b's fully qualified ref (see branchRef). func (b branch) ref() string { @@ -130,9 +156,25 @@ var ( // It is what picks the cap, so a new network subcommand belongs here rather than // at its call site. func networkBound(args []string) bool { - return len(args) > 0 && (args[0] == "push" || args[0] == "fetch") + if len(args) == 0 { + return false + } + switch args[0] { + case "push", "fetch": + return true + case "cherry", "diff": + // Both read file contents, which a partial clone fetches on demand. + // Killing either on the timeout is safe: they change nothing. + return lazyFetch.Load() + } + return false } +// lazyFetch records that the repository is a partial clone (git clone +// --filter), which fetches missing file contents from its remote on demand. +// loadRepo sets it on every load. +var lazyFetch atomic.Bool + // gauge records the highest number of concurrent holders it has seen. It sits on // the subprocesses rather than on the slots, so a test can tell a cap that works // from one that was removed — instrumenting the cap would stop reporting along @@ -455,6 +497,11 @@ func baseBranch(name string) string { // Returns 0 when there is nothing to compare against. func riskCommitCount(name, base string) int { return riskCommitCountRef(branchRef(name), base) } +// riskUnknown is the count of a branch git cherry could not measure. A partial +// clone that cannot reach its remote fails there, and reading the failure as 0 +// would let x and p select a branch whose commits -D then discards. +const riskUnknown = -1 + // riskCommitCountRef is riskCommitCount for any fully qualified ref, which is // what lets a remote branch be measured the same way. func riskCommitCountRef(ref, base string) int { @@ -463,7 +510,7 @@ func riskCommitCountRef(ref, base string) int { } out, err := runGit("cherry", base, ref) if err != nil { - return 0 + return riskUnknown } n := 0 for _, line := range strings.Split(out, "\n") { @@ -510,23 +557,45 @@ func localMergedSet() map[string]bool { return mergedSet("branch", "--merged", " type repoReads struct { headMerged map[string]bool // local branches merged into HEAD remotes remoteRefs - config prunerConfig + config repoConfig +} + +// repoConfig is the git config the tool reads: its own pruner.* section, and +// the remote settings that decide what a remote delete targets and which git +// commands can reach the network. +type repoConfig struct { + protect []string // pruner.protect: branch name globs that can never be marked + staleDays int // pruner.staleDays: the default age for the merged-and-old rule + fetch []fetchSpec // remote..fetch, in config order + partialClone bool // a remote is a promisor: git fetches missing objects on demand } -// prunerConfig is the tool's own git config, under the pruner.* section. -type prunerConfig struct { - protect []string // pruner.protect: branch name globs that can never be marked - staleDays int // pruner.staleDays: the default age for the merged-and-old rule +// fetchSpec is one remote..fetch refspec: src names refs on the remote, +// dst the tracking refs they are fetched into. Each may hold one "*". +type fetchSpec struct{ remote, src, dst string } + +// srcFor maps a tracking ref back to the remote ref s fetches into it. +func (s fetchSpec) srcFor(ref string) (string, bool) { + pre, post, glob := strings.Cut(s.dst, "*") + if !glob { + return s.src, ref == s.dst + } + if len(ref) < len(pre)+len(post) || !strings.HasPrefix(ref, pre) || !strings.HasSuffix(ref, post) { + return "", false + } + return strings.Replace(s.src, "*", ref[len(pre):len(ref)-len(post)], 1), true } // defaultStaleDays is the age the merged-and-old rule starts from. const defaultStaleDays = 90 -// loadConfig reads every pruner.* key in one call. Unset is the usual case, and -// git exits 1 for it, so an error only means "use the defaults". -func loadConfig() prunerConfig { - c := prunerConfig{staleDays: defaultStaleDays} - out, _ := runGit("config", "--get-regexp", `^pruner\.`) +// loadConfig reads every key repoConfig needs in one call. Unset is the usual +// case for pruner.*, and git exits 1 when nothing matches, so an error only +// means "use the defaults". git lowercases section and key names, but not the +// remote's name between them. +func loadConfig() repoConfig { + c := repoConfig{staleDays: defaultStaleDays} + out, _ := runGit("config", "--get-regexp", `^(pruner|remote)\.|^extensions\.partialclone$`) for _, line := range strings.Split(out, "\n") { key, val, _ := strings.Cut(strings.TrimSpace(line), " ") switch key { @@ -536,11 +605,68 @@ func loadConfig() prunerConfig { if n, err := strconv.Atoi(val); err == nil && n >= 0 { c.staleDays = n } + case "extensions.partialclone": + c.partialClone = true + } + remote, ok := strings.CutPrefix(key, "remote.") + if !ok { + continue + } + if name, ok := strings.CutSuffix(remote, ".promisor"); ok && name != "" && gitBool(val) { + c.partialClone = true + } + // A negative refspec (^) only excludes, and one with no ":" fetches into + // no tracking ref. + if name, ok := strings.CutSuffix(remote, ".fetch"); ok && !strings.HasPrefix(val, "^") { + if src, dst, ok := strings.Cut(strings.TrimPrefix(val, "+"), ":"); ok { + c.fetch = append(c.fetch, fetchSpec{remote: name, src: src, dst: dst}) + } } } return c } +// gitBool reads a config value the way git reads a boolean. A key with no +// value at all is true. +func gitBool(val string) bool { + switch strings.ToLower(val) { + case "", "true", "yes", "on", "1": + return true + } + return false +} + +// mapRemoteRows points each remote row at the branch it tracks on its remote, +// read back through the fetch refspecs, and drops the rows that track no one +// remote branch. A tracking ref's name is only a convention: with the refspec +// refs/heads/jb/*:refs/remotes/origin/*, origin/x tracks jb/x, and a delete +// pushed for x would remove someone else's branch. Refs fetched from outside +// refs/heads/, such as pull request heads, are not branches a push can delete. +// When two refspecs map a ref back to different sources, which one fetched it +// cannot be told locally: the usual pull request refspec next to the default +// one maps origin/pr/1 back to both refs/pull/1/head and refs/heads/pr/1. +func mapRemoteRows(rows []branch, specs []fetchSpec) []branch { + var out []branch + for _, r := range rows { + ref := r.ref() + var remote, src string + n := 0 // distinct sources the ref maps back to + for _, s := range specs { + got, ok := s.srcFor(ref) + if !ok || (n > 0 && remote == s.remote && src == got) { + continue + } + remote, src = s.remote, got + n++ + } + if n == 1 && strings.HasPrefix(src, "refs/heads/") { + r.upRemote, r.upMerge = remote, src + out = append(out, r) + } + } + return out +} + // loadRepo reads the branch list and everything independent of it in one round. // Starting a git subprocess costs about 6ms, and none of these reads waits on // another, so the depth of the chain is what the user waits on — not the work @@ -551,17 +677,23 @@ func loadRepo(withRemote bool) ([]branch, repoReads, error) { err error reads repoReads mainWT string + held map[string]string ) var wg sync.WaitGroup - wg.Add(5) + wg.Add(6) go func() { defer wg.Done(); branches, err = loadBranches() }() go func() { defer wg.Done(); reads.headMerged = localMergedSet() }() go func() { defer wg.Done(); reads.remotes = loadRemoteRefs(withRemote) }() go func() { defer wg.Done(); reads.config = loadConfig() }() go func() { defer wg.Done(); mainWT = mainWorktreePath() }() + go func() { defer wg.Done(); held = loadHeldBranches() }() wg.Wait() + lazyFetch.Store(reads.config.partialClone) + reads.remotes.rows = mapRemoteRows(reads.remotes.rows, reads.config.fetch) for i := range branches { - branches[i].mainWorktree = mainWT != "" && branches[i].worktree == mainWT + b := &branches[i] + b.mainWorktree = mainWT != "" && b.worktree == mainWT + b.heldBy = held[b.ref()] } return branches, reads, err } @@ -580,6 +712,62 @@ func mainWorktreePath() string { return "" } +// loadHeldBranches finds the branches that a rebase or bisect in progress holds, +// in any worktree, mapping each full ref to the operation's name. git refuses +// to delete such a branch under -d and -D alike, yet %(worktreepath) does not +// report it: the worktree's HEAD is detached while the operation runs. Each +// worktree keeps this state in its own git dir: the common dir for the main +// worktree, and worktrees/ under it for each linked one. +func loadHeldBranches() map[string]string { + out, err := runGit("rev-parse", "--path-format=absolute", "--git-common-dir") + if err != nil { + return nil + } + common := strings.TrimSpace(out) + linked, _ := filepath.Glob(filepath.Join(common, "worktrees", "*")) + held := map[string]string{} + for _, dir := range append([]string{common}, linked...) { + readHeld(dir, held) + } + return held +} + +// readHeld adds the branches held in one worktree's git dir. It follows git's +// own list (prepare_checked_out_branches in branch.c). +func readHeld(dir string, held map[string]string) { + read := func(name string) string { + data, _ := os.ReadFile(filepath.Join(dir, name)) + return strings.TrimSpace(string(data)) + } + // A rebase started from a detached HEAD writes "detached HEAD" here. + for _, f := range []string{"rebase-merge/head-name", "rebase-apply/head-name"} { + if ref := read(f); strings.HasPrefix(ref, "refs/heads/") { + held[ref] = "rebase" + } + } + // rebase --update-refs lists each branch it will move, as a ref followed by + // its old and new commit. + lines := strings.Split(read("rebase-merge/update-refs"), "\n") + for i := 0; i < len(lines); i += 3 { + if strings.HasPrefix(lines[i], "refs/heads/") { + held[lines[i]] = "rebase" + } + } + // BISECT_START names the branch bisect started from, or holds a commit ID + // when it started from a detached HEAD. + if start := read("BISECT_START"); start != "" && !isObjectID(start) { + held[branchRef(start)] = "bisect" + } +} + +// isObjectID reports whether s is a full SHA-1 or SHA-256 object ID. +func isObjectID(s string) bool { + if len(s) != 40 && len(s) != 64 { + return false + } + return strings.Trim(s, "0123456789abcdef") == "" +} + // loadDiff returns the patch introduced on name relative to its merge-base with // the repo's default branch — i.e. what the branch contains — and the base it was // compared against, shortened for display. diff --git a/main_test.go b/main_test.go index cbf1712..6001305 100644 --- a/main_test.go +++ b/main_test.go @@ -99,7 +99,9 @@ func wantState(t *testing.T, m model, want viewState, why string) { } } -// remoteHasBranch reports whether origin still has the named branch. +// remoteHasBranch reports whether origin still has the named branch. ls-remote +// matches a short name against the end of every ref, so "old" also finds +// jb/old; pass a full ref such as refs/heads/old to match one branch only. func remoteHasBranch(t *testing.T, dir, name string) bool { t.Helper() return git(t, dir, "ls-remote", "--heads", "origin", name) != "" @@ -2986,3 +2988,231 @@ func TestMainWorktreeBranchCannotBeMarked(t *testing.T) { t.Fatalf("a branch held by another worktree must show the + marker:\n%q", row) } } + +// gitStops runs a git command that is expected to stop partway, such as a +// rebase that hits a conflict. +func gitStops(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("git %s should have stopped partway:\n%s", strings.Join(args, " "), out) + } +} + +// A rebase or bisect holds the branch it started from, and a rebase +// --update-refs holds every branch in its stack. git refuses to delete them in +// any worktree, under -d and -D alike, yet the worktree's HEAD is detached, so +// neither %(HEAD) nor %(worktreepath) reports them. +func TestHeldBranchesAreLocked(t *testing.T) { + repo := initRepo(t, "main") + commitFile(t, repo, "f", "a") + for _, name := range []string{"stack/base", "stack/top", "bisected", "applied"} { + git(t, repo, "branch", name) + } + commitFile(t, repo, "f", "main") + + // Main worktree: a rebase --update-refs that stops on its second commit. + git(t, repo, "switch", "-q", "stack/base") + commitFile(t, repo, "g", "base") + git(t, repo, "switch", "-q", "-C", "stack/top") + commitFile(t, repo, "f", "top") + gitStops(t, repo, "rebase", "--update-refs", "main") + + // Linked worktrees: a bisect, and a rebase on the apply backend. + bisectWT := filepath.Join(realTempDir(t), "bisect") + git(t, repo, "worktree", "add", "-q", bisectWT, "bisected") + commitFile(t, bisectWT, "h", "1") + commitFile(t, bisectWT, "h", "2") + git(t, bisectWT, "bisect", "start", "bisected", "main") + applyWT := filepath.Join(realTempDir(t), "apply") + git(t, repo, "worktree", "add", "-q", applyWT, "applied") + commitFile(t, applyWT, "f", "applied") + gitStops(t, applyWT, "rebase", "--apply", "main") + + want := map[string]string{"stack/base": "rebase", "stack/top": "rebase", "bisected": "bisect", "applied": "rebase"} + for name := range want { + cmd := exec.Command("git", "branch", "-D", name) + cmd.Dir = repo + if out, err := cmd.CombinedOutput(); err == nil || !strings.Contains(string(out), "used by worktree") { + t.Fatalf("precondition: git should refuse to delete %s: %v %s", name, err, out) + } + } + + chdir(t, repo) + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + for name, op := range want { + b := find(m.branches, name) + if b.worktree != "" || b.isCurrent { + t.Fatalf("precondition: git reports %s as checked out: %+v", name, b) + } + if b.heldBy != op || !b.locked() { + t.Fatalf("%s: heldBy=%q locked=%v, want %q and locked", name, b.heldBy, b.locked(), op) + } + row := stripANSI(m.renderRow(*b, 12, false)) + if !strings.Contains(row, " ~ "+name) || !strings.Contains(row, op+" in progress") { + t.Fatalf("the row must show the ~ marker and the reason: %q", row) + } + } + m = press(t, m, key("a")) + for _, b := range m.selectedBranches() { + if want[b.name] != "" { + t.Fatalf("a must skip held branch %s", b.name) + } + } +} + +// r deletes the remote branch a branch tracks, which need not share its name. +// A branch made from origin/main tracks main, one made from a protected branch +// tracks that branch, and one made with --track main tracks no remote branch at +// all. None of these may be armed. +func TestRemoteDeleteRefusesWrongTargets(t *testing.T) { + repo := initRepo(t, "main") + commitFile(t, repo, "a", "a") + addOrigin(t, repo, "main") + git(t, repo, "push", "-q", "origin", "main:release/1.0") + git(t, repo, "fetch", "-q") + git(t, repo, "config", "pruner.protect", "release/*") + git(t, repo, "branch", "hotfix", "origin/release/1.0") + git(t, repo, "branch", "feat", "origin/main") + git(t, repo, "branch", "--track", "stacked", "main") + git(t, repo, "switch", "-q", "-c", "feature/ok") + git(t, repo, "push", "-q", "-u", "origin", "feature/ok") + git(t, repo, "switch", "-q", "main") + chdir(t, repo) + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + + for name, why := range map[string]string{ + "hotfix": "remote branch origin/release/1.0 is protected", + "feat": "remote branch origin/main is protected", + "stacked": "stacked tracks local branch main", + } { + cursorTo(t, &m, name) + m.status = "" + m = press(t, m, key("r")) + if b := find(m.branches, name); b.deleteRemote || b.remoteArmed() { + t.Fatalf("r must not arm the remote delete of %s (target %s/%s)", name, b.remoteName(), b.remoteBranch()) + } + if !strings.Contains(m.status, why) { + t.Fatalf("%s: status %q should say %q", name, m.status, why) + } + } + cursorTo(t, &m, "feature/ok") + if m = press(t, m, key("r")); !find(m.branches, "feature/ok").deleteRemote { + t.Fatal("r must still arm a branch whose remote branch is its own") + } + if !remoteHasBranch(t, repo, "refs/heads/release/1.0") || !remoteHasBranch(t, repo, "refs/heads/main") { + t.Fatal("no remote branch should have been touched") + } +} + +// The remote view maps each tracking ref back to its remote branch through the +// fetch refspecs: with refs/heads/jb/*:refs/remotes/origin/*, origin/old is +// jb/old, and a teammate's branch named old must survive its delete. Refs +// fetched from outside refs/heads/ are not branches, so they are not listed. +// Every remote's trunk is protected, not only the default remote's. +func TestRemoteRowsFollowFetchRefspecs(t *testing.T) { + repo := initRepo(t, "main") + commitFile(t, repo, "a", "a") + addOrigin(t, repo, "main") + git(t, repo, "push", "-q", "origin", "main:jb/old", "main:old", "main:refs/pull/1/head") + git(t, repo, "config", "--replace-all", "remote.origin.fetch", "+refs/heads/jb/*:refs/remotes/origin/*") + git(t, repo, "config", "--add", "remote.origin.fetch", "+refs/pull/*/head:refs/remotes/origin/pr/*") + git(t, repo, "fetch", "-q", "--prune", "origin") + fork := t.TempDir() + git(t, fork, "init", "--bare", "-q", "-b", "main") + git(t, repo, "remote", "add", "fork", fork) + git(t, repo, "push", "-q", "fork", "main", "main:topic") + git(t, repo, "fetch", "-q", "fork") + chdir(t, repo) + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + nm, cmd := m.Update(special(tea.KeyTab)) + nm, _ = nm.(model).Update(cmd()) + m = nm.(model) + + if find(m.branches, "origin/pr/1") != nil { + t.Fatal("a pull request ref is not a remote branch and must not be listed") + } + if b := find(m.branches, "fork/main"); b == nil || !b.protected { + t.Fatalf("another remote's trunk must be protected: %+v", b) + } + if b := find(m.branches, "fork/topic"); b == nil || b.locked() || b.remoteBranch() != "topic" { + t.Fatalf("a remote with the default refspec maps by name: %+v", b) + } + old := find(m.branches, "origin/old") + if old == nil || old.remoteName() != "origin" || old.remoteBranch() != "jb/old" { + t.Fatalf("origin/old must map to origin's jb/old: %+v", old) + } + + cursorTo(t, &m, "origin/old") + m = press(t, m, key(" "), key("d")) + wantState(t, m, stateConfirm, "d opens the confirm screen") + nm, cmd = m.Update(key("R")) + m = drainDeletions(t, nm.(model), cmd) + if r := result(t, m, "origin/old"); !r.remoteOK { + t.Fatalf("the delete should succeed: %+v", r) + } + if remoteHasBranch(t, repo, "refs/heads/jb/old") { + t.Fatal("the tracked branch jb/old should be deleted") + } + if !remoteHasBranch(t, repo, "refs/heads/old") { + t.Fatal("the teammate's branch old must not be deleted") + } +} + +// A partial clone fetches file contents on demand, so git cherry can need the +// remote. When the remote cannot be reached the count is unknown, not zero: a +// zero would let x select a gone branch whose commit -D then discards. +func TestPartialCloneRiskIsNotGuessed(t *testing.T) { + t.Cleanup(func() { lazyFetch.Store(false) }) + src := initRepo(t, "main") + git(t, src, "config", "uploadpack.allowFilter", "true") + commitFile(t, src, "f", "a") + git(t, src, "branch", "feat") + commitFile(t, src, "f", "main") + // Both commits change only f, so cherry has to compare their contents. + git(t, src, "switch", "-q", "feat") + commitFile(t, src, "f", "feat") + git(t, src, "switch", "-q", "main") + + clone := filepath.Join(t.TempDir(), "clone") + git(t, src, "clone", "-q", "--filter=blob:none", "--no-checkout", "file://"+src, clone) + git(t, clone, "branch", "-q", "feat", "origin/feat") + git(t, src, "branch", "-q", "-D", "feat") + git(t, clone, "fetch", "-q", "--prune") + git(t, clone, "remote", "set-url", "origin", "file://"+filepath.Join(t.TempDir(), "gone")) + chdir(t, clone) + + m, err := initialModel() + if err != nil { + t.Fatal(err) + } + if !networkBound([]string{"cherry"}) || !networkBound([]string{"diff"}) { + t.Fatal("cherry and diff can reach the remote in a partial clone") + } + b := find(m.branches, "feat") + if !b.gone || !b.riskMeasured || b.riskCommits != riskUnknown { + t.Fatalf("want a gone branch with an unknown risk count: %+v", b) + } + if status := m.selectGone(); find(m.branches, "feat").selected || !strings.Contains(status, "1 could not be checked") { + t.Fatalf("x must leave an unmeasured branch unselected: %q", status) + } + b.selected = true + if w := m.riskWarning(*b); !strings.Contains(w, "an unknown number of commits") { + t.Fatalf("the confirm screen must say the count is unknown: %q", w) + } + + lazyFetch.Store(false) + if networkBound([]string{"cherry"}) { + t.Fatal("outside a partial clone, cherry is local") + } +} diff --git a/model.go b/model.go index ea83af7..cae13c9 100644 --- a/model.go +++ b/model.go @@ -100,7 +100,7 @@ type model struct { // can strand a remote copy, so a second press is needed. abortArmed bool - config prunerConfig + config repoConfig remoteDefault string // resolved remote default branch, e.g. "origin/main" remoteDefaultRef string // remoteDefault fully qualified riskBase string // ref that branch.riskCommits is measured against ("" if unresolved) @@ -168,7 +168,10 @@ func switchCmd(b branch, withRemote bool) tea.Cmd { args := []string{"switch", b.name} msg := switchDoneMsg{name: b.name} if b.remoteOnly { - msg.name, msg.fromRemote = b.remoteBranch(), true + // Name the local copy after the tracking ref, as git's own checkout + // does: a fetch refspec can give the remote's branch another name. + _, local, _ := strings.Cut(b.name, "/") + msg.name, msg.fromRemote = local, true args = []string{"switch", "-c", msg.name, "--track", b.ref()} } return func() tea.Msg { @@ -705,8 +708,12 @@ func (m model) updateList(msg tea.KeyMsg) (tea.Model, tea.Cmd) { b.selected = !b.selected } case "r": - if b := m.cur(); b != nil && b.upstream != "" && !b.locked() && !b.remoteOnly { + if b := m.cur(); b != nil && b.remoteDeletable() { b.deleteRemote = !b.deleteRemote + } else if b != nil { + if why := b.remoteRefusal(); why != "" { + m.status = why + } } case "tab": m.showRemote = !m.showRemote @@ -992,8 +999,14 @@ func (m *model) installBranches(branches []branch, reads repoReads) { b := &m.branches[i] b.headMerged = !b.remoteOnly && reads.headMerged[b.name] b.protected = m.isProtected(*b) + // The trunk and protect checks apply to the remote branch r would delete, + // not only to the local name: a branch made from origin/main tracks main. + b.remoteProtected = !b.remoteOnly && b.upstream != "" && m.protectsName(b.remoteBranch()) if b.locked() { - b.selected, b.deleteRemote = false, false + b.selected = false + } + if !b.remoteDeletable() { + b.deleteRemote = false } } m.sortRows() @@ -1020,13 +1033,16 @@ func remoteOnlyRows(local, remote []branch) []branch { // isProtected reports whether b is the trunk or matches a pruner.protect glob. // A remote row is matched by its branch part, so "release/*" covers both views. func (m model) isProtected(b branch) bool { - name := b.name if b.remoteOnly { - if b.name == m.remoteDefault { - return true - } - name = b.remoteBranch() - } else if _, trunk, ok := strings.Cut(m.remoteDefault, "/"); b.ref() == m.riskBaseRef || (ok && name == trunk) { + return b.name == m.remoteDefault || m.protectsName(b.remoteBranch()) + } + return m.protectsName(b.name) +} + +// protectsName reports whether a branch of this name, local or on a remote, is +// the trunk or matches a pruner.protect glob. +func (m model) protectsName(name string) bool { + if _, trunk, ok := strings.Cut(m.remoteDefault, "/"); (ok && name == trunk) || branchRef(name) == m.riskBaseRef { return true } for _, p := range m.config.protect { @@ -1293,28 +1309,37 @@ func (m *model) applyUndo(msg undoDoneMsg) { // commits stay unselected so discarding them is a deliberate keystroke rather // than a side effect of `p` or `x`. func (m *model) selectGone() string { - gone, risky := 0, 0 + gone, risky, unknown := 0, 0, 0 for i := range m.branches { br := &m.branches[i] if !br.gone || br.locked() { continue } gone++ - if br.riskCommits > 0 { + switch { + case br.riskCommits > 0: risky++ continue + case br.riskCommits == riskUnknown: + unknown++ + continue } br.selected = true } switch { case gone == 0: return "no gone branches" - case risky == 0: + case risky+unknown == 0: return fmt.Sprintf("%d gone branch(es) selected; press d to prune", gone) - default: - return fmt.Sprintf("%d of %d gone branch(es) selected; %d hold commits not in %s (select with space to discard)", - gone-risky, gone, risky, m.riskBase) } + msg := fmt.Sprintf("%d of %d gone branch(es) selected", gone-risky-unknown, gone) + if risky > 0 { + msg += fmt.Sprintf("; %d hold commits not in %s", risky, m.riskBase) + } + if unknown > 0 { + msg += fmt.Sprintf("; %d could not be checked", unknown) + } + return msg + " (select with space to discard)" } // merged reports whether b's work is already in the default branch. A local diff --git a/view.go b/view.go index b90fb77..93a312d 100644 --- a/view.go +++ b/view.go @@ -331,6 +331,8 @@ func (m model) renderRow(br branch, nameW int, isCursor bool) string { cur = currentStyle.Render("*") case br.worktree != "": cur = hunkStyle.Render("+") + case br.heldBy != "": + cur = goneStyle.Render("~") case br.protected: cur = dimStyle.Render("P") } @@ -365,7 +367,11 @@ func (m model) renderRow(br branch, nameW int, isCursor bool) string { row += " " + hashStyle.Render(fmt.Sprintf("%-8s", br.hash)) } if c.subject > 0 { - row += " " + subjectStyle.Render(truncate(br.subject, c.subject)) + subject := br.subject + if br.heldBy != "" { + subject = br.heldBy + " in progress · " + subject + } + row += " " + subjectStyle.Render(truncate(subject, c.subject)) } // Last guard against a wrap on a terminal too narrow for even the fixed columns. row = ansi.Truncate(row, max(1, m.width), "") @@ -536,6 +542,7 @@ func (m model) helpView() string { writeRows([][2]string{ {"* / P", "current / protected (locked)"}, {"+", "in another worktree (removed on delete; locked if the main one)"}, + {"~", "held by a rebase or bisect in progress (locked)"}, {"[x]", "selected for deletion"}, {"R", "its remote branch will also be deleted"}, {"↑/↓", "commits ahead of / behind upstream"}, @@ -658,8 +665,8 @@ func (m model) worktreeWarning(br branch) string { // discarded, under -d the delete simply fails. func (m model) riskWarning(br branch) string { if br.remoteOnly { - if br.riskCommits > 0 { - return fmt.Sprintf("⚠ %d commit(s) not in %s — this may be the only shared copy of them", br.riskCommits, m.riskBase) + if br.riskCommits != 0 { + return fmt.Sprintf("⚠ %s — this may be the only shared copy of them", m.riskCount(br)) } return "" } @@ -667,8 +674,8 @@ func (m model) riskWarning(br branch) string { return "" } if br.forcedDelete(m.force) { // -D: the question is what gets discarded - if br.riskCommits > 0 { - return fmt.Sprintf("⚠ %d commit(s) not in %s — force delete (-D) will discard them", br.riskCommits, m.riskBase) + if br.riskCommits != 0 { + return fmt.Sprintf("⚠ %s — force delete (-D) will discard them", m.riskCount(br)) } if m.riskBase == "" { return "⚠ no base branch to compare against — force delete (-D) discards any unmerged commits" @@ -676,12 +683,22 @@ func (m model) riskWarning(br branch) string { return "" // measured against a real base: nothing here is at risk } // -d will be refused either way; the count is what a force would then cost. - if br.riskCommits > 0 { - return fmt.Sprintf("⚠ not fully merged: %d commit(s) not in %s — safe delete (-d) will fail; use force (f)", br.riskCommits, m.riskBase) + if br.riskCommits != 0 { + return fmt.Sprintf("⚠ not fully merged: %s — safe delete (-d) will fail; use force (f)", m.riskCount(br)) } return "⚠ not fully merged — safe delete (-d) will fail; use force (f)" } +// riskCount phrases br's risk count for a warning. A count git cherry could not +// take (riskUnknown) is nonzero on purpose, so every warning that tests for +// commits at risk also speaks for it. +func (m model) riskCount(br branch) string { + if br.riskCommits == riskUnknown { + return "an unknown number of commits not in " + m.riskBase + } + return fmt.Sprintf("%d commit(s) not in %s", br.riskCommits, m.riskBase) +} + func (m model) forcePromptParts() (header, body, footer []string) { failures := m.forceableFailures() @@ -701,8 +718,8 @@ func (m model) forcePromptParts() (header, body, footer []string) { // ran: riskCommits == 0 means either nothing is missing from the base or // there was no base to measure against. switch { - case r.br.riskCommits > 0: - body = append(body, " "+errStyle.Render(fmt.Sprintf("⚠ %d commit(s) not in %s will be lost", r.br.riskCommits, m.riskBase))) + case r.br.riskCommits != 0: + body = append(body, " "+errStyle.Render(fmt.Sprintf("⚠ %s will be lost", m.riskCount(r.br)))) case m.riskBase == "": body = append(body, " "+errStyle.Render("⚠ no base branch to compare against — unmerged commits may be lost")) default: