Abort and redo a cherry-pick or revert that a lock error stopped

If a lock stops a cherry-pick or revert of several commits partway
through, the git command runner ran the same command again. This failed
with "cherry-pick is already in progress", and the user was left with
an operation that stopped halfway. Continuing it would skip the commit
that failed, because git doesn't reschedule it the way a rebase does.

Abort the operation instead, and then run the command again. Also
abort when the runner gives up, so that the user doesn't end up with
the stopped operation.

Only do this for several commits. For a single commit, git doesn't use
.git/sequencer, and a cherry-pick of one commit may even run while a
cherry-pick of several commits is in progress. If .git/sequencer exists
after a single cherry-pick failed, it belongs to that other operation,
and we must not abort it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Stefan Haller
2026-11-08 14:54:41 +01:00
co-authored by Claude Opus 5.5
parent 3ecf3ae4cb
commit d4b11b8cfd
5 changed files with 116 additions and 17 deletions
+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
@@ -49,6 +49,45 @@ func resumeRebaseAfterLockError(
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
})
}
// 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.
@@ -1,6 +1,7 @@
package git_commands
import (
"errors"
"os"
"path/filepath"
"testing"
@@ -106,3 +107,71 @@ func TestResumeRebaseAfterLockError(t *testing.T) {
}))
})
}
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()
})
}
}
+4 -1
View File
@@ -598,7 +598,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 {
@@ -63,25 +63,10 @@ var LockErrorPartwayThroughCherryPick = NewIntegrationTest(NewIntegrationTestArg
Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")).
Confirm()
}).
/* EXPECTED:
Lines(
Contains("two"),
Contains("one"),
Contains("base"),
)
ACTUAL: */
Tap(func() {
t.ExpectPopup().Alert().
Title(Equals("Error")).
Content(Contains("cherry-pick is already in progress")).
Confirm()
}).
Lines(
Contains("─── Pending cherry-picks"),
Contains("pick").Contains("two"),
Contains("─── Commits"),
Contains("one"),
Contains("base"),
)
},
})