From 83bff569a0e4443db84c50ab9f165381d30a26d8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 23 Sep 2026 14:02:32 +0200 Subject: [PATCH] Allow fast-forwarding a range of branches After somebody else rebased and force-pushed a stack of branches, every branch of the stack has to be brought back to its upstream, and pressing `f` on them one by one is as tedious as the checking out and pulling it replaces. Let `f` work on a range selection. The upstream branches are fetched with one `git fetch` per remote, and the branches that aren't checked out anywhere move in a single `git update-ref` call. All the selected branches are looked at before any of them is moved, so a branch that has to be refused leaves the others alone rather than updating the stack halfway. Co-Authored-By: Claude Opus 5 (1M context) --- docs-master/Stacked_Branches.md | 5 + pkg/commands/git_commands/sync.go | 24 ++- pkg/gui/controllers/branches_controller.go | 16 +- .../controllers/helpers/branches_helper.go | 203 +++++++++++++----- ...ast_forward_rewritten_stack_of_branches.go | 45 ++++ ...en_stack_of_branches_with_local_commits.go | 50 +++++ pkg/integration/tests/sync/shared.go | 41 ++++ pkg/integration/tests/test_list.go | 2 + 8 files changed, 317 insertions(+), 69 deletions(-) create mode 100644 pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches.go create mode 100644 pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches_with_local_commits.go diff --git a/docs-master/Stacked_Branches.md b/docs-master/Stacked_Branches.md index 426fc9203..c9b080e11 100644 --- a/docs-master/Stacked_Branches.md +++ b/docs-master/Stacked_Branches.md @@ -36,3 +36,8 @@ branch at some point. It finds that out from the reflog of the remote-tracking branch. Reflogs are enabled by default, except in a bare repository; if you work in one with linked worktrees, set `core.logAllRefUpdates` to true there to make this work. + +`f` works on a [range selection](Range_Select.md) too, so you can select the +whole stack and bring all of it back in sync at once. If any of the selected +branches can't be updated, none of them is, so that you don't end up with half +of the stack updated. diff --git a/pkg/commands/git_commands/sync.go b/pkg/commands/git_commands/sync.go index c11846c8d..603e5b8eb 100644 --- a/pkg/commands/git_commands/sync.go +++ b/pkg/commands/git_commands/sync.go @@ -6,6 +6,7 @@ import ( "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/gocui" + "github.com/samber/lo" ) type SyncCommands struct { @@ -113,20 +114,25 @@ func (self *SyncCommands) Pull(task gocui.Task, opts PullOptions) error { return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).Run() } -// Fetches the given branch of the given remote, updating its remote-tracking -// branch. Local branches are left alone, including the one that tracks it. -func (self *SyncCommands) FetchRemoteBranch( +// Fetches the given branches of the given remote, updating their +// remote-tracking branches. Local branches are left alone, including the ones +// that track them. +func (self *SyncCommands) FetchRemoteBranches( task gocui.Task, remoteName string, - remoteBranchName string, + remoteBranchNames []string, ) error { + // The explicit destinations and the leading + make sure that the + // remote-tracking branches are updated even when the remote branches were + // rewritten, whatever the remote's fetch refspec says + refspecs := lo.Map(remoteBranchNames, func(remoteBranchName string, _ int) string { + return fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%s", + remoteBranchName, remoteName, remoteBranchName) + }) + cmdArgs := self.fetchCommandBuilder(false). Arg(remoteName). - // The explicit destination and the leading + make sure that the - // remote-tracking branch is updated even when the remote branch was - // rewritten, whatever the remote's fetch refspec says - Arg(fmt.Sprintf("+refs/heads/%s:refs/remotes/%s/%s", - remoteBranchName, remoteName, remoteBranchName)). + Arg(refspecs...). ToArgv() return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 8d51fa09b..d4060e34e 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -143,8 +143,8 @@ func (self *BranchesController) GetKeybindings(opts types.KeybindingsOpts) []*ty }, { Keys: opts.GetKeys(opts.Config.Branches.FastForward), - Handler: self.withItem(self.fastForward), - GetDisabledReason: self.require(self.singleItemSelected(self.branchIsReal)), + Handler: self.withItems(self.fastForward), + GetDisabledReason: self.require(self.itemRangeSelected(self.branchesAreReal)), Description: self.c.Tr.FastForward, Tooltip: self.c.Tr.FastForwardTooltip, }, @@ -654,21 +654,23 @@ func (self *BranchesController) rebase(branch *models.Branch) error { return self.c.Helpers().MergeAndRebase.RebaseOntoRef(branch.Name) } -func (self *BranchesController) fastForward(branch *models.Branch) error { - if !branch.IsTrackingRemote() { +func (self *BranchesController) fastForward(branches []*models.Branch) error { + if !lo.EveryBy(branches, func(branch *models.Branch) bool { return branch.IsTrackingRemote() }) { return errors.New(self.c.Tr.FwdNoUpstream) } - if !branch.RemoteBranchStoredLocally() { + if !lo.EveryBy(branches, func(branch *models.Branch) bool { return branch.RemoteBranchStoredLocally() }) { return errors.New(self.c.Tr.FwdNoLocalUpstream) } // A branch that is only ahead has nothing to fast-forward to. One that is // both ahead and behind may still be reset to its upstream, so let the // helper look into it. - if branch.IsAheadForPull() && !branch.IsBehindForPull() { + if lo.SomeBy(branches, func(branch *models.Branch) bool { + return branch.IsAheadForPull() && !branch.IsBehindForPull() + }) { return errors.New(self.c.Tr.FwdCommitsToPush) } - return self.c.Helpers().BranchesHelper.FastForwardBranch(branch) + return self.c.Helpers().BranchesHelper.FastForwardBranches(branches) } func (self *BranchesController) createTag(branch *models.Branch) error { diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 9e010786c..effbf6a42 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -434,16 +434,33 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool, fe return fetchErr } -// Updates the given branch to its upstream branch, fetching that first. The -// branch is moved forward if it is behind its upstream, and reset to it if it -// has diverged without having any commits of its own. -func (self *BranchesHelper) FastForwardBranch(branch *models.Branch) error { - worktree, checkedOut := self.worktreeForBranch(branch) +// One of the branches that a fast-forward is about to bring to its upstream +type branchToForward struct { + branch *models.Branch + // the worktree that the branch is checked out in, nil if there is none + worktree *models.Worktree + // whether the branch has to be reset to its upstream because it has + // diverged from it, as opposed to being moved forward + reset bool +} - return self.c.WithInlineStatus(branch, types.ItemOperationFastForwarding, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { +// Updates the given branches to their upstream branches, fetching those first. +// A branch that is behind its upstream is moved forward to it; one that has +// diverged from it is reset to it, as long as it has no commits of its own. If +// any of the branches can't be updated, none of them is. +func (self *BranchesHelper) FastForwardBranches(branches []*models.Branch) error { + // The worktrees come from the model, so they have to be looked up here, + // before the work moves to a worker + toForward := lo.Map(branches, func(branch *models.Branch, _ int) *branchToForward { + worktree, _ := self.worktreeForBranch(branch) + return &branchToForward{branch: branch, worktree: worktree} + }) + anyCheckedOut := lo.SomeBy(toForward, func(f *branchToForward) bool { return f.worktree != nil }) + + return self.withFastForwardingStatus(branches, func(task gocui.Task) error { defer func() { - if checkedOut { - // The files of that worktree have changed as well + if anyCheckedOut { + // The files of those worktrees have changed as well self.c.RefreshFromWorker(types.RefreshOptions{}) } else { self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}}) @@ -452,72 +469,152 @@ func (self *BranchesHelper) FastForwardBranch(branch *models.Branch) error { self.c.LogAction(self.c.Tr.Actions.FastForwardBranch) - err := self.c.Git().Sync.FetchRemoteBranch(task, branch.UpstreamRemote, branch.UpstreamBranch) - if err != nil { + if err := self.fetchUpstreamBranches(task, branches); err != nil { return err } - isFastForward := self.c.Git().Branch.IsAncestor( - branch.FullRefName(), branch.FullUpstreamRefName()) - if !isFastForward { - // Moving the branch to its upstream means giving up the commits it - // is ahead by, so make sure that none of them is ours - hasLocalOnlyCommits, err := self.c.Git().Branch.HasLocalOnlyCommits(branch) - if err != nil { + // Look at all the branches before moving any of them, so that one we + // have to refuse leaves the others alone too + for _, f := range toForward { + if err := self.planForwardingBranch(f); err != nil { return err } - if hasLocalOnlyCommits { - return errors.New(utils.ResolvePlaceholderString( - self.c.Tr.FwdLocalOnlyCommits, - map[string]string{"branchName": branch.Name}, - )) - } } - if checkedOut { - return self.forwardCheckedOutBranch(branch, worktree, isFastForward) - } - - updateCommand := fmt.Sprintf("update %s %s %s", - branch.FullRefName(), branch.FullUpstreamRefName(), branch.CommitHash) - self.c.LogCommand(updateCommand, false) - return self.c.Git().Branch.UpdateBranchRefs(updateCommand + "\n") + return self.forwardBranches(toForward) }) } -// Updates a branch that is checked out in the given worktree, which needs the -// files there to be updated along with it. -func (self *BranchesHelper) forwardCheckedOutBranch( - branch *models.Branch, worktree *models.Worktree, isFastForward bool, -) error { - worktreeGitDir := "" - worktreePath := "" - // if it is the current worktree path, no need to specify the path - if !worktree.IsCurrent { - worktreeGitDir = worktree.GitDir - worktreePath = worktree.Path +// Runs f with all the given branches shown as being fast-forwarded +func (self *BranchesHelper) withFastForwardingStatus(branches []*models.Branch, f func(gocui.Task) error) error { + return self.c.WithInlineStatus(branches[0], types.ItemOperationFastForwarding, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { + for _, branch := range branches[1:] { + self.c.State().SetItemOperation(branch, types.ItemOperationFastForwarding) + } + defer func() { + for _, branch := range branches[1:] { + self.c.State().ClearItemOperation(branch) + } + }() + + return f(task) + }) +} + +func (self *BranchesHelper) fetchUpstreamBranches(task gocui.Task, branches []*models.Branch) error { + remotes := lo.Uniq(lo.Map(branches, func(branch *models.Branch, _ int) string { + return branch.UpstreamRemote + })) + + for _, remote := range remotes { + remoteBranches := []string{} + for _, branch := range branches { + if branch.UpstreamRemote == remote { + remoteBranches = append(remoteBranches, branch.UpstreamBranch) + } + } + + if err := self.c.Git().Sync.FetchRemoteBranches(task, remote, remoteBranches); err != nil { + return err + } } - if isFastForward { - return self.c.Git().Branch.FastForwardMerge( - branch.FullUpstreamRefName(), worktreeGitDir, worktreePath) + return nil +} + +// Works out whether the branch has to be reset to its upstream, and returns an +// error if it can't be brought there at all. +func (self *BranchesHelper) planForwardingBranch(f *branchToForward) error { + f.reset = !self.c.Git().Branch.IsAncestor( + f.branch.FullRefName(), f.branch.FullUpstreamRefName()) + if !f.reset { + return nil } - // Resetting the branch changes the files of the worktree under the user's - // feet, so only do it while they have no changes of their own there - hasChanges, err := self.c.Git().WorkingTree.HasChangesToTrackedFiles(worktreeGitDir, worktreePath) + // Moving the branch to its upstream means giving up the commits it is + // ahead by, so make sure that none of them is ours + hasLocalOnlyCommits, err := self.c.Git().Branch.HasLocalOnlyCommits(f.branch) if err != nil { return err } - if hasChanges { + if hasLocalOnlyCommits { return errors.New(utils.ResolvePlaceholderString( - self.c.Tr.FwdUncommittedChanges, - map[string]string{"branchName": branch.Name}, + self.c.Tr.FwdLocalOnlyCommits, + map[string]string{"branchName": f.branch.Name}, )) } - return self.c.Git().WorkingTree.ResetKeep( - branch.FullUpstreamRefName(), worktreeGitDir, worktreePath) + if f.worktree != nil { + // Resetting the branch changes the files of the worktree under the + // user's feet, so only do it while they have no changes of their own + // there + worktreeGitDir, worktreePath := self.worktreeArgs(f.worktree) + hasChanges, err := self.c.Git().WorkingTree.HasChangesToTrackedFiles(worktreeGitDir, worktreePath) + if err != nil { + return err + } + if hasChanges { + return errors.New(utils.ResolvePlaceholderString( + self.c.Tr.FwdUncommittedChanges, + map[string]string{"branchName": f.branch.Name}, + )) + } + } + + return nil +} + +func (self *BranchesHelper) forwardBranches(toForward []*branchToForward) error { + // The branches that aren't checked out anywhere are nothing but refs to + // update, so they can all be done in one go + updateCommands := "" + for _, f := range toForward { + if f.worktree == nil { + updateCommands += fmt.Sprintf("update %s %s %s\n", + f.branch.FullRefName(), f.branch.FullUpstreamRefName(), f.branch.CommitHash) + } + } + + if updateCommands != "" { + self.c.LogCommand(strings.TrimRight(updateCommands, "\n"), false) + if err := self.c.Git().Branch.UpdateBranchRefs(updateCommands); err != nil { + return err + } + } + + // A branch that is checked out somewhere needs the files of that worktree + // to be updated along with it + for _, f := range toForward { + if f.worktree == nil { + continue + } + + worktreeGitDir, worktreePath := self.worktreeArgs(f.worktree) + + var err error + if f.reset { + err = self.c.Git().WorkingTree.ResetKeep( + f.branch.FullUpstreamRefName(), worktreeGitDir, worktreePath) + } else { + err = self.c.Git().Branch.FastForwardMerge( + f.branch.FullUpstreamRefName(), worktreeGitDir, worktreePath) + } + if err != nil { + return err + } + } + + return nil +} + +// Returns the git dir and the path to pass for the given worktree; both are +// empty for the current one, which git commands use by default anyway. +func (self *BranchesHelper) worktreeArgs(worktree *models.Worktree) (string, string) { + if worktree.IsCurrent { + return "", "" + } + + return worktree.GitDir, worktree.Path } func (self *BranchesHelper) AutoForwardBranches(background bool) error { diff --git a/pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches.go b/pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches.go new file mode 100644 index 000000000..64d4928c2 --- /dev/null +++ b/pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches.go @@ -0,0 +1,45 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FastForwardRewrittenStackOfBranches = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Fast-forward a whole stack of branches whose upstream branches were rewritten", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "alphabetical" + }, + SetupRepo: func(shell *Shell) { + createStackRewrittenOnTheRemote(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master").IsSelected(), + Contains("branch1 ↓1↑1"), + Contains("branch2 ↓2↑2"), + Contains("branch3 ↓3↑3"), + ). + SelectNextItem(). + Press(keys.Universal.ToggleRangeSelect). + SelectNextItem(). + SelectNextItem(). + Lines( + Contains("master"), + Contains("branch1 ↓1↑1").IsSelected(), + Contains("branch2 ↓2↑2").IsSelected(), + Contains("branch3 ↓3↑3").IsSelected(), + ). + Press(keys.Branches.FastForward). + Lines( + Contains("master"), + Contains("branch1 ✓").IsSelected(), + Contains("branch2 ✓").IsSelected(), + Contains("branch3 ✓").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches_with_local_commits.go b/pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches_with_local_commits.go new file mode 100644 index 000000000..2b68155cb --- /dev/null +++ b/pkg/integration/tests/sync/fast_forward_rewritten_stack_of_branches_with_local_commits.go @@ -0,0 +1,50 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FastForwardRewrittenStackOfBranchesWithLocalCommits = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Try to fast-forward a stack of branches of which one has a commit of its own", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "alphabetical" + }, + SetupRepo: func(shell *Shell) { + createStackRewrittenOnTheRemote(shell) + + shell.Checkout("branch2") + shell.EmptyCommit("mine") + shell.Checkout("master") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master").IsSelected(), + Contains("branch1 ↓1↑1"), + Contains("branch2 ↓2↑3"), + Contains("branch3 ↓3↑3"), + ). + SelectNextItem(). + Press(keys.Universal.ToggleRangeSelect). + SelectNextItem(). + SelectNextItem(). + Press(keys.Branches.FastForward) + + t.ExpectPopup().Alert().Title(Equals("Error")). + Content(Contains("Cannot fast-forward 'branch2' because it has commits")). + Confirm() + + // None of them was touched, not even the ones we could have forwarded + t.Views().Branches(). + Lines( + Contains("master"), + Contains("branch1 ↓1↑1").IsSelected(), + Contains("branch2 ↓2↑3").IsSelected(), + Contains("branch3 ↓3↑3").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/sync/shared.go b/pkg/integration/tests/sync/shared.go index 5b144e328..d753bbd45 100644 --- a/pkg/integration/tests/sync/shared.go +++ b/pkg/integration/tests/sync/shared.go @@ -80,6 +80,47 @@ func createBranchRewrittenOnTheRemote(shell *Shell) { shell.Checkout("master") } +// Creates the branches branch1, branch2 and branch3, each on top of the +// previous one and pushed to origin, and then rewrites their commits and +// force-pushes them the way somebody else rebasing the stack would. The local +// branches stay where they were, so all of them have diverged from their +// remote branches without having any commits of their own. Leaves master +// checked out. +func createStackRewrittenOnTheRemote(shell *Shell) { + shell.EmptyCommit("base") + shell.NewBranch("branch1") + shell.EmptyCommit("one") + shell.NewBranch("branch2") + shell.EmptyCommit("two") + shell.NewBranch("branch3") + shell.EmptyCommit("three") + + shell.CloneIntoRemote("origin") + shell.SetBranchUpstream("branch1", "origin/branch1") + shell.SetBranchUpstream("branch2", "origin/branch2") + shell.SetBranchUpstream("branch3", "origin/branch3") + + shell.CreateLightweightTag("before-rewrite", "branch3") + shell.Checkout("branch1") + shell.HardReset("master") + shell.EmptyCommit("one-rewritten") + shell.Checkout("branch2") + shell.HardReset("branch1") + shell.EmptyCommit("two-rewritten") + shell.Checkout("branch3") + shell.HardReset("branch2") + shell.EmptyCommit("three-rewritten") + shell.RunCommand([]string{"git", "push", "--force", "origin", "branch1", "branch2", "branch3"}) + + // Put the local branches back where they were + shell.HardReset("before-rewrite") + shell.RunCommand([]string{"git", "branch", "-f", "branch2", "before-rewrite~"}) + shell.RunCommand([]string{"git", "branch", "-f", "branch1", "before-rewrite~2"}) + shell.RunCommand([]string{"git", "tag", "-d", "before-rewrite"}) + + shell.Checkout("master") +} + func assertSuccessfullyPushed(t *TestDriver) { t.Views().Status().Content(Equals("✓ repo → master")) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 9ccee7912..a25535ff6 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -483,6 +483,8 @@ var tests = []*components.IntegrationTest{ sync.FastForwardRewrittenBranchWithStaleSubmodule, sync.FastForwardRewrittenBranchWithUncommittedChanges, sync.FastForwardRewrittenBranchWithoutReflogs, + sync.FastForwardRewrittenStackOfBranches, + sync.FastForwardRewrittenStackOfBranchesWithLocalCommits, sync.FetchAndAutoForwardBranchesAllBranches, sync.FetchAndAutoForwardBranchesAllBranchesCheckedOutInOtherWorktree, sync.FetchAndAutoForwardBranchesNone,