diff --git a/docs/decisions.md b/docs/decisions.md index c43fdb4..c669595 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -7,7 +7,8 @@ Each entry: context, decision, consequences. Newest last. **Context.** Hashes don't tell users whether a pin is current. **Decision.** Resolve each pin to a ref and show the distance (`origin/main ↓1`). The resolution order is fixed and the applied rule is shown in the detail pane -(see [state-model.md](state-model.md#resolving-a-pin-to-a-ref)). +(see [state-model.md](state-model.md#resolving-a-pin-to-a-ref); amended: +the closest branch wins, see "Pin resolves to the closest branch"). **Consequences.** Resolution is a heuristic; showing the rule keeps it transparent. @@ -218,3 +219,15 @@ shown in a palette in the detail pane, next to what they act on; global keys (hidden below 90 columns, or under 12 lines) the footer shows these keys instead, so small terminals need no extra UI (no overlay or action menu). Keys never depend on what is visible. + +## Pin resolves to the closest branch + +A pin on a pushed feature branch showed as diverged from `origin/main`, +because `branch = main` in `.gitmodules` won before any distance was looked +at. Now the closest branch containing the pin wins; the `.gitmodules` +branch, the upstream and the remote default branch only break ties (and are +always compared, even past the 16-branch cap). An unpushed pin compares only +those preferred refs, by commits on either side: scanning every branch's +merge base would be too slow. Cost: a pin on main far behind its tip shows a +younger branch that contains it if that branch's tip is closer, so "n behind +main" is no longer visible there; `via ` still tells which ref it is. diff --git a/docs/state-model.md b/docs/state-model.md index 0d7ee37..d11244b 100644 --- a/docs/state-model.md +++ b/docs/state-model.md @@ -104,17 +104,30 @@ A descendant whose load failed (`error`) counts as one danger. ## Resolving a pin to a ref The pin is displayed as a ref, not a hash. A commit can be on several -branches, on none, or deep in the history, so the git provider resolves it by -this order and records which rule applied in `PinInfo.Source`: +branches, on none, or deep in the history. The git provider shows the +**closest** branch and records which rule proposed it in `PinInfo.Source`. + +Preferred refs, in this order (each only if it exists): + +| Preference | Ref | `RefSource` | +|---|---|---| +| a | `branch = …` in `.gitmodules` | `RefFromGitmodules` | +| b | Upstream of the branch checked out in the submodule | `RefFromCurrentBranch` | +| c | Remote default branch (`/HEAD`) | `RefFromDefaultBranch` | + +Resolution: | # | Rule | `RefSource` | |---|---|---| -| 1 | `branch = …` in `.gitmodules` | `RefFromGitmodules` | -| 2 | Branch currently checked out in the submodule | `RefFromCurrentBranch` | -| 3 | Remote default branch, if it contains the pin | `RefFromDefaultBranch` | -| 4 | Any remote branch containing the pin, preferring the closest tip (`git branch -r --contains`) | `RefFromContains` | -| 5 | A tag (`git describe`) | `RefFromTag` | -| 6 | Short SHA | `RefFromSHA` | +| 1 | Pin pushed: the branch containing the pin with the fewest commits after it. Candidates are the preferred refs that contain the pin plus any remote branch containing it (`for-each-ref --contains`); a tie goes to the preferred ref | preferred ref's, else `RefFromContains` | +| 2 | Pin unpushed: the preferred ref with the fewest commits on either side (`↑n↓m`), ties by preference | preferred ref's | +| 3 | A tag (`git describe`) | `RefFromTag` | +| 4 | Short SHA | `RefFromSHA` | + +So a `.gitmodules` branch is shown when it is as close as any other branch, +but a pin that sits on a feature branch shows that branch rather than +"diverged from main". The trade-off: a pin on main that a younger branch +also contains shows the younger branch if its tip is fewer commits away. Distances: `git rev-list --left-right --count ...` gives `Ahead` (left) and `Behind` (right). @@ -143,12 +156,13 @@ are local; `` is the index gitlink. Details: -- Rule 1: `branch = .` means the superproject's current branch. The rule - applies only if `/` exists locally. -- Rule 2 uses the checked-out branch's upstream (`@{upstream}`). -- Rule 4 compares at most 16 containing branches (oldest tip first) and - picks the one with the fewest commits after the pin. -- Rule 6 shows the short SHA; there is no tip and no distance. +- Preference a: `branch = .` means the superproject's current branch. The + ref counts only if `/` exists locally. +- Preference b uses the checked-out branch's upstream (`@{upstream}`). +- Rule 1 counts `rev-list --count ..`. Besides the preferred refs + it compares at most 16 containing branches (oldest tip first). +- Rule 2 counts `rev-list --count ...`. +- Rule 4 shows the short SHA; there is no tip and no distance. - `--ignore-submodules=dirty`: changes inside a nested submodule do not make its parent `changed` (D7), but a moved nested pin does. - Renames are detected only for staged moves (`git mv`); an unstaged move diff --git a/internal/submodule/git/provider.go b/internal/submodule/git/provider.go index e9c41d9..f769582 100644 --- a/internal/submodule/git/provider.go +++ b/internal/submodule/git/provider.go @@ -20,7 +20,7 @@ import ( ) // maxContainsCandidates limits how many remote branches containing the pin -// are compared when resolving the pin by RefFromContains. +// are compared besides the preferred refs when resolving a pushed pin. const maxContainsCandidates = 16 // maxBetween limits the commits listed between pin and tip. @@ -269,80 +269,130 @@ func status(ctx context.Context, in info) (*submodule.Status, error) { } // resolve finds the ref that describes the pin, following the order in -// docs/state-model.md. containing lists the remote branches containing the -// pin, oldest tip first. +// docs/state-model.md: the closest branch wins, the preferred refs (see +// preferredRefs) break ties. containing lists the remote branches containing +// the pin, oldest tip first. func resolve(ctx context.Context, in info, pin, branch string, containing []string) (string, submodule.RefSource, error) { - remote, err := remoteName(ctx, in.dir) + preferred, err := preferredRefs(ctx, in, branch) if err != nil { return "", 0, err } - // 1. branch from .gitmodules - if b := in.branch; b != "" && remote != "" { - if b == "." { - b, _, err = try(ctx, in.superDir, "symbolic-ref", "--short", "-q", "HEAD") - if err != nil { - return "", 0, err - } - } - if b != "" { - if _, ok, err := try(ctx, in.dir, "rev-parse", "--verify", "-q", "refs/remotes/"+remote+"/"+b); err != nil { + // 1. pushed: the branch containing the pin with the fewest commits after it + if len(containing) > 0 { + var cands []candidate + for _, c := range preferred { + if _, ok, err := try(ctx, in.dir, "merge-base", "--is-ancestor", pin, c.ref); err != nil { return "", 0, err } else if ok { - return remote + "/" + b, submodule.RefFromGitmodules, nil + cands = append(cands, c) } } - } - - // 2. upstream of the checked-out branch - if branch != "" { - if up, ok, err := try(ctx, in.dir, "rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}"); err != nil { - return "", 0, err - } else if ok && up != "" { - return up, submodule.RefFromCurrentBranch, nil + for _, r := range containing[:min(len(containing), maxContainsCandidates)] { + if !slices.ContainsFunc(cands, func(c candidate) bool { return c.ref == r }) { + cands = append(cands, candidate{r, submodule.RefFromContains}) + } } - } - - // 3. remote default branch, if it contains the pin - if remote != "" { - if def, ok, err := try(ctx, in.dir, "symbolic-ref", "--short", "-q", "refs/remotes/"+remote+"/HEAD"); err != nil { + if c, ok, err := closest(ctx, in.dir, pin+"..", cands); err != nil { return "", 0, err } else if ok { - if _, ok, err := try(ctx, in.dir, "merge-base", "--is-ancestor", pin, def); err != nil { - return "", 0, err - } else if ok { - return def, submodule.RefFromDefaultBranch, nil - } + return c.ref, c.source, nil } } - // 4. remote branch containing the pin with the closest tip - best, bestN := "", -1 - for _, r := range containing[:min(len(containing), maxContainsCandidates)] { - out, err := run(ctx, in.dir, "rev-list", "--count", pin+".."+r) - if err != nil { - return "", 0, err - } - var n int - if _, err := fmt.Sscan(out, &n); err == nil && (bestN < 0 || n < bestN) { - best, bestN = r, n - } - } - if best != "" { - return best, submodule.RefFromContains, nil + // 2. unpushed: the preferred ref with the fewest commits on either side + if c, ok, err := closest(ctx, in.dir, pin+"...", preferred); err != nil { + return "", 0, err + } else if ok { + return c.ref, c.source, nil } - // 5. nearest tag + // 3. nearest tag if tag, ok, err := try(ctx, in.dir, "describe", "--tags", "--abbrev=0", pin); err != nil { return "", 0, err } else if ok && tag != "" { return tag, submodule.RefFromTag, nil } - // 6. the commit itself + // 4. the commit itself return "", submodule.RefFromSHA, nil } +// candidate is a ref the pin may resolve to and the rule that proposed it. +type candidate struct { + ref string + source submodule.RefSource +} + +// preferredRefs returns the refs the user most likely means, in order of +// preference and without duplicates: the .gitmodules branch, the upstream of +// the checked-out branch, the remote default branch. Each must exist. +func preferredRefs(ctx context.Context, in info, branch string) ([]candidate, error) { + remote, err := remoteName(ctx, in.dir) + if err != nil { + return nil, err + } + var refs []candidate + add := func(ref string, source submodule.RefSource) { + if ref != "" && !slices.ContainsFunc(refs, func(c candidate) bool { return c.ref == ref }) { + refs = append(refs, candidate{ref, source}) + } + } + + // branch from .gitmodules + if b := in.branch; b != "" && remote != "" { + if b == "." { + b, _, err = try(ctx, in.superDir, "symbolic-ref", "--short", "-q", "HEAD") + if err != nil { + return nil, err + } + } + if b != "" { + if _, ok, err := try(ctx, in.dir, "rev-parse", "--verify", "-q", "refs/remotes/"+remote+"/"+b); err != nil { + return nil, err + } else if ok { + add(remote+"/"+b, submodule.RefFromGitmodules) + } + } + } + + // upstream of the checked-out branch + if branch != "" { + if up, ok, err := try(ctx, in.dir, "rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}"); err != nil { + return nil, err + } else if ok { + add(up, submodule.RefFromCurrentBranch) + } + } + + // remote default branch + if remote != "" { + if def, ok, err := try(ctx, in.dir, "symbolic-ref", "--short", "-q", "refs/remotes/"+remote+"/HEAD"); err != nil { + return nil, err + } else if ok { + add(def, submodule.RefFromDefaultBranch) + } + } + return refs, nil +} + +// closest returns the candidate with the fewest commits in the range +// `` (from is e.g. ".."); the first one wins a tie. +func closest(ctx context.Context, dir, from string, cands []candidate) (candidate, bool, error) { + best, bestN := candidate{}, -1 + for _, c := range cands { + out, err := run(ctx, dir, "rev-list", "--count", from+c.ref) + if err != nil { + return candidate{}, false, err + } + var n int + if _, err := fmt.Sscan(out, &n); err == nil && (bestN < 0 || n < bestN) { + best, bestN = c, n + } + } + return best, bestN >= 0, nil +} + // remoteName returns "origin" if it exists, else the first remote, else "". func remoteName(ctx context.Context, dir string) (string, error) { out, err := run(ctx, dir, "remote") diff --git a/internal/submodule/git/provider_test.go b/internal/submodule/git/provider_test.go index 340dec2..39e6b7e 100644 --- a/internal/submodule/git/provider_test.go +++ b/internal/submodule/git/provider_test.go @@ -75,7 +75,7 @@ func TestTree(t *testing.T) { submodule.Walk(repo.Submodules, func(s *submodule.Submodule) { paths = append(paths, s.Path) }) want := []string{ "src/core", "src/ui", "src/net", "libs/store", "libs/log", "libs/log/modules/nested", - "libs/feature", "libs/conflict", "libs/staged", "libs/missing", "vendor", "libs/work", + "libs/feature", "libs/conflict", "libs/staged", "libs/missing", "vendor", "libs/work", "libs/topic", } if !slices.Equal(paths, want) { t.Errorf("paths = %v\nwant %v", paths, want) @@ -106,6 +106,7 @@ func TestStatus(t *testing.T) { {path: "libs/log", ref: "origin/main", source: submodule.RefFromCurrentBranch}, {path: "libs/log/modules/nested", issues: []submodule.IssueKind{submodule.IssueChanged}, ref: "origin/main", source: submodule.RefFromDefaultBranch, checkout: submodule.CheckoutDetached}, {path: "libs/feature", ref: "origin/feat/search", source: submodule.RefFromContains, behind: 1, checkout: submodule.CheckoutDetached}, + {path: "libs/topic", ref: "origin/feat/search", source: submodule.RefFromContains, checkout: submodule.CheckoutDetached}, {path: "libs/conflict", issues: []submodule.IssueKind{submodule.IssueConflict}, ref: "origin/main", source: submodule.RefFromCurrentBranch, staged: true}, {path: "libs/staged", ref: "origin/main", source: submodule.RefFromCurrentBranch, staged: true}, {path: "libs/missing", issues: []submodule.IssueKind{submodule.IssueMissing}}, diff --git a/scripts/make-fixture.sh b/scripts/make-fixture.sh index 707346b..e9e4972 100755 --- a/scripts/make-fixture.sh +++ b/scripts/make-fixture.sh @@ -72,6 +72,8 @@ g "$w" push origin main g "$root/work/log" submodule add "$root/remotes/nested.git" modules/nested g "$root/work/log" commit -m "Add nested" g "$root/work/log" push origin main +# "archive" ties with main (same tip, sorts first) -> the preferred ref wins +g "$root/work/log" push origin main:archive super="$root/super" git init -q "$super" @@ -88,6 +90,7 @@ add ui libs/staged add net libs/missing add ui vendor add work libs/work +add feature libs/topic -b main g "$super" submodule update --init --recursive g "$super" commit -m "Add submodules" @@ -109,6 +112,13 @@ g "$super/libs/feature" checkout origin/feat/search~1 g "$super" add libs/feature g "$super" commit -m "Pin feature search" +# libs/topic: .gitmodules says main, but the pin is the tip of feat/search +# -> the closest branch wins over the configured one +g "$super/libs/topic" fetch +g "$super/libs/topic" checkout origin/feat/search +g "$super" add libs/topic +g "$super" commit -m "Pin topic search" + # src/core: checkout moved away from the pin -> off pin g "$super/src/core" checkout HEAD~1