From 94db69f64b7b05840cdf2839bd987f856f9b1d63 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 19 Jun 2026 18:14:24 +0200 Subject: [PATCH 1/3] Add GlobalArg/GlobalArgIf to GitCommandBuilder This can be used to add a git argument that goes before the git subcommand. --- pkg/commands/git_commands/git_command_builder.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/commands/git_commands/git_command_builder.go b/pkg/commands/git_commands/git_command_builder.go index 30496f453..4178cbd22 100644 --- a/pkg/commands/git_commands/git_command_builder.go +++ b/pkg/commands/git_commands/git_command_builder.go @@ -38,6 +38,22 @@ func (self *GitCommandBuilder) ArgIfElse(condition bool, ifTrue string, ifFalse return self.Arg(ifFalse) } +// GlobalArg adds top-level options for git itself (e.g. --no-optional-locks). +// Unlike Arg, these are prepended before the command, where git expects them. +func (self *GitCommandBuilder) GlobalArg(args ...string) *GitCommandBuilder { + self.args = append(append([]string{}, args...), self.args...) + + return self +} + +func (self *GitCommandBuilder) GlobalArgIf(condition bool, args ...string) *GitCommandBuilder { + if condition { + self.GlobalArg(args...) + } + + return self +} + func (self *GitCommandBuilder) Config(value string) *GitCommandBuilder { // config settings come before the command self.args = append([]string{"-c", value}, self.args...) From d94f2f05aca1862fa0555fb87b45ea79c08fd4b6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 19 Jun 2026 17:31:13 +0200 Subject: [PATCH 2/3] 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 } From eb988395e6d1caaac40a0dfbeb2cf259fb2889b0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 19 Jun 2026 17:40:51 +0200 Subject: [PATCH 3/3] Remove the now-redundant gitCmdObjBuilder wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wrapper existed to add a git-specific env var to every command. Now that that's gone, its New/NewShell/Quote methods just delegated to the inner builder. The only remaining git-specific behavior — the command runner — is attached in the constructor via CloneWithNewRunner, which already returns a complete builder, so we can return that directly and drop the wrapper struct. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/commands/git_cmd_obj_builder.go | 35 ++++++----------------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/pkg/commands/git_cmd_obj_builder.go b/pkg/commands/git_cmd_obj_builder.go index 495582722..6b6bd26d8 100644 --- a/pkg/commands/git_cmd_obj_builder.go +++ b/pkg/commands/git_cmd_obj_builder.go @@ -5,37 +5,16 @@ import ( "github.com/sirupsen/logrus" ) -// all we're doing here is wrapping the default command object builder with -// some git-specific stuff: e.g. adding a git-specific env var - -type gitCmdObjBuilder struct { - innerBuilder *oscommands.CmdObjBuilder -} - -var _ oscommands.ICmdObjBuilder = &gitCmdObjBuilder{} - -func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuilder) *gitCmdObjBuilder { - // the price of having a convenient interface where we can say .New(...).Run() is that our builder now depends on our runner, so when we want to wrap the default builder/runner in new functionality we need to jump through some hoops. We could avoid the use of a decorator function here by just exporting the runner field on the default builder but that would be misleading because we don't want anybody using that to run commands (i.e. we want there to be a single API used across the codebase) - updatedBuilder := innerBuilder.CloneWithNewRunner(func(runner oscommands.ICmdObjRunner) oscommands.ICmdObjRunner { +// NewGitCmdObjBuilder returns a command object builder whose runner is wrapped +// with our git-specific runner (logging, credential handling, etc.). +func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuilder) *oscommands.CmdObjBuilder { + // We decorate the runner rather than exposing the builder's runner field: + // that field stays unexported so there's a single API for running commands + // across the codebase. + return innerBuilder.CloneWithNewRunner(func(runner oscommands.ICmdObjRunner) oscommands.ICmdObjRunner { return &gitCmdObjRunner{ log: log, innerRunner: runner, } }) - - return &gitCmdObjBuilder{ - innerBuilder: updatedBuilder, - } -} - -func (self *gitCmdObjBuilder) New(args []string) *oscommands.CmdObj { - return self.innerBuilder.New(args) -} - -func (self *gitCmdObjBuilder) NewShell(cmdStr string, shellFunctionsFile string) *oscommands.CmdObj { - return self.innerBuilder.NewShell(cmdStr, shellFunctionsFile) -} - -func (self *gitCmdObjBuilder) Quote(str string) string { - return self.innerBuilder.Quote(str) }