From 16ed3c23773d5e7b0573c5af805f6a8cb86512ac Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Mon, 10 Jul 2023 18:52:08 +1000 Subject: Retry on index.lock error I don't know why we're getting index.lock errors but they're impossile to stop anyway given that other processes can be calling git commands. So we're retrying a few times before re-raising. To do this we need to clone the command and the current implementation for that is best-effort. I do worry about the maintainability of that but we'll see how it goes. Also, I thought you'd need to clone the task (if it exists) but now I think not; as long as you don't call done twice on it you should be fine, and you shouldn't be done'ing a task as part of running a command: that should happen higher up. --- pkg/commands/git_cmd_obj_runner.go | 43 +++++++++++++++++++++++++-- pkg/commands/oscommands/cmd_obj.go | 16 ++++++++++ pkg/integration/tests/file/discard_changes.go | 2 +- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/pkg/commands/git_cmd_obj_runner.go b/pkg/commands/git_cmd_obj_runner.go index 96cef3c61..163b85145 100644 --- a/pkg/commands/git_cmd_obj_runner.go +++ b/pkg/commands/git_cmd_obj_runner.go @@ -1,12 +1,20 @@ package commands import ( + "strings" + "time" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/sirupsen/logrus" ) // here we're wrapping the default command runner in some git-specific stuff e.g. retry logic if we get an error due to the presence of .git/index.lock +const ( + WaitTime = 50 * time.Millisecond + RetryCount = 5 +) + type gitCmdObjRunner struct { log *logrus.Entry innerRunner oscommands.ICmdObjRunner @@ -18,13 +26,44 @@ func (self *gitCmdObjRunner) Run(cmdObj oscommands.ICmdObj) error { } func (self *gitCmdObjRunner) RunWithOutput(cmdObj oscommands.ICmdObj) (string, error) { - return self.innerRunner.RunWithOutput(cmdObj) + var output string + var err error + for i := 0; i < RetryCount; i++ { + newCmdObj := cmdObj.Clone() + output, err = self.innerRunner.RunWithOutput(newCmdObj) + + if err == nil || !strings.Contains(output, ".git/index.lock") { + return output, err + } + + // if we have an error based on the index lock, we should wait a bit and then retry + self.log.Warn("index.lock prevented command from running. Retrying command after a small wait") + time.Sleep(WaitTime) + } + + return output, err } func (self *gitCmdObjRunner) RunWithOutputs(cmdObj oscommands.ICmdObj) (string, string, error) { - return self.innerRunner.RunWithOutputs(cmdObj) + var stdout, stderr string + var err error + for i := 0; i < RetryCount; i++ { + newCmdObj := cmdObj.Clone() + stdout, stderr, err = self.innerRunner.RunWithOutputs(newCmdObj) + + if err == nil || !strings.Contains(stdout+stderr, ".git/index.lock") { + return stdout, stderr, err + } + + // if we have an error based on the index lock, we should wait a bit and then retry + self.log.Warn("index.lock prevented command from running. Retrying command after a small wait") + time.Sleep(WaitTime) + } + + return stdout, stderr, err } +// Retry logic not implemented here, but these commands typically don't need to obtain a lock. func (self *gitCmdObjRunner) RunAndProcessLines(cmdObj oscommands.ICmdObj, onLine func(line string) (bool, error)) error { return self.innerRunner.RunAndProcessLines(cmdObj, onLine) } diff --git a/pkg/commands/oscommands/cmd_obj.go b/pkg/commands/oscommands/cmd_obj.go index d8f287727..520a76a1b 100644 --- a/pkg/commands/oscommands/cmd_obj.go +++ b/pkg/commands/oscommands/cmd_obj.go @@ -65,6 +65,8 @@ type ICmdObj interface { GetCredentialStrategy() CredentialStrategy GetTask() gocui.Task + + Clone() ICmdObj } type CmdObj struct { @@ -215,3 +217,17 @@ func (self *CmdObj) GetCredentialStrategy() CredentialStrategy { func (self *CmdObj) GetTask() gocui.Task { return self.task } + +func (self *CmdObj) Clone() ICmdObj { + clone := &CmdObj{} + *clone = *self + clone.cmd = cloneCmd(self.cmd) + return clone +} + +func cloneCmd(cmd *exec.Cmd) *exec.Cmd { + clone := &exec.Cmd{} + *clone = *cmd + + return clone +} diff --git a/pkg/integration/tests/file/discard_changes.go b/pkg/integration/tests/file/discard_changes.go index 9d2791fbf..0ddd08675 100644 --- a/pkg/integration/tests/file/discard_changes.go +++ b/pkg/integration/tests/file/discard_changes.go @@ -8,7 +8,7 @@ import ( var DiscardChanges = NewIntegrationTest(NewIntegrationTestArgs{ Description: "Discarding all possible permutations of changed files", ExtraCmdArgs: []string{}, - Skip: true, // failing due to index.lock file being created + Skip: false, SetupConfig: func(config *config.AppConfig) { }, SetupRepo: func(shell *Shell) { -- cgit v1.2.3