From 667df9e78490958455c1a6c382be9830ff711069 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 23 Sep 2026 13:49:02 +0200 Subject: [PATCH] Split fast-forwarding a branch into fetching and updating it Fast-forwarding a branch that isn't checked out ran a single `git fetch refs/heads/:`, so git's own refusal to move a local branch backwards served as the safety check. A later commit widens the set of branches the command accepts, and for that lazygit has to make that decision itself. Fetch the remote branch on its own, updating only its remote-tracking branch. Then ask git whether the branch is an ancestor of it, and only then move the branch: with `git update-ref` when it isn't checked out anywhere, guarded by the hash we knew it to have, and with `git merge --ff-only` in its worktree when it is checked out there. Keeping `git pull --ff-only` for the latter case would fetch a second time. Fast-forwarding a branch that isn't checked out in a worktree had no integration test at all, so add one. The second new test covers a repo that keeps no reflogs, because a later commit starts reading those and this case has to keep working without them. Co-Authored-By: Claude Opus 5 (1M context) --- pkg/commands/git_commands/branch.go | 14 +++++ pkg/commands/git_commands/sync.go | 13 ++++- .../controllers/helpers/branches_helper.go | 53 +++++++++-------- .../tests/sync/fast_forward_behind_branch.go | 49 ++++++++++++++++ .../fast_forward_behind_branch_checked_out.go | 57 +++++++++++++++++++ ...t_forward_behind_branch_without_reflogs.go | 44 ++++++++++++++ pkg/integration/tests/test_list.go | 3 + 7 files changed, 207 insertions(+), 26 deletions(-) create mode 100644 pkg/integration/tests/sync/fast_forward_behind_branch.go create mode 100644 pkg/integration/tests/sync/fast_forward_behind_branch_checked_out.go create mode 100644 pkg/integration/tests/sync/fast_forward_behind_branch_without_reflogs.go diff --git a/pkg/commands/git_commands/branch.go b/pkg/commands/git_commands/branch.go index 8f5fb2d8c..6d060613a 100644 --- a/pkg/commands/git_commands/branch.go +++ b/pkg/commands/git_commands/branch.go @@ -285,6 +285,20 @@ func (self *BranchCommands) Merge(branchName string, variant MergeVariant) error return self.cmd.New(cmdArgs).Run() } +// Fast-forwards the branch that is checked out in the given worktree to the +// given ref. Fails if that can't be done without a merge commit. Pass empty +// strings for the worktree to use the current one. +func (self *BranchCommands) FastForwardMerge(refName string, worktreeGitDir string, worktreePath string) error { + cmdArgs := NewGitCmd("merge"). + Arg("--ff-only"). + Arg(refName). + GitDirIf(worktreeGitDir != "", worktreeGitDir). + WorktreePathIf(worktreePath != "", worktreePath). + ToArgv() + + return self.cmd.New(cmdArgs).Run() +} + // Returns whether the first ref is an ancestor of the second one, which also // means that the second one can be fast-forward merged into the first one func (self *BranchCommands) IsAncestor(ancestorRefName string, refName string) bool { diff --git a/pkg/commands/git_commands/sync.go b/pkg/commands/git_commands/sync.go index 1a20a3293..c11846c8d 100644 --- a/pkg/commands/git_commands/sync.go +++ b/pkg/commands/git_commands/sync.go @@ -1,6 +1,8 @@ package git_commands import ( + "fmt" + "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/gocui" @@ -111,15 +113,20 @@ func (self *SyncCommands) Pull(task gocui.Task, opts PullOptions) error { return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).Run() } -func (self *SyncCommands) FastForward( +// 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( task gocui.Task, - branchName string, remoteName string, remoteBranchName string, ) error { cmdArgs := self.fetchCommandBuilder(false). Arg(remoteName). - Arg("refs/heads/" + remoteBranchName + ":" + branchName). + // 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)). ToArgv() return self.cmd.New(cmdArgs).PromptOnCredentialRequest(task).Run() diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 75cb788ca..feb513736 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -434,14 +434,34 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool, fe return fetchErr } +// Updates the given branch to its upstream branch, fetching that first. The +// branch must not have any commits that its upstream doesn't have, so that +// nothing is lost. func (self *BranchesHelper) FastForwardBranch(branch *models.Branch) error { - action := self.c.Tr.Actions.FastForwardBranch - worktree, ok := self.worktreeForBranch(branch) + worktree, checkedOut := self.worktreeForBranch(branch) return self.c.WithInlineStatus(branch, types.ItemOperationFastForwarding, context.LOCAL_BRANCHES_CONTEXT_KEY, func(task gocui.Task) error { - if ok { - self.c.LogAction(action) + defer func() { + if checkedOut { + // The files of that worktree have changed as well + self.c.RefreshFromWorker(types.RefreshOptions{}) + } else { + self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}}) + } + }() + self.c.LogAction(self.c.Tr.Actions.FastForwardBranch) + + err := self.c.Git().Sync.FetchRemoteBranch(task, branch.UpstreamRemote, branch.UpstreamBranch) + if err != nil { + return err + } + + if !self.c.Git().Branch.IsAncestor(branch.FullRefName(), branch.FullUpstreamRefName()) { + return errors.New(self.c.Tr.FwdCommitsToPush) + } + + if checkedOut { worktreeGitDir := "" worktreePath := "" // if it is the current worktree path, no need to specify the path @@ -450,27 +470,14 @@ func (self *BranchesHelper) FastForwardBranch(branch *models.Branch) error { worktreePath = worktree.Path } - err := self.c.Git().Sync.Pull( - task, - git_commands.PullOptions{ - RemoteName: branch.UpstreamRemote, - BranchName: branch.UpstreamBranch, - FastForwardOnly: true, - WorktreeGitDir: worktreeGitDir, - WorktreePath: worktreePath, - }, - ) - self.c.RefreshFromWorker(types.RefreshOptions{}) - return err + return self.c.Git().Branch.FastForwardMerge( + branch.FullUpstreamRefName(), worktreeGitDir, worktreePath) } - self.c.LogAction(action) - - err := self.c.Git().Sync.FastForward( - task, branch.Name, branch.UpstreamRemote, branch.UpstreamBranch, - ) - self.c.RefreshFromWorker(types.RefreshOptions{Scope: []types.RefreshableView{types.BRANCHES}}) - return err + 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") }) } diff --git a/pkg/integration/tests/sync/fast_forward_behind_branch.go b/pkg/integration/tests/sync/fast_forward_behind_branch.go new file mode 100644 index 000000000..9b767c4de --- /dev/null +++ b/pkg/integration/tests/sync/fast_forward_behind_branch.go @@ -0,0 +1,49 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FastForwardBehindBranch = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Fast-forward a branch that is behind its upstream branch, without checking it out", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("one") + shell.NewBranch("feature") + shell.EmptyCommit("two") + shell.EmptyCommit("three") + + shell.CloneIntoRemote("origin") + shell.SetBranchUpstream("feature", "origin/feature") + + // remove the two commits so that there is something to fast-forward to + shell.HardReset("HEAD~2") + shell.Checkout("master") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master").IsSelected(), + Contains("feature ↓2"), + ). + SelectNextItem(). + Press(keys.Branches.FastForward). + Lines( + Contains("master"), + Contains("feature ✓").IsSelected(), + ). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + Lines( + Contains("three"), + Contains("two"), + Contains("one"), + ) + }, +}) diff --git a/pkg/integration/tests/sync/fast_forward_behind_branch_checked_out.go b/pkg/integration/tests/sync/fast_forward_behind_branch_checked_out.go new file mode 100644 index 000000000..57d566a19 --- /dev/null +++ b/pkg/integration/tests/sync/fast_forward_behind_branch_checked_out.go @@ -0,0 +1,57 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FastForwardBehindBranchCheckedOut = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Fast-forward the checked out branch when it is behind its upstream branch", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file", "content") + shell.Commit("one") + shell.EmptyCommit("two") + shell.EmptyCommit("three") + + shell.CloneIntoRemote("origin") + shell.SetBranchUpstream("master", "origin/master") + + // remove the two commits so that there is something to fast-forward to + shell.HardReset("HEAD~2") + + // a change that the fast-forward doesn't get in the way of + shell.UpdateFile("file", "changed") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Lines( + Contains("one"), + ) + + t.Views().Branches(). + Focus(). + Lines( + Contains("master ↓2").IsSelected(), + ). + Press(keys.Branches.FastForward). + Lines( + Contains("master ✓").IsSelected(), + ) + + t.Views().Commits(). + Lines( + Contains("three"), + Contains("two"), + Contains("one"), + ) + + // Moving the branch forward doesn't touch the change + t.Views().Files(). + Lines( + Contains("file"), + ) + }, +}) diff --git a/pkg/integration/tests/sync/fast_forward_behind_branch_without_reflogs.go b/pkg/integration/tests/sync/fast_forward_behind_branch_without_reflogs.go new file mode 100644 index 000000000..364b95be0 --- /dev/null +++ b/pkg/integration/tests/sync/fast_forward_behind_branch_without_reflogs.go @@ -0,0 +1,44 @@ +package sync + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FastForwardBehindBranchWithoutReflogs = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Fast-forward a branch that is behind its upstream branch in a repo that keeps no reflogs", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + // This is the default in a bare repository, so it is what people with a + // bare repo and linked worktrees work with + shell.SetConfig("core.logAllRefUpdates", "false") + + shell.EmptyCommit("one") + shell.NewBranch("feature") + shell.EmptyCommit("two") + shell.EmptyCommit("three") + + shell.CloneIntoRemote("origin") + shell.SetBranchUpstream("feature", "origin/feature") + + // remove the two commits so that there is something to fast-forward to + shell.HardReset("HEAD~2") + shell.Checkout("master") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("master").IsSelected(), + Contains("feature ↓2"), + ). + SelectNextItem(). + Press(keys.Branches.FastForward). + Lines( + Contains("master"), + Contains("feature ✓").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 802993d31..3f7101ab2 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -474,6 +474,9 @@ var tests = []*components.IntegrationTest{ submodule.Stage, submodule.StageAllWithDirtySubmodule, submodule.StageDirtyOnly, + sync.FastForwardBehindBranch, + sync.FastForwardBehindBranchCheckedOut, + sync.FastForwardBehindBranchWithoutReflogs, sync.FetchAndAutoForwardBranchesAllBranches, sync.FetchAndAutoForwardBranchesAllBranchesCheckedOutInOtherWorktree, sync.FetchAndAutoForwardBranchesNone,