mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
Prevent stale index.lock files from diffs rendered through a pty on Windows (#5888)
At the end of a diff against the worktree, git re-reads and refreshes
the index and writes it back if it found stale stat information
(diff.autoRefreshIndex, on by default). It holds index.lock for the
whole refresh; GIT_OPTIONAL_LOCKS does not cover this lock, and the
window scales with the size of the repository (~150ms for a 6k-file
repository with a warm stat cache).
On Windows, a pty task that is stopped because the user moved on
terminates its git process at an arbitrary point: tearing down the
pseudoconsole delivers CTRL_CLOSE_EVENT, which git leaves to the default
handler, which simply calls ExitProcess. If that lands inside the
refresh, a stale index.lock is left behind and the next git command
chokes on it. This is the same problem that 98801da106 fixed by no
longer killing git processes; the ConPTY support added in 0.63
reintroduced it through the close event.
Disable the automatic refresh for pty-rendered commands. They can afford
it: the refresh only persists refreshed stat information, and lazygit's
foreground git status refreshes -- which never run in a pty and are
never killed -- already write that back on every user action and on
terminal focus-in. The cost is that while the on-disk stat cache is
stale, an external differ is invoked even for files whose stat
information changed but whose content didn't, showing them as empty
diffs; this heals with the next foreground refresh, which also
re-renders the view.
Unix keeps the refresh: a stopped pty child gets SIGTERM there, and
git's signal handlers remove its lock files, so the lock window is
harmless. The rawGit renderer keeps it too: its tasks don't run in a pty
and are never killed on Windows -- they either run to completion or die
on a broken pipe mid-output, before the refresh begins.
This commit is contained in:
commit
9b22c7dc77
|
|
@ -5,6 +5,8 @@ import (
|
|||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
|
||||
|
|
@ -68,6 +70,8 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
return gui.newCmdTask(view, cmd, prefix)
|
||||
}
|
||||
|
||||
cmd.Args = withPtyGitConfig(cmd.Args, runtime.GOOS)
|
||||
|
||||
// Run the pty after layout so that it gets the correct size
|
||||
gui.afterLayout(func() error {
|
||||
// Need to get the width and the pager command again because the layout might have
|
||||
|
|
@ -139,6 +143,43 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
return nil
|
||||
}
|
||||
|
||||
// withPtyGitConfig returns args with extra git configuration for commands
|
||||
// that render into a pty. On Windows, such a command is terminated at an
|
||||
// arbitrary point of its execution when its task stops: tearing down the
|
||||
// pseudoconsole delivers CTRL_CLOSE_EVENT, which git leaves to the default
|
||||
// handler, which just calls ExitProcess. git's automatic index refresh
|
||||
// (diff.autoRefreshIndex, on by default) takes index.lock at the end of a
|
||||
// diff against the worktree to write back refreshed stat information —
|
||||
// GIT_OPTIONAL_LOCKS does not cover this lock — and a termination landing
|
||||
// in that window leaves a stale index.lock behind that the next git command
|
||||
// chokes on. So don't let pty-rendered commands refresh the index;
|
||||
// lazygit's foreground `git status` refreshes, which never run in a pty,
|
||||
// keep the stat cache fresh instead.
|
||||
//
|
||||
// On Unix a stopped pty child gets SIGTERM, and git's signal handlers remove
|
||||
// its lock files, so the refresh can stay enabled there and keep healing
|
||||
// stale stat info.
|
||||
func withPtyGitConfig(args []string, goos string) []string {
|
||||
if goos != "windows" {
|
||||
return args
|
||||
}
|
||||
// Most pty commands are direct git invocations, but the user-configured
|
||||
// ones can be arbitrary command lines (e.g. a branchLogCmd wrapping git
|
||||
// in `sh -c`), and injecting git flags into those would corrupt them.
|
||||
// Only direct git invocations get the config; that loses nothing, since
|
||||
// the wrapped commands are log commands, which never take the index
|
||||
// lock. (For direct invocations other than worktree diffs the config is
|
||||
// simply a no-op.)
|
||||
base := strings.TrimSuffix(strings.ToLower(filepath.Base(args[0])), ".exe")
|
||||
if base != "git" {
|
||||
return args
|
||||
}
|
||||
result := make([]string, 0, len(args)+2)
|
||||
result = append(result, args[0])
|
||||
result = append(result, "-c", "diff.autoRefreshIndex=false")
|
||||
return append(result, args[1:]...)
|
||||
}
|
||||
|
||||
func removeExistingTermEnvVars(env []string) []string {
|
||||
return lo.Filter(env, func(envVar string, _ int) bool {
|
||||
return !isTermEnvVar(envVar)
|
||||
|
|
|
|||
29
pkg/gui/pty_test.go
Normal file
29
pkg/gui/pty_test.go
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
package gui
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestWithPtyGitConfig(t *testing.T) {
|
||||
args := []string{"git", "-C", "/repo", "diff", "--color=always"}
|
||||
|
||||
assert.Equal(t,
|
||||
[]string{"git", "-c", "diff.autoRefreshIndex=false", "-C", "/repo", "diff", "--color=always"},
|
||||
withPtyGitConfig(args, "windows"))
|
||||
|
||||
assert.Equal(t, args, withPtyGitConfig(args, "linux"))
|
||||
assert.Equal(t, args, withPtyGitConfig(args, "darwin"))
|
||||
|
||||
// A user-configured command that wraps git in a shell must not have git
|
||||
// flags injected into it.
|
||||
shellArgs := []string{"sh", "-c", "git log --graph {{branchName}} -- | sed -e s/x/y/"}
|
||||
assert.Equal(t, shellArgs, withPtyGitConfig(shellArgs, "windows"))
|
||||
|
||||
// The guard recognizes git regardless of case and extension.
|
||||
exeArgs := []string{"GIT.EXE", "diff"}
|
||||
assert.Equal(t,
|
||||
[]string{"GIT.EXE", "-c", "diff.autoRefreshIndex=false", "diff"},
|
||||
withPtyGitConfig(exeArgs, "windows"))
|
||||
}
|
||||
Loading…
Reference in a new issue