diff --git a/pkg/commands/git_cmd_obj_builder.go b/pkg/commands/git_cmd_obj_builder.go index 753489ef4..495582722 100644 --- a/pkg/commands/git_cmd_obj_builder.go +++ b/pkg/commands/git_cmd_obj_builder.go @@ -28,14 +28,12 @@ func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuild } } -var defaultEnvVar = "GIT_OPTIONAL_LOCKS=0" - func (self *gitCmdObjBuilder) New(args []string) *oscommands.CmdObj { - return self.innerBuilder.New(args).AddEnvVars(defaultEnvVar) + return self.innerBuilder.New(args) } func (self *gitCmdObjBuilder) NewShell(cmdStr string, shellFunctionsFile string) *oscommands.CmdObj { - return self.innerBuilder.NewShell(cmdStr, shellFunctionsFile).AddEnvVars(defaultEnvVar) + return self.innerBuilder.NewShell(cmdStr, shellFunctionsFile) } func (self *gitCmdObjBuilder) Quote(str string) string { diff --git a/pkg/commands/git_commands/file_loader.go b/pkg/commands/git_commands/file_loader.go index 36ab8ef67..9df977bb9 100644 --- a/pkg/commands/git_commands/file_loader.go +++ b/pkg/commands/git_commands/file_loader.go @@ -36,6 +36,11 @@ type GetStatusFileOptions struct { // This is useful for users with bare repos for dotfiles who default to hiding untracked files, // but want to occasionally see them to `git add` a new file. ForceShowUntracked bool + // When true, this status is part of an unattended background refresh, so we + // pass --no-optional-locks to avoid index.lock contention with git commands + // the user runs in a terminal (at the cost of not persisting git's refreshed + // stat-cache). + Background bool } func (self *FileLoader) GetStatusFiles(opts GetStatusFileOptions) []*models.File { @@ -47,7 +52,7 @@ func (self *FileLoader) GetStatusFiles(opts GetStatusFileOptions) []*models.File } untrackedFilesArg := fmt.Sprintf("--untracked-files=%s", untrackedFilesSetting) - statuses, err := self.gitStatus(GitStatusOptions{NoRenames: opts.NoRenames, UntrackedFilesArg: untrackedFilesArg}) + statuses, err := self.gitStatus(GitStatusOptions{NoRenames: opts.NoRenames, UntrackedFilesArg: untrackedFilesArg, Background: opts.Background}) if err != nil { self.Log.Error(err) } @@ -148,6 +153,7 @@ func (self *FileLoader) getFileDiffs() (map[string]FileDiff, error) { type GitStatusOptions struct { NoRenames bool UntrackedFilesArg string + Background bool } type FileStatus struct { @@ -169,6 +175,7 @@ func (self *FileLoader) gitDiffNumStat() (string, error) { func (self *FileLoader) gitStatus(opts GitStatusOptions) ([]FileStatus, error) { cmdArgs := NewGitCmd("status"). + GlobalArgIf(opts.Background, "--no-optional-locks"). Arg(opts.UntrackedFilesArg). Arg("--porcelain"). Arg("-z"). diff --git a/pkg/commands/git_commands/file_loader_test.go b/pkg/commands/git_commands/file_loader_test.go index ec1f502f1..4f6b5e136 100644 --- a/pkg/commands/git_commands/file_loader_test.go +++ b/pkg/commands/git_commands/file_loader_test.go @@ -13,6 +13,7 @@ func TestFileGetStatusFiles(t *testing.T) { type scenario struct { testName string similarityThreshold int + background bool runner oscommands.ICmdObjRunner showNumstatInFilesView bool expectedFiles []*models.File @@ -26,6 +27,14 @@ func TestFileGetStatusFiles(t *testing.T) { ExpectGitArgs([]string{"status", "--untracked-files=yes", "--porcelain", "-z", "--find-renames=50%"}, "", nil), expectedFiles: []*models.File{}, }, + { + testName: "Background refresh passes --no-optional-locks", + similarityThreshold: 50, + background: true, + runner: oscommands.NewFakeRunner(t). + ExpectGitArgs([]string{"--no-optional-locks", "status", "--untracked-files=yes", "--porcelain", "-z", "--find-renames=50%"}, "", nil), + expectedFiles: []*models.File{}, + }, { testName: "Several files found", similarityThreshold: 50, @@ -246,7 +255,7 @@ func TestFileGetStatusFiles(t *testing.T) { getFileType: func(string) string { return "file" }, } - assert.EqualValues(t, s.expectedFiles, loader.GetStatusFiles(GetStatusFileOptions{})) + assert.EqualValues(t, s.expectedFiles, loader.GetStatusFiles(GetStatusFileOptions{Background: s.background})) }) } } diff --git a/pkg/gui/background.go b/pkg/gui/background.go index afd343df3..94bf4f678 100644 --- a/pkg/gui/background.go +++ b/pkg/gui/background.go @@ -133,7 +133,7 @@ func (self *BackgroundRoutineMgr) startBackgroundFilesRefresh() { userConfig := self.gui.UserConfig() self.goEvery(userConfig.Refresher.RefreshIntervalDuration(), self.gui.stopChan, func(_ bool) error { - self.gui.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) + self.gui.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}, Background: true}) return nil }) } @@ -184,7 +184,7 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() { // No need to update the stored snapshot here; Refresh does that. self.gui.c.Log.Info("External ref change detected — refreshing") - self.gui.c.Refresh(types.RefreshOptions{}) + self.gui.c.Refresh(types.RefreshOptions{Background: true}) } // returns a channel that can be used to trigger the callback immediately @@ -226,7 +226,7 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru func (self *BackgroundRoutineMgr) backgroundFetch() (err error) { err = self.gui.git.Sync.FetchBackground() - return self.gui.helpers.BranchesHelper.PostFetchRefresh(err) + return self.gui.helpers.BranchesHelper.PostFetchRefresh(err, true) } func (self *BackgroundRoutineMgr) triggerImmediateFetch() { diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 09f654e2b..d048da508 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -1372,7 +1372,7 @@ func (self *FilesController) fetch() error { return errors.New(self.c.Tr.PassUnameWrong) } - return self.c.Helpers().BranchesHelper.PostFetchRefresh(err) + return self.c.Helpers().BranchesHelper.PostFetchRefresh(err, false) }) } diff --git a/pkg/gui/controllers/helpers/branches_helper.go b/pkg/gui/controllers/helpers/branches_helper.go index 8af447f79..ccc9d33ac 100644 --- a/pkg/gui/controllers/helpers/branches_helper.go +++ b/pkg/gui/controllers/helpers/branches_helper.go @@ -285,7 +285,7 @@ func (self *BranchesHelper) deleteRemoteBranches(remoteBranches []*models.Remote return nil } -func (self *BranchesHelper) PostFetchRefresh(fetchErr error) error { +func (self *BranchesHelper) PostFetchRefresh(fetchErr error, background bool) error { scope := []types.RefreshableView{ types.BRANCHES, types.COMMITS, types.REMOTES, types.TAGS, types.PULL_REQUESTS, } @@ -293,7 +293,7 @@ func (self *BranchesHelper) PostFetchRefresh(fetchErr error) error { if self.c.UserConfig().Git.AutoForwardBranches != "none" { scope = append(scope, types.WORKTREES) } - self.c.Refresh(types.RefreshOptions{Scope: scope, Mode: types.SYNC}) + self.c.Refresh(types.RefreshOptions{Scope: scope, Mode: types.SYNC, Background: background}) if fetchErr != nil { return fetchErr } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index b51c528e2..31035d104 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -200,7 +200,7 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { if scopeSet.Includes(types.FILES) { fileWg.Add(1) refresh("files", func() { - _ = self.refreshFilesAndSubmodules() + _ = self.refreshFilesAndSubmodules(options.Background) fileWg.Done() }) } @@ -624,7 +624,7 @@ func (self *RefreshHelper) refreshBranches(refreshWorktrees bool, keepBranchSele self.refreshStatus() } -func (self *RefreshHelper) refreshFilesAndSubmodules() error { +func (self *RefreshHelper) refreshFilesAndSubmodules(background bool) error { self.c.Mutexes().RefreshingFilesMutex.Lock() self.c.State().SetIsRefreshingFiles(true) defer func() { @@ -636,7 +636,7 @@ func (self *RefreshHelper) refreshFilesAndSubmodules() error { return err } - if err := self.refreshStateFiles(); err != nil { + if err := self.refreshStateFiles(background); err != nil { return err } @@ -649,7 +649,7 @@ func (self *RefreshHelper) refreshFilesAndSubmodules() error { return nil } -func (self *RefreshHelper) refreshStateFiles() error { +func (self *RefreshHelper) refreshStateFiles(background bool) error { fileTreeViewModel := self.c.Contexts().Files.FileTreeViewModel prevConflictFileCount := 0 @@ -687,6 +687,7 @@ func (self *RefreshHelper) refreshStateFiles() error { files := self.c.Git().Loaders.FileLoader. GetStatusFiles(git_commands.GetStatusFileOptions{ ForceShowUntracked: self.c.Contexts().Files.ForceShowUntracked(), + Background: background, }) conflictFileCount := 0 diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index 8092ee36e..c9e156180 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -44,4 +44,12 @@ type RefreshOptions struct { // keeps the selection index the same. Useful after checking out a detached // head, and selecting index 0. KeepBranchSelectionIndex bool + + // When true, this refresh was initiated by a background routine rather than + // by a user action. We use it to keep background `git status` calls from + // taking optional git locks, so they don't contend for index.lock with git + // commands the user runs in a terminal. The cost is that such a status won't + // persist git's refreshed stat-cache, which is the right trade-off for + // unattended work; foreground refreshes leave this false so they do persist. + Background bool }