From ebfe7f8fe40f7dd7f2219026dae581ce9284c2aa Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Wed, 4 Nov 2026 16:49:18 +0000 Subject: [PATCH] Sync stacks with the remote trunk when the local one can't move When trunk is checked out somewhere with a change in the way of its fast forward (say an uncommitted go.work.sum in the main checkout while you work in a linked worktree), sync left trunk behind, and then everything after it worked from that stale tip. A branch whose pull request had merged was kept ("merged into main but that isn't in main yet"), and the branches above it weren't restacked past it, so the submit that syncs first went on to open a pull request carrying the already merged commits and pulled in their reviewers. For a trunk that was only behind its remote, sync now uses the remote's tip in its place for cleanup and restack, and says so in a notice. Local trunk is still left alone and the result still says it wasn't updated; it fast forwards onto the same commits once the change is out of the way. --- README.md | 5 ++++- docs/architecture.md | 5 ++++- pkg/app/sync.go | 18 ++++++++++++++++- pkg/app/sync_cleanup.go | 4 ++-- pkg/app/sync_cleanup_test.go | 38 ++++++++++++++++++++++++++++++++++++ pkg/app/sync_restack.go | 2 +- pkg/app/sync_trunk.go | 8 ++++++++ 7 files changed, 74 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 1afa18b..01689e2 100644 --- a/README.md +++ b/README.md @@ -633,7 +633,10 @@ Every worktree shares one set of stacks, and sync covers all of them. A branch checked out in another worktree is fast forwarded there too, and like `git pull` any uncommitted changes it doesn't touch come along. If a change or an untracked file is in the way, the branch is left where it was and sync ends on `⚠ Synced, but main was not updated`, naming the files -and the checkout, with the `git -C … stash` to run. If another git +and the checkout, with the `git -C … stash` to run. When that branch is a trunk that was only +behind, the stacks on it are synced with the remote's trunk instead (`origin/main`), so a branch +whose pull request merged still goes and the ones above it are restacked past it; a submit +straight after never pushes commits that are already in trunk. If another git process is holding that worktree's index lock (an IDE, a prompt or status bar running `git status`), sync waits up to about two seconds for it; if the lock is still there it leaves the branch, finishes everything else and exits non-zero saying which lock file was held. A diff --git a/docs/architecture.md b/docs/architecture.md index 0b2629c..2cc80f8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -310,7 +310,10 @@ metadata dies with the worktree. So `git stack sync` does the whole job itself: one `git fetch --prune` of the first trunk's remote (`branch..remote`, else `origin`; a second trunk that tracks another remote is still compared with the first remote's copy, and gets a `no-remote` notice when there is none); fast forward every trunk (a diverged trunk is only reset -with `-f` or a yes at the prompt, a dirty checkout gets a notice); delete branches whose PR +with `-f` or a yes at the prompt, a dirty checkout gets a notice, and a trunk that was only +behind but couldn't be moved is stood in for by the remote's tip for the rest of the sync, so +cleanup and restack still see the merges and `submit`'s sync first never leaves merged commits +under the branches it pushes); delete branches whose PR merged or closed, whose tip is already in trunk (a branch sitting exactly on the trunk tip only counts when its recorded base is an older commit it has moved past, i.e. trunk was fast forwarded onto it; an empty branch has base and tip the same and stays, and restack leaves diff --git a/pkg/app/sync.go b/pkg/app/sync.go index 92879cd..7d56fdc 100644 --- a/pkg/app/sync.go +++ b/pkg/app/sync.go @@ -176,6 +176,22 @@ type syncState struct { // done names branches cleanup would have deleted (merged, closed or in // trunk) but kept; restack leaves them where they are. done map[string]bool + // remoteTrunk holds, for a trunk that should have been fast forwarded + // but couldn't be (its checkout is in the way), the remote's tip, which + // cleanup and restack use in its place; see trunkTip. + remoteTrunk map[string]string +} + +// trunkTip is the commit a trunk is synced to: the remote's tip when the +// local branch is only behind it but couldn't be moved, else the local tip. +// Using the remote's means a branch whose pull request merged is still +// cleaned up and the branches above it restacked past it, so a submit +// straight after never pushes commits that are already in trunk. +func (st *syncState) trunkTip(name string) string { + if tip, ok := st.remoteTrunk[name]; ok { + return tip + } + return st.local[name].Head } // worktreeRepo is repo seen from another worktree. @@ -303,7 +319,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()} + st := &syncState{repo: repo, graph: graph, dirty: newDirtyCache(), remoteTrunk: map[string]string{}} st.trunks = graph.Trunks if len(st.trunks) == 0 { def, ok, err := a.d.Git.DefaultBranch(ctx, repo) diff --git a/pkg/app/sync_cleanup.go b/pkg/app/sync_cleanup.go index c61b251..6caba24 100644 --- a/pkg/app/sync_cleanup.go +++ b/pkg/app/sync_cleanup.go @@ -191,7 +191,7 @@ func withWorktree(b git.Branch, path string) git.Branch { // no longer exist locally. func (a *App) cleanupCandidates(ctx context.Context, st *syncState, res *SyncResult) (cands []candidate, gone []string) { for _, s := range st.graph.Stacks { - trunkTip := st.local[s.Trunk].Head + trunkTip := st.trunkTip(s.Trunk) for _, b := range s.Branches { lb, exists := st.local[b.Name] if !exists { @@ -236,7 +236,7 @@ func (a *App) cleanupCandidates(ctx context.Context, st *syncState, res *SyncRes if slices.Contains(st.trunks, pr.Base) { trunk = pr.Base } - if pr.MergeCommit != "" && !a.ancestor(ctx, st, pr.MergeCommit, st.local[trunk].Head) { + if pr.MergeCommit != "" && !a.ancestor(ctx, st, pr.MergeCommit, st.trunkTip(trunk)) { continue } cands = append(cands, candidate{name, lb.Head, ReasonMerged, trunk}) diff --git a/pkg/app/sync_cleanup_test.go b/pkg/app/sync_cleanup_test.go index 33a6340..94b8559 100644 --- a/pkg/app/sync_cleanup_test.go +++ b/pkg/app/sync_cleanup_test.go @@ -649,3 +649,41 @@ func TestSyncRestackDropsAClosedParentsCommits(t *testing.T) { t.Errorf("b = %+v, want its base on main", b) } } + +// main is checked out in the main checkout with a file in the way of its +// fast forward, and the stack is synced from a linked worktree after a's +// pull request merged. main stays where it is, but the stack is synced with +// origin/main: a goes and b is restacked onto the merge, so a submit right +// after pushes only b's own commit. +func TestSyncUsesRemoteTrunkWhenTrunkCantMove(t *testing.T) { + f := newSyncFixture(t) // on b + merge := f.mergeOnRemote(t, "a", 0) + gittest.Run(t, f.dir, "switch", "-q", "main") + before := f.rev(t, "main") + wt := filepath.Join(t.TempDir(), "wt") + gittest.Run(t, f.dir, "worktree", "add", "-q", wt, "b") + gittest.WriteFile(t, f.dir, "a.txt", "mine") // untracked where the merge writes + repo, err := f.deps.Git.Discover(context.Background(), wt) + if err != nil { + t.Fatal(err) + } + res, err := app.New(f.deps).Sync(context.Background(), repo, app.SyncOptions{}) + if err != nil { + t.Fatal(err) + } + if trunk(res, "main").Status != app.TrunkDirty || f.rev(t, "main") != before { + t.Errorf("main should be left: %+v", trunk(res, "main")) + } + if _, ok := deleted(res, "a"); !ok || branchExists(t, f.dir, "a") { + t.Errorf("a should be deleted: deleted %+v kept %+v", res.Deleted, res.Kept) + } + if got := gittest.Run(t, f.dir, "rev-parse", "b~1"); got != merge { + t.Errorf("b should sit on the merge %s, its parent is %s", merge, got) + } + if got := gittest.Run(t, f.dir, "rev-list", "--count", "origin/main..b"); got != "1" { + t.Errorf("b should have one commit of its own past origin/main, has %s", got) + } + if !slices.ContainsFunc(res.Notices, func(n string) bool { return strings.Contains(n, "synced with origin/main instead") }) { + t.Errorf("notices = %q", res.Notices) + } +} diff --git a/pkg/app/sync_restack.go b/pkg/app/sync_restack.go index 96f6600..5899784 100644 --- a/pkg/app/sync_restack.go +++ b/pkg/app/sync_restack.go @@ -117,7 +117,7 @@ func (a *App) syncRestack(ctx context.Context, st *syncState, res *SyncResult) ( func (a *App) planRestack(ctx context.Context, st *syncState, s *stack.Stack) stackPlan { return stackPlan{stack: s, restackPlan: a.planMoves(ctx, planInput{ repo: st.repo, stackName: s.Bottom(), branches: s.Branches, - parentName: s.Trunk, parentTip: st.local[s.Trunk].Head, + parentName: s.Trunk, parentTip: st.trunkTip(s.Trunk), local: st.local, dirty: st.dirty, done: st.done, })} } diff --git a/pkg/app/sync_trunk.go b/pkg/app/sync_trunk.go index b74595d..3129ef4 100644 --- a/pkg/app/sync_trunk.go +++ b/pkg/app/sync_trunk.go @@ -96,6 +96,14 @@ func (a *App) syncTrunks(ctx context.Context, st *syncState, o SyncOptions, res return err } res.NotUpdated = append(res.NotUpdated, n) + if t.Status == TrunkFastForwarded { + // Only behind: the remote's tip has everything the local + // one has, so the stacks on it can go there instead. + st.remoteTrunk[trunk] = remoteTip + if st.graph.IsTrunk(trunk) { + res.notice("the stacks on %s were synced with %s/%s instead", trunk, st.remote, trunk) + } + } t.Status = TrunkNotUpdated if !n.Failed() { t.Status = TrunkDirty