diff --git a/pkg/commands/git_commands/lock_error_recovery.go b/pkg/commands/git_commands/lock_error_recovery.go new file mode 100644 index 000000000..8925a65a4 --- /dev/null +++ b/pkg/commands/git_commands/lock_error_recovery.go @@ -0,0 +1,92 @@ +package git_commands + +import ( + "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)) +} + +// 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() +} diff --git a/pkg/commands/git_commands/lock_error_recovery_test.go b/pkg/commands/git_commands/lock_error_recovery_test.go new file mode 100644 index 000000000..1fe967983 --- /dev/null +++ b/pkg/commands/git_commands/lock_error_recovery_test.go @@ -0,0 +1,108 @@ +package git_commands + +import ( + "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)) + })) + }) +} diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index be0485e96..bce778472 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -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 skipEditor(self.cmd.New(cmdArgs)).Run() + return self.resumeAfterLockError(skipEditor(self.cmd.New(cmdArgs))).Run() } // BeginInteractiveRebaseForCommit starts an interactive rebase to edit the current @@ -513,6 +513,16 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma return nil } +// resumeAfterLockError makes cmdObj, a command that starts 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 { diff --git a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go index 0d0b9cf92..d5d697dfc 100644 --- a/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go +++ b/pkg/integration/tests/interactive_rebase/lock_error_partway_through_rebase.go @@ -34,26 +34,11 @@ var LockErrorPartwayThroughRebase = NewIntegrationTest(NewIntegrationTestArgs{ Content(Contains("Are you sure you want to drop the selected commit(s)?")). Confirm() }). - /* EXPECTED: IsFocused(). Lines( Contains("commit-04"), Contains("commit-03"), Contains("commit-01"), ) - ACTUAL: */ - Tap(func() { - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content(Contains("It seems that there is already a rebase-merge directory")). - Confirm() - }). - Lines( - Contains("─── Pending rebase todos"), - Contains("pick").Contains("commit-04"), - Contains("─── Commits"), - Contains("commit-03"), - Contains("commit-01"), - ) }, })