Fix index.lock retry logic for rebase and cherry-pick commands (#6195)

Our git command runner has logic to retry a failed git command if it can
determine that the failure was caused by an index.lock held by a
background process. This is great, but it's the wrong thing to do for an
interactive rebase that fails while applying one of its pick todos; in
that case, git stops in the middle of the rebase, and retrying it
results in a confusing error ("a rebase is already in progress").

This is just one of many examples of what can go wrong when retrying
rebase, cherry-pick, or revert commands that fail with an index.lock
error; this PR tries to handle as many situations as reasonable
automatically (e.g. by issuing `git rebase --continue` when a pick
failed and lazygit can confirm that the pick was rescheduled by git).
See the individual commits for details.
This commit is contained in:
Stefan Haller
2026-11-08 15:16:49 +01:00
committed by GitHub
23 changed files with 1581 additions and 81 deletions
+17 -7
View File
@@ -55,14 +55,14 @@ func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error {
}
func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) {
return self.retryOnLockError(func() (string, error) {
return self.retryOnLockError(cmdObj, func(cmdObj *oscommands.CmdObj) (string, error) {
return self.innerRunner.RunWithOutput(cmdObj.Clone())
})
}
func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
var stdout, stderr string
_, err := self.retryOnLockError(func() (string, error) {
_, err := self.retryOnLockError(cmdObj, func(cmdObj *oscommands.CmdObj) (string, error) {
var runErr error
stdout, stderr, runErr = self.innerRunner.RunWithOutputs(cmdObj.Clone())
return stdout + stderr, runErr
@@ -70,23 +70,33 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string,
return stdout, stderr, err
}
// retryOnLockError runs the given function, retrying if it fails with a
// transient lock error (see isRetryableError). The string returned by run is
// retryOnLockError runs the given command with the given function, trying
// again if it fails with a transient lock error (see isRetryableError). If the
// command has a lock-error recovery (see CmdObj.RecoverFromLockErrorWith), the
// recovery decides which command to run next. The string returned by run is
// the command output we inspect to classify the failure. We clone the command
// for each attempt (inside run) because an *exec.Cmd can only be run once.
func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (string, error) {
func (self *gitCmdObjRunner) retryOnLockError(cmdObj *oscommands.CmdObj, run func(*oscommands.CmdObj) (string, error)) (string, error) {
delay := self.initialRetryDelay
var output string
var err error
for attempt := range maxRetries {
output, err = run()
output, err = run(cmdObj)
if err == nil || !isRetryableError(output, err) {
break
}
if recovery := cmdObj.GetLockErrorRecovery(); recovery != nil {
cmdObj = recovery()
if cmdObj == nil {
self.log.Warn("lock error stopped command partway through; giving up")
break
}
}
if attempt < maxRetries-1 {
self.log.Warnf("lock error prevented command from running; retrying in %s", delay)
self.log.Warnf("lock error prevented command from completing; running %q in %s", cmdObj.ToString(), delay)
time.Sleep(delay)
delay *= 2
}
+88 -9
View File
@@ -16,29 +16,32 @@ type runnerResult struct {
// scriptedRunner is an ICmdObjRunner stub that returns a preconfigured result
// for each successive call, letting us drive the retry loop deterministically.
// It counts calls so tests can assert whether a command was retried.
// It counts calls and records the commands it ran, so tests can assert whether
// and how a command was retried.
type scriptedRunner struct {
results []runnerResult
calls int
ran []string
}
func (self *scriptedRunner) next() (string, error) {
func (self *scriptedRunner) next(cmdObj *oscommands.CmdObj) (string, error) {
result := self.results[self.calls]
self.calls++
self.ran = append(self.ran, cmdObj.ToString())
return result.output, result.err
}
func (self *scriptedRunner) Run(*oscommands.CmdObj) error {
_, err := self.next()
func (self *scriptedRunner) Run(cmdObj *oscommands.CmdObj) error {
_, err := self.next(cmdObj)
return err
}
func (self *scriptedRunner) RunWithOutput(*oscommands.CmdObj) (string, error) {
return self.next()
func (self *scriptedRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) {
return self.next(cmdObj)
}
func (self *scriptedRunner) RunWithOutputs(*oscommands.CmdObj) (string, string, error) {
output, err := self.next()
func (self *scriptedRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
output, err := self.next(cmdObj)
return output, "", err
}
@@ -58,9 +61,15 @@ func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner {
// dummyCmdObj returns a throwaway command; only its clonability matters, since
// the scriptedRunner ignores it and returns preconfigured results.
func dummyCmdObj() *oscommands.CmdObj {
return oscommands.NewDummyCmdObjBuilder(nil).New([]string{"git", "status"})
return newCmdObj("git", "status")
}
func newCmdObj(args ...string) *oscommands.CmdObj {
return oscommands.NewDummyCmdObjBuilder(nil).New(args)
}
var errLock = errors.New("fatal: Unable to create '/repo/.git/index.lock': File exists.")
func TestRunWithOutputReturnsSuccessWithoutRetrying(t *testing.T) {
inner := &scriptedRunner{results: []runnerResult{{output: "done", err: nil}}}
@@ -135,3 +144,73 @@ func TestRunWithOutputRetriesLockErrorInLinkedWorktree(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 2, inner.calls)
}
func TestRunWithOutputRunsTheCommandReturnedByTheLockErrorRecovery(t *testing.T) {
inner := &scriptedRunner{results: []runnerResult{
{err: errLock},
{err: errLock},
{output: "done", err: nil},
}}
continueCmdObj := newCmdObj("git", "rebase", "--continue")
cmdObj := newCmdObj("git", "rebase", "--interactive", "main")
cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { return continueCmdObj })
output, err := newTestRunner(inner).RunWithOutput(cmdObj)
assert.NoError(t, err)
assert.Equal(t, "done", output)
// After switching to the continue command, the runner uses that command's
// recovery; it has none, so the runner runs it again.
assert.Equal(t, []string{
"git rebase --interactive main",
"git rebase --continue",
"git rebase --continue",
}, inner.ran)
}
func TestRunWithOutputGivesUpWhenTheLockErrorRecoveryReturnsNil(t *testing.T) {
inner := &scriptedRunner{results: []runnerResult{{err: errLock}}}
cmdObj := dummyCmdObj()
cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj { return nil })
_, err := newTestRunner(inner).RunWithOutput(cmdObj)
assert.Equal(t, errLock, err)
assert.Equal(t, 1, inner.calls)
}
func TestRunWithOutputCallsTheLockErrorRecoveryAfterTheLastAttempt(t *testing.T) {
results := make([]runnerResult, maxRetries)
for i := range results {
results[i] = runnerResult{err: errLock}
}
inner := &scriptedRunner{results: results}
recoveryCalls := 0
cmdObj := dummyCmdObj()
cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj {
recoveryCalls++
return cmdObj
})
_, err := newTestRunner(inner).RunWithOutput(cmdObj)
assert.Error(t, err)
assert.Equal(t, maxRetries, inner.calls)
assert.Equal(t, maxRetries, recoveryCalls)
}
func TestRunWithOutputDoesNotCallTheLockErrorRecoveryForOtherErrors(t *testing.T) {
inner := &scriptedRunner{results: []runnerResult{{output: "boom", err: errors.New("boom")}}}
recoveryCalls := 0
cmdObj := dummyCmdObj()
cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj {
recoveryCalls++
return cmdObj
})
_, err := newTestRunner(inner).RunWithOutput(cmdObj)
assert.Error(t, err)
assert.Equal(t, 1, inner.calls)
assert.Equal(t, 0, recoveryCalls)
}
+4 -1
View File
@@ -275,7 +275,10 @@ func (self *CommitCommands) Revert(hashes []string, isMerge bool) error {
Arg(hashes...).
ToArgv()
return self.cmd.New(cmdArgs).Run()
return abortSequenceAfterLockError(self.cmd.New(cmdArgs), self.repoPaths.WorktreeGitDirPath(), len(hashes),
func() *oscommands.CmdObj {
return self.cmd.New(NewGitCmd("revert").Arg("--abort").ToArgv())
}).Run()
}
// CreateFixupCommit creates a commit that fixes up a previous commit
+10 -34
View File
@@ -259,8 +259,15 @@ func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool
commits := self.getSequencerCommits(hashPool)
if len(commits) > 0 {
// If we have any commits in .git/sequencer/todo, then the last one of
// those is the conflicting one.
commits[len(commits)-1].Status = models.StatusConflicted
// those is the one that git stopped at.
switch getCurrentSequencerTodoState(self.cmd, self.repoPaths.WorktreeGitDirPath()) {
case sequencerTodoStarted:
commits[len(commits)-1].Status = models.StatusConflicted
case sequencerTodoNotStarted:
// It is still pending like the other ones
case sequencerTodoCommitted:
commits = commits[:len(commits)-1]
}
} else {
// For single-commit cherry-picks and reverts, git apparently doesn't
// use the sequencer; in that case, CHERRY_PICK_HEAD or REVERT_HEAD is
@@ -430,38 +437,7 @@ func (self *CommitLoader) getConflictedCommitImpl(hashPool *utils.StringPool, to
return nil
}
// In certain cases, git reschedules commands that failed. One example is if
// a patch would overwrite an untracked file (another one is an "exec" that
// failed, but we don't care about that here because we dealt with exec
// already above). To detect this, compare the last command of the "done"
// file against the first command of "git-rebase-todo"; if they are the
// same, the command was rescheduled.
if len(doneTodos) > 0 && len(todos) > 0 && doneTodos[len(doneTodos)-1] == todos[0] {
// Command was rescheduled, no need to display it
return nil
}
// Older versions of git have a bug whereby, if a command is rescheduled,
// the last successful command is appended to the "done" file again. To
// detect this, we need to compare the second-to-last done entry against the
// first todo entry, and also compare the last done entry against the
// last-but-two done entry; this latter check is needed for the following
// case:
// pick A
// exec make test
// pick B
// exec make test
// If pick B fails with conflicts, then the "done" file contains
// pick A
// exec make test
// pick B
// and git-rebase-todo contains
// exec make test
// Without the last condition we would erroneously treat this as the exec
// command being rescheduled, so we wouldn't display our fake entry for
// "pick B".
if len(doneTodos) >= 3 && len(todos) > 0 && doneTodos[len(doneTodos)-2] == todos[0] &&
doneTodos[len(doneTodos)-1] == doneTodos[len(doneTodos)-3] {
if utils.LastDoneTodoWasRescheduled(doneTodos, todos) {
// Command was rescheduled, no need to display it
return nil
}
@@ -0,0 +1,156 @@
package git_commands
import (
"bytes"
"os"
"path/filepath"
"strings"
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
"github.com/jesseduffield/lazygit/pkg/utils"
)
// resumeRebaseAfterLockError sets the lock-error recovery (see
// CmdObj.RecoverFromLockErrorWith) of cmdObj, a command that starts or
// continues a rebase in the worktree whose git dir is gitDir. Call it right
// before running the command, since it records the state of the rebase at
// that time.
//
// If a lock stops the command before it changes the state of the rebase, we
// run the command again. This includes a command that hasn't started the
// rebase yet, since git cleans up after itself in that case. If a lock stops
// the rebase partway through, git normally reschedules the todo that failed,
// and the command made by newContinueCmdObj ("git rebase --continue") runs it
// again. In some cases git doesn't reschedule it, for example if the lock
// stops a pick after it has applied the changes but before it has committed
// them. Continuing could then lose the commit, so we give up and leave the
// rebase to the user.
func resumeRebaseAfterLockError(
cmdObj *oscommands.CmdObj,
gitDir string,
commentChar byte,
newContinueCmdObj func() *oscommands.CmdObj,
) *oscommands.CmdObj {
var recoverFrom func(cmdObj *oscommands.CmdObj, stateBefore string) *oscommands.CmdObj
recoverFrom = func(cmdObj *oscommands.CmdObj, stateBefore string) *oscommands.CmdObj {
return cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj {
state := rebaseState(gitDir)
if state == stateBefore {
return cmdObj
}
if !wasLastRebaseTodoRescheduled(gitDir, commentChar) {
return nil
}
return recoverFrom(newContinueCmdObj(), state)
})
}
return recoverFrom(cmdObj, rebaseState(gitDir))
}
// abortSequenceAfterLockError sets the lock-error recovery (see
// CmdObj.RecoverFromLockErrorWith) of cmdObj, a cherry-pick or revert of
// numCommits commits in the worktree whose git dir is gitDir.
//
// If a lock stops a cherry-pick or revert of several commits after it has
// applied some of them, git leaves the operation in progress in
// .git/sequencer. Unlike a rebase, it doesn't reschedule the commit that
// failed, and continuing the operation would skip that commit. So we abort the
// operation with the command made by newAbortCmdObj, and run the command
// again. The runner calls the recovery after its last attempt too, so we
// also abort when it gives up, and the user isn't left with an operation that
// stopped halfway.
func abortSequenceAfterLockError(
cmdObj *oscommands.CmdObj,
gitDir string,
numCommits int,
newAbortCmdObj func() *oscommands.CmdObj,
) *oscommands.CmdObj {
// For a single commit, git doesn't use .git/sequencer. If a lock stops it,
// it has either not changed anything yet, or it has staged the changes, and
// running it again fails with an error. Git also doesn't check whether
// another operation is in progress in .git/sequencer, so we must not abort
// that one.
if numCommits < 2 {
return cmdObj
}
return cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj {
// Git refuses to start a cherry-pick or revert of several commits while
// one is in progress, so if there is one now, our command started it.
if dirExists(filepath.Join(gitDir, "sequencer")) {
if err := newAbortCmdObj().Run(); err != nil {
return nil
}
}
return cmdObj
})
}
// retryContinueOrSkipAfterLockError sets the lock-error recovery (see
// CmdObj.RecoverFromLockErrorWith) of cmdObj, a command that continues or skips
// a step of an operation in progress, such as a cherry-pick or revert.
// progressFile is the file in which git records how far the operation has got.
// Call this function right before running the command, since it records the
// contents of progressFile at that time.
//
// If a lock stops the command before it changes progressFile, we run the
// command again. Otherwise it has got past the step that it continued or
// skipped, and the lock stopped it at the next one. Unlike a rebase, git doesn't
// reschedule that step (see abortSequenceAfterLockError), and running the
// command again could skip it. So we give up and leave the operation to the
// user.
func retryContinueOrSkipAfterLockError(cmdObj *oscommands.CmdObj, progressFile string) *oscommands.CmdObj {
progressBefore, _ := os.ReadFile(progressFile)
return cmdObj.RecoverFromLockErrorWith(func() *oscommands.CmdObj {
progress, _ := os.ReadFile(progressFile)
if bytes.Equal(progress, progressBefore) {
return cmdObj
}
return nil
})
}
// rebaseState returns a snapshot of the rebase that is in progress in gitDir,
// made of the files in which git records how far it has got. It's empty if no
// rebase is in progress.
func rebaseState(gitDir string) string {
var state strings.Builder
for _, dir := range []string{"rebase-merge", "rebase-apply"} {
if dirExists(filepath.Join(gitDir, dir)) {
state.WriteString(dir + "\n")
}
}
for _, file := range []string{"rebase-merge/done", "rebase-merge/git-rebase-todo", "rebase-apply/next"} {
if content, err := os.ReadFile(filepath.Join(gitDir, file)); err == nil {
state.WriteString(file + "\n")
state.Write(content)
}
}
return state.String()
}
// wasLastRebaseTodoRescheduled tells whether git put the todo that it executed
// last back into the todo list of the rebase that is in progress in gitDir.
// It's false if no rebase is in progress, or if the rebase doesn't use the
// rebase-merge directory; that kind of rebase never reschedules anything.
func wasLastRebaseTodoRescheduled(gitDir string, commentChar byte) bool {
doneTodos, err := utils.ReadRebaseTodoFile(filepath.Join(gitDir, "rebase-merge", "done"), commentChar)
if err != nil {
return false
}
todos, err := utils.ReadRebaseTodoFile(filepath.Join(gitDir, "rebase-merge", "git-rebase-todo"), commentChar)
if err != nil {
return false
}
return utils.LastDoneTodoWasRescheduled(doneTodos, todos)
}
func dirExists(path string) bool {
info, err := os.Stat(path)
return err == nil && info.IsDir()
}
@@ -0,0 +1,229 @@
package git_commands
import (
"errors"
"os"
"path/filepath"
"testing"
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
"github.com/stretchr/testify/assert"
)
func TestResumeRebaseAfterLockError(t *testing.T) {
newCmdObj := func(args ...string) *oscommands.CmdObj {
return oscommands.NewDummyCmdObjBuilder(nil).New(args)
}
writeRebaseMergeFiles := func(gitDir string, done string, todo string) {
dir := filepath.Join(gitDir, "rebase-merge")
assert.NoError(t, os.MkdirAll(dir, 0o755))
assert.NoError(t, os.WriteFile(filepath.Join(dir, "done"), []byte(done), 0o644))
assert.NoError(t, os.WriteFile(filepath.Join(dir, "git-rebase-todo"), []byte(todo), 0o644))
}
// Sets up the recovery for cmdObj, then calls runUntilLockError to do what
// the command did before the lock stopped it, and returns the command that
// the recovery wants to run next.
recoverFromLockError := func(gitDir string, cmdObj *oscommands.CmdObj, runUntilLockError func()) *oscommands.CmdObj {
resumeRebaseAfterLockError(cmdObj, gitDir, '#', func() *oscommands.CmdObj {
return newCmdObj("git", "rebase", "--continue")
})
runUntilLockError()
return cmdObj.GetLockErrorRecovery()()
}
t.Run("runs the command again if it didn't start the rebase", func(t *testing.T) {
cmdObj := newCmdObj("git", "rebase", "--interactive", "main")
assert.Same(t, cmdObj, recoverFromLockError(t.TempDir(), cmdObj, func() {}))
})
t.Run("runs the command again if it didn't change the rebase", func(t *testing.T) {
gitDir := t.TempDir()
writeRebaseMergeFiles(gitDir,
"pick 1234567 one\n",
"pick 2345678 two\n")
cmdObj := newCmdObj("git", "rebase", "--continue")
assert.Same(t, cmdObj, recoverFromLockError(gitDir, cmdObj, func() {}))
})
t.Run("continues the rebase if git rescheduled the todo that failed", func(t *testing.T) {
gitDir := t.TempDir()
next := recoverFromLockError(gitDir, newCmdObj("git", "rebase", "--interactive", "main"), func() {
writeRebaseMergeFiles(gitDir,
"pick 1234567 one\npick 2345678 two\n",
"pick 2345678 two\npick 3456789 three\n")
})
assert.Equal(t, "git rebase --continue", next.ToString())
// If continuing fails before it changes the rebase, we run it again
assert.Same(t, next, next.GetLockErrorRecovery()())
// If continuing fails after it has changed the rebase, and git
// rescheduled the todo that failed, we continue again
writeRebaseMergeFiles(gitDir,
"pick 1234567 one\npick 2345678 two\npick 2345678 two\npick 3456789 three\n",
"pick 3456789 three\n")
nextAfterContinuing := next.GetLockErrorRecovery()()
assert.Equal(t, "git rebase --continue", nextAfterContinuing.ToString())
assert.NotSame(t, next, nextAfterContinuing)
// If the rebase is gone after continuing failed, we give up
assert.NoError(t, os.RemoveAll(filepath.Join(gitDir, "rebase-merge")))
assert.Nil(t, nextAfterContinuing.GetLockErrorRecovery()())
})
t.Run("continues the rebase if an older git version rescheduled the todo that failed", func(t *testing.T) {
gitDir := t.TempDir()
next := recoverFromLockError(gitDir, newCmdObj("git", "rebase", "--interactive", "main"), func() {
writeRebaseMergeFiles(gitDir,
"pick 1234567 one\npick 2345678 two\npick 1234567 one\n",
"pick 2345678 two\npick 3456789 three\n")
})
assert.Equal(t, "git rebase --continue", next.ToString())
})
t.Run("gives up if git didn't reschedule the todo that failed", func(t *testing.T) {
gitDir := t.TempDir()
assert.Nil(t, recoverFromLockError(gitDir, newCmdObj("git", "rebase", "--interactive", "main"), func() {
writeRebaseMergeFiles(gitDir,
"pick 1234567 one\npick 2345678 two\n",
"pick 3456789 three\n")
}))
})
t.Run("gives up if a rebase without a rebase-merge directory is in progress", func(t *testing.T) {
gitDir := t.TempDir()
assert.Nil(t, recoverFromLockError(gitDir, newCmdObj("git", "pull"), func() {
assert.NoError(t, os.MkdirAll(filepath.Join(gitDir, "rebase-apply"), 0o755))
}))
})
}
func TestAbortSequenceAfterLockError(t *testing.T) {
scenarios := []struct {
testName string
numCommits int
sequencerExists bool
runner *oscommands.FakeCmdObjRunner
expectRecovery bool
expectRunningAgain bool
}{
{
testName: "single commit",
numCommits: 1,
runner: oscommands.NewFakeRunner(t),
expectRecovery: false,
},
{
testName: "the lock stopped the command before it started",
numCommits: 2,
runner: oscommands.NewFakeRunner(t),
expectRecovery: true,
expectRunningAgain: true,
},
{
testName: "the lock stopped the command partway through",
numCommits: 2,
sequencerExists: true,
runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"cherry-pick", "--abort"}, "", nil),
expectRecovery: true,
expectRunningAgain: true,
},
{
testName: "aborting fails",
numCommits: 2,
sequencerExists: true,
runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"cherry-pick", "--abort"}, "", errors.New("error")),
expectRecovery: true,
expectRunningAgain: false,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
gitDir := t.TempDir()
if s.sequencerExists {
assert.NoError(t, os.MkdirAll(filepath.Join(gitDir, "sequencer"), 0o755))
}
builder := oscommands.NewDummyCmdObjBuilder(s.runner)
cmdObj := builder.New([]string{"git", "cherry-pick", "1234567", "2345678"})
abortSequenceAfterLockError(cmdObj, gitDir, s.numCommits, func() *oscommands.CmdObj {
return builder.New([]string{"git", "cherry-pick", "--abort"})
})
recovery := cmdObj.GetLockErrorRecovery()
if !s.expectRecovery {
assert.Nil(t, recovery)
} else if s.expectRunningAgain {
assert.Same(t, cmdObj, recovery())
} else {
assert.Nil(t, recovery())
}
s.runner.CheckForMissingCalls()
})
}
}
func TestRetryContinueOrSkipAfterLockError(t *testing.T) {
newCmdObj := func(args ...string) *oscommands.CmdObj {
return oscommands.NewDummyCmdObjBuilder(nil).New(args)
}
writeTodos := func(gitDir string, todos string) {
dir := filepath.Join(gitDir, "sequencer")
assert.NoError(t, os.MkdirAll(dir, 0o755))
assert.NoError(t, os.WriteFile(filepath.Join(dir, "todo"), []byte(todos), 0o644))
}
t.Run("runs the command again if it didn't change the todos", func(t *testing.T) {
gitDir := t.TempDir()
writeTodos(gitDir, "pick 1234567 one\npick 2345678 two\n")
cmdObj := newCmdObj("git", "cherry-pick", "--continue")
retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(gitDir, "sequencer", "todo"))
assert.Same(t, cmdObj, cmdObj.GetLockErrorRecovery()())
})
t.Run("runs the command again for a cherry-pick of a single commit", func(t *testing.T) {
cmdObj := newCmdObj("git", "cherry-pick", "--continue")
retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(t.TempDir(), "sequencer", "todo"))
assert.Same(t, cmdObj, cmdObj.GetLockErrorRecovery()())
})
t.Run("gives up if it changed the todos", func(t *testing.T) {
gitDir := t.TempDir()
writeTodos(gitDir, "pick 1234567 one\npick 2345678 two\n")
cmdObj := newCmdObj("git", "cherry-pick", "--continue")
retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(gitDir, "sequencer", "todo"))
writeTodos(gitDir, "pick 2345678 two\n")
assert.Nil(t, cmdObj.GetLockErrorRecovery()())
})
t.Run("gives up if the cherry-pick is gone", func(t *testing.T) {
gitDir := t.TempDir()
writeTodos(gitDir, "pick 1234567 one\n")
cmdObj := newCmdObj("git", "cherry-pick", "--continue")
retryContinueOrSkipAfterLockError(cmdObj, filepath.Join(gitDir, "sequencer", "todo"))
assert.NoError(t, os.RemoveAll(filepath.Join(gitDir, "sequencer")))
assert.Nil(t, cmdObj.GetLockErrorRecovery()())
})
}
+62 -7
View File
@@ -256,7 +256,7 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract
cmdObj.AddEnvVars("GIT_EDITOR=" + ex)
}
return cmdObj
return self.resumeAfterLockError(cmdObj)
}
// GitRebaseEditTodo runs "git rebase --edit-todo", saving the given todosFileContent to the file
@@ -408,7 +408,7 @@ func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) er
Arg("--interactive", "--rebase-merges", "--autostash", "--autosquash", hashOrRoot).
ToArgv()
return self.runSkipEditorCommand(self.cmd.New(cmdArgs))
return self.resumeAfterLockError(skipEditor(self.cmd.New(cmdArgs))).Run()
}
// BeginInteractiveRebaseForCommit starts an interactive rebase to edit the current
@@ -491,8 +491,36 @@ func (self *RebaseCommands) AbortRebase() error {
// GenericMerge takes a commandType of "merge" or "rebase" and a command of "abort", "skip" or "continue"
// By default we skip the editor in the case where a commit will be made
func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, command string) error {
err := self.runSkipEditorCommand(self.GenericMergeOrRebaseActionCmdObj(commandType, command))
undoRepeatingTodo := func() {}
if command == "continue" && (commandType == "cherry-pick" || commandType == "revert") {
var err error
undoRepeatingTodo, err = repeatCurrentSequencerTodo(
self.cmd, self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar())
if err != nil {
return err
}
}
if commandType == "rebase" && command == "continue" {
if err := self.commitPickThatRebaseStoppedAt(); err != nil {
return err
}
}
cmdObj := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command))
if command == "continue" || command == "skip" {
switch commandType {
case "rebase":
cmdObj = self.resumeAfterLockError(cmdObj)
case "cherry-pick", "revert":
cmdObj = retryContinueOrSkipAfterLockError(cmdObj,
filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer", "todo"))
}
}
err := cmdObj.Run()
if err != nil {
undoRepeatingTodo()
if !strings.Contains(err.Error(), "no rebase in progress") {
return err
}
@@ -513,7 +541,32 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma
return nil
}
func (self *RebaseCommands) runSkipEditorCommand(cmdObj *oscommands.CmdObj) error {
// commitPickThatRebaseStoppedAt commits the changes of a pick if the rebase
// stopped after it applied them, but before it committed them (see
// rebaseStoppedBeforeCommittingPick). "git rebase --continue" refuses to commit
// them in that case. "git cherry-pick --continue" commits them with the author
// and message of the pick, like the rebase would have.
func (self *RebaseCommands) commitPickThatRebaseStoppedAt() error {
if !rebaseStoppedBeforeCommittingPick(self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar()) {
return nil
}
return skipEditor(self.GenericMergeOrRebaseActionCmdObj("cherry-pick", "continue")).Run()
}
// resumeAfterLockError makes cmdObj, a command that starts or continues a
// rebase, resume the rebase if a lock error stops it partway through; see
// resumeRebaseAfterLockError.
func (self *RebaseCommands) resumeAfterLockError(cmdObj *oscommands.CmdObj) *oscommands.CmdObj {
return resumeRebaseAfterLockError(cmdObj, self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar(),
func() *oscommands.CmdObj {
return skipEditor(self.GenericMergeOrRebaseActionCmdObj("rebase", "continue"))
})
}
// skipEditor makes git use lazygit as the editor for the given command; lazygit
// then exits right away, so git uses the file as it is.
func skipEditor(cmdObj *oscommands.CmdObj) *oscommands.CmdObj {
instruction := daemon.NewExitImmediatelyInstruction()
lazyGitPath := oscommands.GetLazygitPath()
return cmdObj.
@@ -523,8 +576,7 @@ func (self *RebaseCommands) runSkipEditorCommand(cmdObj *oscommands.CmdObj) erro
"EDITOR="+lazyGitPath,
"VISUAL="+lazyGitPath,
).
AddEnvVars(daemon.ToEnvVars(instruction)...).
Run()
AddEnvVars(daemon.ToEnvVars(instruction)...)
}
// DiscardOldFileChanges discards changes to a file from an old commit
@@ -576,7 +628,10 @@ func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error {
Arg(lo.Reverse(lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash() }))...).
ToArgv()
return self.cmd.New(cmdArgs).Run()
return abortSequenceAfterLockError(self.cmd.New(cmdArgs), self.repoPaths.WorktreeGitDirPath(), len(commits),
func() *oscommands.CmdObj {
return self.cmd.New(NewGitCmd("cherry-pick").Arg("--abort").ToArgv())
}).Run()
}
func (self *RebaseCommands) DropMergeCommit(commits []*models.Commit, commitIndex int) error {
+1 -1
View File
@@ -89,7 +89,7 @@ func TestRebaseSkipEditorCommand(t *testing.T) {
return true
}, "", nil)
instance := buildRebaseCommands(commonDeps{runner: runner})
err := instance.runSkipEditorCommand(instance.cmd.New(cmdArgs))
err := skipEditor(instance.cmd.New(cmdArgs)).Run()
assert.NoError(t, err)
runner.CheckForMissingCalls()
}
+101
View File
@@ -0,0 +1,101 @@
package git_commands
import (
"bytes"
"os"
"path/filepath"
"strings"
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
"github.com/samber/lo"
)
// sequencerTodoState tells what has happened so far with the todo that a
// cherry-pick or revert of several commits stopped at. This is the first todo
// in .git/sequencer/todo.
type sequencerTodoState int
const (
// Git started to apply the todo, and stopped because of conflicts (or
// because it couldn't commit the result). CHERRY_PICK_HEAD or REVERT_HEAD
// exists, and continuing commits the changes.
sequencerTodoStarted sequencerTodoState = iota
// Git stopped before it started to apply the todo, for example because the
// commit would overwrite an untracked file.
sequencerTodoNotStarted
// The user has committed the result of the todo themselves, for example
// after resolving its conflicts. Continuing skips it.
sequencerTodoCommitted
)
// getCurrentSequencerTodoState returns the state of the todo that the
// cherry-pick or revert of several commits that is in progress in gitDir
// stopped at.
//
// Without CHERRY_PICK_HEAD or REVERT_HEAD, we can tell whether the user has
// committed the todo by whether HEAD has moved since git last tried to apply a
// commit, which git records in .git/sequencer/abort-safety. Git does the same
// when it decides whether "--skip" is possible (see rollback_is_safe in git's
// sequencer.c).
func getCurrentSequencerTodoState(cmd oscommands.ICmdObjBuilder, gitDir string) sequencerTodoState {
for _, file := range []string{"CHERRY_PICK_HEAD", "REVERT_HEAD"} {
if _, err := os.Stat(filepath.Join(gitDir, file)); err == nil {
return sequencerTodoStarted
}
}
// If the file doesn't exist, git compares HEAD to the null hash; an empty
// string does the same for us
abortSafety, _ := os.ReadFile(filepath.Join(gitDir, "sequencer", "abort-safety"))
// If HEAD is unborn, this fails and returns an empty string
head, _ := cmd.New(NewGitCmd("rev-parse").Arg("--verify", "--quiet", "HEAD").ToArgv()).DontLog().RunWithOutput()
if strings.TrimSpace(head) == strings.TrimSpace(string(abortSafety)) {
return sequencerTodoNotStarted
}
return sequencerTodoCommitted
}
// repeatCurrentSequencerTodo prepares the cherry-pick or revert of several
// commits that is in progress in gitDir for "--continue". If git stopped it
// before it started to apply the current todo, "--continue" would skip that
// todo, because git can't tell it apart from a todo that the user has
// committed themselves. So we put a copy of the todo in front of it, and git
// skips the copy instead.
//
// The returned function removes the copy again if git hasn't got as far as
// skipping it, for example because it refused to continue.
func repeatCurrentSequencerTodo(cmd oscommands.ICmdObjBuilder, gitDir string, commentChar byte) (func(), error) {
noop := func() {}
todoPath := filepath.Join(gitDir, "sequencer", "todo")
content, err := os.ReadFile(todoPath)
if err != nil {
// No cherry-pick or revert of several commits is in progress
return noop, nil
}
if getCurrentSequencerTodoState(cmd, gitDir) != sequencerTodoNotStarted {
return noop, nil
}
currentTodo, found := lo.Find(strings.Split(string(content), "\n"), func(line string) bool {
line = strings.TrimSpace(line)
return line != "" && line[0] != commentChar
})
if !found {
return noop, nil
}
newContent := append([]byte(currentTodo+"\n"), content...)
if err := os.WriteFile(todoPath, newContent, 0o644); err != nil {
return nil, err
}
return func() {
if currentContent, err := os.ReadFile(todoPath); err == nil && bytes.Equal(currentContent, newContent) {
_ = os.WriteFile(todoPath, content, 0o644)
}
}, nil
}
+145
View File
@@ -0,0 +1,145 @@
package git_commands
import (
"errors"
"os"
"path/filepath"
"testing"
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
"github.com/stretchr/testify/assert"
)
func TestGetCurrentSequencerTodoState(t *testing.T) {
revParseArgs := []string{"rev-parse", "--verify", "--quiet", "HEAD"}
scenarios := []struct {
testName string
files map[string]string
runner *oscommands.FakeCmdObjRunner
expectedState sequencerTodoState
}{
{
testName: "CHERRY_PICK_HEAD exists",
files: map[string]string{"CHERRY_PICK_HEAD": "1234567\n"},
runner: oscommands.NewFakeRunner(t),
expectedState: sequencerTodoStarted,
},
{
testName: "REVERT_HEAD exists",
files: map[string]string{"REVERT_HEAD": "1234567\n"},
runner: oscommands.NewFakeRunner(t),
expectedState: sequencerTodoStarted,
},
{
testName: "HEAD hasn't moved since git tried to apply a commit",
files: map[string]string{"sequencer/abort-safety": "1234567\n"},
runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "1234567\n", nil),
expectedState: sequencerTodoNotStarted,
},
{
testName: "HEAD has moved since git tried to apply a commit",
files: map[string]string{"sequencer/abort-safety": "1234567\n"},
runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "2345678\n", nil),
expectedState: sequencerTodoCommitted,
},
{
testName: "HEAD is unborn, and there is no abort-safety file",
files: map[string]string{},
runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "", errors.New("error")),
expectedState: sequencerTodoNotStarted,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
gitDir := t.TempDir()
for path, content := range s.files {
fullPath := filepath.Join(gitDir, path)
assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755))
assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644))
}
state := getCurrentSequencerTodoState(oscommands.NewDummyCmdObjBuilder(s.runner), gitDir)
assert.Equal(t, s.expectedState, state)
s.runner.CheckForMissingCalls()
})
}
}
func TestRepeatCurrentSequencerTodo(t *testing.T) {
revParseArgs := []string{"rev-parse", "--verify", "--quiet", "HEAD"}
todos := "pick 1234567 one\npick 2345678 two\n"
repeatedTodos := "pick 1234567 one\npick 1234567 one\npick 2345678 two\n"
setUp := func(t *testing.T, files map[string]string) string {
t.Helper()
gitDir := t.TempDir()
for path, content := range files {
fullPath := filepath.Join(gitDir, path)
assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755))
assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644))
}
return gitDir
}
readTodos := func(t *testing.T, gitDir string) string {
t.Helper()
content, err := os.ReadFile(filepath.Join(gitDir, "sequencer", "todo"))
assert.NoError(t, err)
return string(content)
}
t.Run("no cherry-pick or revert of several commits in progress", func(t *testing.T) {
gitDir := setUp(t, map[string]string{})
runner := oscommands.NewFakeRunner(t)
undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#')
assert.NoError(t, err)
undo()
assert.NoFileExists(t, filepath.Join(gitDir, "sequencer", "todo"))
runner.CheckForMissingCalls()
})
t.Run("git started to apply the current todo", func(t *testing.T) {
gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "CHERRY_PICK_HEAD": "1234567\n"})
runner := oscommands.NewFakeRunner(t)
_, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#')
assert.NoError(t, err)
assert.Equal(t, todos, readTodos(t, gitDir))
runner.CheckForMissingCalls()
})
t.Run("git stopped before it started to apply the current todo", func(t *testing.T) {
gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "sequencer/abort-safety": "abcdef0\n"})
runner := oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "abcdef0\n", nil)
undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#')
assert.NoError(t, err)
assert.Equal(t, repeatedTodos, readTodos(t, gitDir))
// If git didn't touch the todos, undo restores them
undo()
assert.Equal(t, todos, readTodos(t, gitDir))
runner.CheckForMissingCalls()
})
t.Run("undo leaves the todos alone if git has changed them", func(t *testing.T) {
gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "sequencer/abort-safety": "abcdef0\n"})
runner := oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "abcdef0\n", nil)
undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#')
assert.NoError(t, err)
gitsTodos := "pick 2345678 two\n"
assert.NoError(t, os.WriteFile(filepath.Join(gitDir, "sequencer", "todo"), []byte(gitsTodos), 0o644))
undo()
assert.Equal(t, gitsTodos, readTodos(t, gitDir))
runner.CheckForMissingCalls()
})
}
+71 -21
View File
@@ -7,7 +7,9 @@ import (
"time"
"github.com/jesseduffield/lazygit/pkg/commands/models"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/spf13/afero"
"github.com/stefanhaller/git-todo-parser/todo"
)
type StatusCommands struct {
@@ -49,39 +51,87 @@ func (self *StatusCommands) IsInMergeState() (bool, error) {
}
func (self *StatusCommands) IsInCherryPick() (bool, error) {
exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD"))
hasCherryPickHead, err := self.hasCherryPickHead()
if err != nil || hasCherryPickHead {
return hasCherryPickHead, err
}
return self.sequencerCommand() == todo.Pick, nil
}
func (self *StatusCommands) hasCherryPickHead() (bool, error) {
gitDir := self.repoPaths.WorktreeGitDirPath()
exists, err := self.os.FileExists(filepath.Join(gitDir, "CHERRY_PICK_HEAD"))
if err != nil || !exists {
return exists, err
}
// Sometimes, CHERRY_PICK_HEAD is present during rebases even if no
// cherry-pick is in progress. I suppose this is because rebase used to be
// implemented as a series of cherry-picks, so this could be remnants of
// code that is shared between cherry-pick and rebase, or something. The way
// to tell if this is the case is to check for the presence of the
// stopped-sha file, which records the sha of the last pick that was
// executed before the rebase stopped, and seeing if the sha in that file is
// the same as the one in CHERRY_PICK_HEAD.
cherryPickHead, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD"))
cherryPickHead, err := os.ReadFile(filepath.Join(gitDir, "CHERRY_PICK_HEAD"))
if err != nil {
return false, err
}
stoppedSha, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge", "stopped-sha"))
return !cherryPickHeadBelongsToRebase(gitDir, strings.TrimSpace(string(cherryPickHead)), self.config.GetCoreCommentChar()), nil
}
// cherryPickHeadBelongsToRebase tells whether CHERRY_PICK_HEAD, whose value is
// cherryPickHead, belongs to a pick of the rebase that is in progress in
// gitDir, rather than to a cherry-pick. A rebase writes CHERRY_PICK_HEAD when it
// has applied the changes of a pick, and removes it when it has committed them.
// So it is still there if the rebase stopped because the pick conflicted; git
// records the pick in rebase-merge/stopped-sha then. It is also still there if
// git couldn't commit the changes, for example because it couldn't take the
// index lock. Git exits without writing stopped-sha in that case, but the pick
// is the last line of rebase-merge/done.
func cherryPickHeadBelongsToRebase(gitDir string, cherryPickHead string, commentChar byte) bool {
// The hashes in these files may be abbreviated
isPrefixOfCherryPickHead := func(hash string) bool {
return hash != "" && strings.HasPrefix(cherryPickHead, hash)
}
stoppedSha, err := os.ReadFile(filepath.Join(gitDir, "rebase-merge", "stopped-sha"))
if err == nil && isPrefixOfCherryPickHead(strings.TrimSpace(string(stoppedSha))) {
return true
}
doneTodos, err := utils.ReadRebaseTodoFile(filepath.Join(gitDir, "rebase-merge", "done"), commentChar)
return err == nil && len(doneTodos) > 0 && isPrefixOfCherryPickHead(doneTodos[len(doneTodos)-1].Commit)
}
// rebaseStoppedBeforeCommittingPick tells whether the rebase that is in
// progress in gitDir has applied the changes of a pick, but exited before it
// could commit them (see cherryPickHeadBelongsToRebase). "git rebase
// --continue" refuses to commit the changes in that case, because git didn't
// write the message file that it writes when a pick conflicts.
func rebaseStoppedBeforeCommittingPick(gitDir string, commentChar byte) bool {
cherryPickHead, err := os.ReadFile(filepath.Join(gitDir, "CHERRY_PICK_HEAD"))
if err != nil {
// If we get an error we assume the file doesn't exist
return true, nil
return false
}
cherryPickHeadStr := strings.TrimSpace(string(cherryPickHead))
stoppedShaStr := strings.TrimSpace(string(stoppedSha))
// Need to use HasPrefix here because the cherry-pick HEAD is a full sha1,
// but stopped-sha is an abbreviated sha1
if strings.HasPrefix(cherryPickHeadStr, stoppedShaStr) {
return false, nil
if _, err := os.Stat(filepath.Join(gitDir, "rebase-merge", "message")); err == nil {
return false
}
return true, nil
return cherryPickHeadBelongsToRebase(gitDir, strings.TrimSpace(string(cherryPickHead)), commentChar)
}
func (self *StatusCommands) IsInRevert() (bool, error) {
return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD"))
hasRevertHead, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD"))
if err != nil || hasRevertHead {
return hasRevertHead, err
}
return self.sequencerCommand() == todo.Revert, nil
}
// sequencerCommand returns the command of the first todo of a cherry-pick or
// revert of several commits that is in progress, or 0 if there is none. Git
// keeps the todos of such an operation in .git/sequencer. If it stops before it
// starts to apply a commit (for example because the commit would overwrite an
// untracked file), it doesn't write CHERRY_PICK_HEAD or REVERT_HEAD. Like git
// status, we still consider the cherry-pick or revert to be in progress then.
func (self *StatusCommands) sequencerCommand() todo.TodoCommand {
todos, err := utils.ReadRebaseTodoFile(
filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer", "todo"), self.config.GetCoreCommentChar())
if err != nil || len(todos) == 0 {
return 0
}
return todos[0].Command
}
// RefsSnapshot returns a string fingerprint of the current state of local
+133
View File
@@ -1,6 +1,8 @@
package git_commands
import (
"os"
"path/filepath"
"testing"
"github.com/go-errors/errors"
@@ -89,3 +91,134 @@ func TestStatusRefsSnapshot(t *testing.T) {
})
}
}
func TestStatusWorkingTreeStateOfCherryPickOrRevert(t *testing.T) {
scenarios := []struct {
testName string
files map[string]string
expectedCherryPicking bool
expectedReverting bool
}{
{
testName: "nothing in progress",
files: map[string]string{},
},
{
testName: "CHERRY_PICK_HEAD",
files: map[string]string{"CHERRY_PICK_HEAD": "1234567\n"},
expectedCherryPicking: true,
},
{
testName: "REVERT_HEAD",
files: map[string]string{"REVERT_HEAD": "1234567\n"},
expectedReverting: true,
},
{
testName: "CHERRY_PICK_HEAD of a rebase that stopped because of conflicts",
files: map[string]string{
"CHERRY_PICK_HEAD": "2345678abcdef\n",
"rebase-merge/stopped-sha": "2345678\n",
"rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n",
},
expectedCherryPicking: false,
},
{
testName: "CHERRY_PICK_HEAD of a rebase that couldn't commit a pick",
files: map[string]string{
"CHERRY_PICK_HEAD": "2345678abcdef\n",
"rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n",
},
expectedCherryPicking: false,
},
{
testName: "cherry-pick while a rebase is stopped",
files: map[string]string{
"CHERRY_PICK_HEAD": "3456789abcdef\n",
"rebase-merge/done": "pick 1234567abcdef one\nedit 2345678abcdef two\n",
},
expectedCherryPicking: true,
},
{
testName: "cherry-pick of several commits that stopped before applying one",
files: map[string]string{"sequencer/todo": "pick 1234567 one\npick 2345678 two\n"},
expectedCherryPicking: true,
},
{
testName: "revert of several commits that stopped before applying one",
files: map[string]string{"sequencer/todo": "revert 1234567 one\nrevert 2345678 two\n"},
expectedReverting: true,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
repoDir := t.TempDir()
for path, content := range s.files {
fullPath := filepath.Join(repoDir, ".git", path)
assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755))
assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644))
}
instance := buildStatusCommands(commonDeps{repoPaths: MockRepoPaths(repoDir)})
state := instance.WorkingTreeState()
assert.Equal(t, s.expectedCherryPicking, state.CherryPicking)
assert.Equal(t, s.expectedReverting, state.Reverting)
})
}
}
func TestRebaseStoppedBeforeCommittingPick(t *testing.T) {
scenarios := []struct {
testName string
files map[string]string
expected bool
}{
{
testName: "the rebase couldn't commit a pick",
files: map[string]string{
"CHERRY_PICK_HEAD": "2345678abcdef\n",
"rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n",
},
expected: true,
},
{
testName: "the rebase stopped because a pick conflicted",
files: map[string]string{
"CHERRY_PICK_HEAD": "2345678abcdef\n",
"rebase-merge/stopped-sha": "2345678\n",
"rebase-merge/message": "two\n",
"rebase-merge/done": "pick 1234567abcdef one\npick 2345678abcdef two\n",
},
expected: false,
},
{
testName: "no CHERRY_PICK_HEAD",
files: map[string]string{
"rebase-merge/done": "pick 1234567abcdef one\nedit 2345678abcdef two\n",
},
expected: false,
},
{
testName: "CHERRY_PICK_HEAD of a cherry-pick while a rebase is stopped",
files: map[string]string{
"CHERRY_PICK_HEAD": "3456789abcdef\n",
"rebase-merge/done": "pick 1234567abcdef one\nedit 2345678abcdef two\n",
},
expected: false,
},
}
for _, s := range scenarios {
t.Run(s.testName, func(t *testing.T) {
gitDir := t.TempDir()
for path, content := range s.files {
fullPath := filepath.Join(gitDir, path)
assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755))
assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644))
}
assert.Equal(t, s.expected, rebaseStoppedBeforeCommittingPick(gitDir, '#'))
})
}
}
+13 -1
View File
@@ -111,7 +111,19 @@ func (self *SyncCommands) Pull(task gocui.Task, opts PullOptions) error {
// setting GIT_SEQUENCE_EDITOR to ':' as a way of skipping it, in case the user
// has 'pull.rebase = interactive' configured.
return self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task).Run()
cmdObj := self.cmd.New(cmdArgs).AddEnvVars("GIT_SEQUENCE_EDITOR=:").PromptOnCredentialRequest(task)
// With pull.rebase set, a lock error can stop the pull partway through its
// rebase.
gitDir := lo.Ternary(opts.WorktreeGitDir != "", opts.WorktreeGitDir, self.repoPaths.WorktreeGitDirPath())
return resumeRebaseAfterLockError(cmdObj, gitDir, self.config.GetCoreCommentChar(), func() *oscommands.CmdObj {
continueArgs := NewGitCmd("rebase").
Arg("--continue").
GitDirIf(opts.WorktreeGitDir != "", opts.WorktreeGitDir).
WorktreePathIf(opts.WorktreePath != "", opts.WorktreePath).
ToArgv()
return skipEditor(self.cmd.New(continueArgs))
}).Run()
}
// Fetches the given branches of the given remote, updating their
+24
View File
@@ -37,6 +37,9 @@ type CmdObj struct {
// can be set so that we don't run certain commands simultaneously
mutex *deadlock.Mutex
// see RecoverFromLockErrorWith()
lockErrorRecovery func() *CmdObj
}
type CredentialStrategy int
@@ -201,6 +204,27 @@ func (self *CmdObj) WithMutex(mutex *deadlock.Mutex) *CmdObj {
return self
}
// RecoverFromLockErrorWith sets what the git command runner does when this
// command fails because another git process holds a lock (see
// gitCmdObjRunner.retryOnLockError). Without it, the runner runs the command
// again. That is wrong for a command that starts an operation of several
// steps, such as a rebase, if the lock only stopped it partway through.
//
// The runner calls recovery after each failed attempt, including the last
// one, so that recovery can also clean up after the command. It returns the
// command to run next: this one to run it again, a different one (for
// example, one that resumes the operation), or nil to give up and return the
// error.
func (self *CmdObj) RecoverFromLockErrorWith(recovery func() *CmdObj) *CmdObj {
self.lockErrorRecovery = recovery
return self
}
func (self *CmdObj) GetLockErrorRecovery() func() *CmdObj {
return self.lockErrorRecovery
}
// runs the command and returns an error if any
func (self *CmdObj) Run() error {
return self.runner.Run(self)
@@ -0,0 +1,84 @@
package cherry_pick
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
)
var CherryPickConflictsCommittedByUser = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Cherry pick two commits, the first of which conflicts; the user commits the resolution themselves before continuing",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {
config.GetUserConfig().Git.LocalBranchSortOrder = "recency"
},
SetupRepo: func(shell *Shell) {
shared.MergeConflictsSetup(shell)
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Branches().
Focus().
Lines(
Contains("first-change-branch"),
Contains("second-change-branch"),
Contains("original-branch"),
).
SelectNextItem().
PressEnter()
t.Views().SubCommits().
IsFocused().
TopLines(
Contains("second-change-branch unrelated change"),
Contains("second change"),
).
Press(keys.Commits.CherryPickCopy).
SelectNextItem().
Press(keys.Commits.CherryPickCopy)
t.Views().Information().Content(Contains("2 commits copied"))
t.Views().Commits().
Focus().
TopLines(
Contains("first change").IsSelected(),
).
Press(keys.Commits.PasteCommits)
t.ExpectPopup().Alert().
Title(Equals("Cherry-pick")).
Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")).
Confirm()
t.Common().AcknowledgeConflicts()
t.Shell().
UpdateFileAndAdd("file", shared.SecondChangeFileContent).
RunCommand([]string{"git", "commit", "--no-edit"})
t.GlobalPress(keys.Universal.Refresh)
t.Views().Status().Content(Equals("(cherry-picking) repo → first-change-branch"))
t.Views().Commits().
TopLines(
Contains("─── Pending cherry-picks"),
Contains("pick").DoesNotContain("CONFLICT").Contains("second-change-branch unrelated change"),
Contains("─── Commits"),
Contains("second change"),
Contains("first change"),
)
t.Common().ContinueOnConflictsResolved("cherry-pick")
t.Views().Status().Content(Equals("repo → first-change-branch"))
t.Views().Commits().
TopLines(
Contains("second-change-branch unrelated change"),
Contains("second change"),
Contains("first change"),
)
},
})
@@ -0,0 +1,96 @@
package cherry_pick
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Cherry pick two commits; an untracked file stops the second one before git starts to apply it; continue after removing the file",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {
config.GetUserConfig().Git.LocalBranchSortOrder = "recency"
},
SetupRepo: func(shell *Shell) {
shell.
CreateFileAndAdd("file", "original\n").
Commit("base").
NewBranch("source").
CreateFileAndAdd("file1", "one\n").
Commit("one").
CreateFileAndAdd("file2", "two\n").
Commit("two").
Checkout("master").
CreateFile("file2", "untracked\n")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Branches().
Focus().
Lines(
Contains("master"),
Contains("source"),
).
SelectNextItem().
PressEnter()
t.Views().SubCommits().
IsFocused().
Lines(
Contains("two").IsSelected(),
Contains("one"),
Contains("base"),
).
Press(keys.Commits.CherryPickCopy).
SelectNextItem().
Press(keys.Commits.CherryPickCopy)
t.Views().Information().Content(Contains("2 commits copied"))
t.Views().Commits().
Focus().
Lines(
Contains("base").IsSelected(),
).
Press(keys.Commits.PasteCommits).
Tap(func() {
t.ExpectPopup().Alert().
Title(Equals("Cherry-pick")).
Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")).
Confirm()
}).
Tap(func() {
t.ExpectPopup().Alert().
Title(Equals("Error")).
Content(Contains("The following untracked working tree files would be overwritten by merge")).
Confirm()
}).
Lines(
Contains("─── Pending cherry-picks"),
Contains("pick").DoesNotContain("CONFLICT").Contains("two"),
Contains("─── Commits"),
Contains("one"),
Contains("base"),
)
t.Views().Status().
Content(Equals("(cherry-picking) repo → master"))
t.Shell().DeleteFile("file2")
t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu)
t.ExpectPopup().Menu().
Title(Equals("Cherry-pick options")).
Select(Contains("continue")).
Confirm()
t.Views().Status().Content(Equals("repo → master"))
t.Views().Commits().
Lines(
Contains("two"),
Contains("one"),
Contains("base"),
)
},
})
@@ -0,0 +1,72 @@
package cherry_pick
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
)
var LockErrorPartwayThroughCherryPick = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Cherry pick two commits while another git process takes index.lock partway through",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {
config.GetUserConfig().Git.LocalBranchSortOrder = "recency"
},
SetupRepo: func(shell *Shell) {
shell.
CreateFileAndAdd("file", "original\n").
Commit("base").
NewBranch("source").
CreateFileAndAdd("file1", "one\n").
Commit("one").
CreateFileAndAdd("file2", "two\n").
Commit("two").
Checkout("master")
// The cherry-pick commits "one"; the lock then stops it when it tries
// to pick "two".
shared.HoldIndexLockAfterNextCommit(shell)
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Branches().
Focus().
Lines(
Contains("master"),
Contains("source"),
).
SelectNextItem().
PressEnter()
t.Views().SubCommits().
IsFocused().
Lines(
Contains("two").IsSelected(),
Contains("one"),
Contains("base"),
).
Press(keys.Commits.CherryPickCopy).
SelectNextItem().
Press(keys.Commits.CherryPickCopy)
t.Views().Information().Content(Contains("2 commits copied"))
t.Views().Commits().
Focus().
Lines(
Contains("base").IsSelected(),
).
Press(keys.Commits.PasteCommits).
Tap(func() {
t.ExpectPopup().Alert().
Title(Equals("Cherry-pick")).
Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")).
Confirm()
}).
Lines(
Contains("two"),
Contains("one"),
Contains("base"),
)
},
})
@@ -0,0 +1,44 @@
package interactive_rebase
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
)
var LockErrorPartwayThroughRebase = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Drops a commit while another git process takes index.lock partway through the rebase",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.CreateNCommits(4)
// The rebase makes a commit when it picks commit-03; the lock then
// stops it when it tries to pick commit-04.
shared.HoldIndexLockAfterNextCommit(shell)
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("commit-04").IsSelected(),
Contains("commit-03"),
Contains("commit-02"),
Contains("commit-01"),
).
NavigateToLine(Contains("commit-02")).
Press(keys.Universal.Remove).
Tap(func() {
t.ExpectPopup().Confirmation().
Title(Equals("Drop commit")).
Content(Contains("Are you sure you want to drop the selected commit(s)?")).
Confirm()
}).
IsFocused().
Lines(
Contains("commit-04"),
Contains("commit-03"),
Contains("commit-01"),
)
},
})
@@ -0,0 +1,84 @@
package interactive_rebase
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
)
var LockErrorPartwayThroughSkip = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Skip in a rebase while another git process takes index.lock before the next commit is committed",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.
CreateFileAndAdd("file", "original\n").
Commit("base").
UpdateFileAndAdd("file", "changed\n").
Commit("commit to edit").
CreateFileAndAdd("file-y", "y\n").
Commit("add file-y").
CreateFileAndAdd("file-z", "z\n").
Commit("add file-z")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("add file-z").IsSelected(),
Contains("add file-y"),
Contains("commit to edit"),
Contains("base"),
).
NavigateToLine(Contains("commit to edit")).
Press(keys.Universal.Edit).
Lines(
Contains("─── Pending rebase todos"),
Contains("pick").Contains("add file-z"),
Contains("pick").Contains("add file-y"),
Contains("─── Commits"),
Contains("commit to edit").IsSelected(),
Contains("base"),
)
// After amending the commit we stopped at, git has to make a new commit
// when it picks "add file-y", rather than fast-forwarding to it. The
// lock then stops it after it has applied the changes, but before it
// has committed them.
t.Shell().RunCommand([]string{"git", "commit", "--amend", "-m", "edited commit"})
shared.HoldIndexLockBeforeNextPickIsCommitted(t.Shell())
t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu)
t.ExpectPopup().Menu().
Title(Equals("Rebase options")).
Select(Contains("skip")).
Confirm()
t.ExpectPopup().Alert().
Title(Equals("Error")).
Content(Contains("index.lock': File exists.")).
Confirm()
t.Views().Files().
Lines(
Contains("A").Contains("file-y"),
)
t.Views().Status().Content(Contains("(rebasing)"))
t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu)
t.ExpectPopup().Menu().
Title(Equals("Rebase options")).
Select(Contains("continue")).
Confirm()
t.Views().Commits().
Lines(
Contains("add file-z"),
Contains("add file-y"),
Contains("edited commit"),
Contains("base"),
)
},
})
@@ -0,0 +1,44 @@
package shared
import (
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
// holdIndexLockScript is the part of a git hook that takes index.lock the
// first time the hook runs, and holds it until the git command that ran the
// hook has exited. This simulates another git process (for example an IDE
// running "git status" in the background) that takes the lock while a command
// of several steps, such as a rebase, is partway through.
const holdIndexLockScript = `if [ ! -e .git/index-lock-taken ]; then
touch .git/index-lock-taken .git/index.lock
(
# If lazygit runs the command in a pty (as it does for a pull), we
# get SIGHUP when the command exits
trap '' HUP
while kill -0 $PPID 2>/dev/null; do sleep 0.01; done
rm -f .git/index.lock
) >/dev/null 2>&1 </dev/null &
fi
`
// HoldIndexLockAfterNextCommit installs a post-commit hook that takes
// index.lock after the next commit (see holdIndexLockScript).
//
// Install it after creating the commits of the test setup, since it acts on
// the first commit that is made after it.
func HoldIndexLockAfterNextCommit(shell *Shell) {
shell.CreateFile(".git/hooks/post-commit", "#!/bin/sh\n"+holdIndexLockScript)
shell.MakeExecutable(".git/hooks/post-commit")
}
// HoldIndexLockBeforeNextPickIsCommitted installs a reference-transaction hook
// that takes index.lock (see holdIndexLockScript) the next time a rebase or
// cherry-pick has applied the changes of a commit, but hasn't committed them
// yet. This is when git creates CHERRY_PICK_HEAD.
func HoldIndexLockBeforeNextPickIsCommitted(shell *Shell) {
shell.CreateFile(".git/hooks/reference-transaction", `#!/bin/sh
[ "$1" = committed ] || exit 0
awk '$3 == "CHERRY_PICK_HEAD" && $2 !~ /^0+$/ { found = 1 } END { exit !found }' || exit 0
`+holdIndexLockScript)
shell.MakeExecutable(".git/hooks/reference-transaction")
}
@@ -0,0 +1,63 @@
package sync
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
)
var LockErrorPartwayThroughPullWithRebase = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Pull with a rebase strategy while another git process takes index.lock partway through the rebase",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.CreateFileAndAdd("file", "content1")
shell.Commit("one")
shell.UpdateFileAndAdd("file", "content2")
shell.Commit("two")
shell.CreateFileAndAdd("file3", "content3")
shell.Commit("three")
shell.CloneIntoRemote("origin")
shell.SetBranchUpstream("master", "origin/master")
shell.HardReset("HEAD^^")
shell.CreateFileAndAdd("file4", "content4")
shell.Commit("four")
shell.CreateFileAndAdd("file5", "content5")
shell.Commit("five")
shell.SetConfig("pull.rebase", "true")
// The rebase makes a commit when it picks "four"; the lock then stops
// it when it tries to pick "five".
shared.HoldIndexLockAfterNextCommit(shell)
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Lines(
Contains("five"),
Contains("four"),
Contains("one"),
)
t.Views().Status().Content(Equals("↓2↑2 repo → master"))
t.Views().Files().
IsFocused().
Press(keys.Universal.Pull)
t.Views().Status().Content(Equals("↑2 repo → master"))
t.Views().Commits().
Lines(
Contains("five"),
Contains("four"),
Contains("three"),
Contains("two"),
Contains("one"),
)
},
})
+6
View File
@@ -110,11 +110,14 @@ var tests = []*components.IntegrationTest{
cherry_pick.CherryPickAcrossWorktrees,
cherry_pick.CherryPickCommitThatBecomesEmpty,
cherry_pick.CherryPickConflicts,
cherry_pick.CherryPickConflictsCommittedByUser,
cherry_pick.CherryPickConflictsEmptyCommitAfterResolving,
cherry_pick.CherryPickDuringRebase,
cherry_pick.CherryPickMerge,
cherry_pick.CherryPickRange,
cherry_pick.CherryPickRangeAfterPaste,
cherry_pick.CherryPickStoppedByUntrackedFile,
cherry_pick.LockErrorPartwayThroughCherryPick,
commit.AddCoAuthor,
commit.AddCoAuthorRange,
commit.AddCoAuthorWhileCommitting,
@@ -358,6 +361,8 @@ var tests = []*components.IntegrationTest{
interactive_rebase.FixupSecondCommit,
interactive_rebase.InteractiveRebaseOfCopiedBranch,
interactive_rebase.InteractiveRebaseWithConflictForEditCommand,
interactive_rebase.LockErrorPartwayThroughRebase,
interactive_rebase.LockErrorPartwayThroughSkip,
interactive_rebase.MidRebaseRangeSelect,
interactive_rebase.Move,
interactive_rebase.MoveAcrossBranchBoundaryOutsideRebase,
@@ -643,6 +648,7 @@ var tests = []*components.IntegrationTest{
sync.ForcePushMultipleUpstream,
sync.ForcePushRemoteBranchNotStoredLocally,
sync.ForcePushTriangular,
sync.LockErrorPartwayThroughPullWithRebase,
sync.Pull,
sync.PullAndSetUpstream,
sync.PullMerge,
+34
View File
@@ -68,6 +68,40 @@ func findTodo(todos []todo.Todo, todoToFind Todo) (int, bool) {
return idx, ok
}
// LastDoneTodoWasRescheduled tells whether git put the last command of
// rebase-merge/done back into git-rebase-todo, given the parsed contents of
// these two files. In certain cases, git reschedules commands that failed. One
// example is if a patch would overwrite an untracked file; another one is an
// "exec" that failed.
func LastDoneTodoWasRescheduled(doneTodos []todo.Todo, todos []todo.Todo) bool {
// If the last command of the "done" file is the same as the first command
// of "git-rebase-todo", the command was rescheduled.
if len(doneTodos) > 0 && len(todos) > 0 && doneTodos[len(doneTodos)-1] == todos[0] {
return true
}
// Older versions of git have a bug whereby, if a command is rescheduled,
// the last successful command is appended to the "done" file again. To
// detect this, we need to compare the second-to-last done entry against the
// first todo entry, and also compare the last done entry against the
// last-but-two done entry; this latter check is needed for the following
// case:
// pick A
// exec make test
// pick B
// exec make test
// If pick B fails with conflicts, then the "done" file contains
// pick A
// exec make test
// pick B
// and git-rebase-todo contains
// exec make test
// Without the last condition we would erroneously treat this as the exec
// command being rescheduled.
return len(doneTodos) >= 3 && len(todos) > 0 && doneTodos[len(doneTodos)-2] == todos[0] &&
doneTodos[len(doneTodos)-1] == doneTodos[len(doneTodos)-3]
}
func ReadRebaseTodoFile(fileName string, commentChar byte) ([]todo.Todo, error) {
f, err := os.Open(fileName)
if err != nil {