From d94f2f05aca1862fa0555fb87b45ea79c08fd4b6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 19 Jun 2026 17:31:13 +0200 Subject: [PATCH] Only pass --no-optional-locks for background status refreshes We set GIT_OPTIONAL_LOCKS=0 for every git command we run. That env var only affects `git status`: it tells git not to take the optional lock it would otherwise use to write the index back after refreshing the cached stat information. The intent was to avoid contending for index.lock with git commands the user runs in a terminal. The downside is that our `git status` never persists the refreshed stat-cache. So whenever the working tree's cached stat info goes stale (e.g. editing files and discarding the changes, or a checkout), every subsequent status re-hashes the affected files to confirm they're clean, and stays slow until something else writes the index (such as the user running `git status` in a terminal). Fix this by only suppressing optional locks for refreshes that run unattended in the background; foreground refreshes triggered by a user action now run a plain `git status` that writes the refreshed index back, just like the command line does. Background refreshes keep passing --no-optional-locks so they still can't cause lock contention. RefreshOptions gains a Background flag that the background routines set, threaded down to the status command. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_builder.go | 6 ++---- pkg/commands/git_commands/file_loader.go | 9 ++++++++- pkg/commands/git_commands/file_loader_test.go | 11 ++++++++++- pkg/gui/background.go | 6 +++--- pkg/gui/controllers/files_controller.go | 2 +- pkg/gui/controllers/helpers/branches_helper.go | 4 ++-- pkg/gui/controllers/helpers/refresh_helper.go | 9 +++++---- pkg/gui/types/refresh.go | 8 ++++++++ 8 files changed, 39 insertions(+), 16 deletions(-) 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 }