Skip post-checkout hook when discarding changes (#4313)

- **PR Description**

Some people have post-checkout hooks that take a lot of time, which
makes discarding changes slow. You can argue that a post-checkout hook
should only run when you switch branches, so it doesnt't have to run
when checking out single files or directories. You can also argue that
lazygit might have implemented discarding changes by taking the current
patch and applying it in reverse, which wouldn't have run a
post-checkout hook either.

So disable them for all cases where we use git checkout with a path;
this includes checking out a file from the commit files view.

Fixes #4272.
This commit is contained in:
Stefan Haller 2025-02-25 11:42:26 +01:00 committed by GitHub
commit 17ab91e668
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 22 additions and 14 deletions

View file

@ -149,7 +149,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"rebase", "--interactive", "--autostash", "--keep-empty", "--no-autosquash", "--rebase-merges", "abcdef"}, "", nil). ExpectGitArgs([]string{"rebase", "--interactive", "--autostash", "--keep-empty", "--no-autosquash", "--rebase-merges", "abcdef"}, "", nil).
ExpectGitArgs([]string{"cat-file", "-e", "HEAD^:test999.txt"}, "", nil). ExpectGitArgs([]string{"cat-file", "-e", "HEAD^:test999.txt"}, "", nil).
ExpectGitArgs([]string{"checkout", "HEAD^", "--", "test999.txt"}, "", nil). ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "HEAD^", "--", "test999.txt"}, "", nil).
ExpectGitArgs([]string{"commit", "--amend", "--no-edit", "--allow-empty"}, "", nil). ExpectGitArgs([]string{"commit", "--amend", "--no-edit", "--allow-empty"}, "", nil).
ExpectGitArgs([]string{"rebase", "--continue"}, "", nil), ExpectGitArgs([]string{"rebase", "--continue"}, "", nil),
test: func(err error) { test: func(err error) {

View file

@ -109,6 +109,10 @@ func (self *WorkingTreeCommands) BeforeAndAfterFileForRename(file *models.File)
return beforeFile, afterFile, nil return beforeFile, afterFile, nil
} }
func newCheckoutCommand() *GitCommandBuilder {
return NewGitCmd("checkout").Config(fmt.Sprintf("core.hooksPath=%s", os.DevNull))
}
// DiscardAllFileChanges directly // DiscardAllFileChanges directly
func (self *WorkingTreeCommands) DiscardAllFileChanges(file *models.File) error { func (self *WorkingTreeCommands) DiscardAllFileChanges(file *models.File) error {
if file.IsRename() { if file.IsRename() {
@ -130,7 +134,7 @@ func (self *WorkingTreeCommands) DiscardAllFileChanges(file *models.File) error
if file.ShortStatus == "AA" { if file.ShortStatus == "AA" {
if err := self.cmd.New( if err := self.cmd.New(
NewGitCmd("checkout").Arg("--ours", "--", file.Name).ToArgv(), newCheckoutCommand().Arg("--ours", "--", file.Name).ToArgv(),
).Run(); err != nil { ).Run(); err != nil {
return err return err
} }
@ -189,7 +193,7 @@ func (self *WorkingTreeCommands) DiscardUnstagedDirChanges(node IFileNode) error
return err return err
} }
cmdArgs := NewGitCmd("checkout").Arg("--", node.GetPath()).ToArgv() cmdArgs := newCheckoutCommand().Arg("--", node.GetPath()).ToArgv()
if err := self.cmd.New(cmdArgs).Run(); err != nil { if err := self.cmd.New(cmdArgs).Run(); err != nil {
return err return err
} }
@ -222,7 +226,7 @@ func (self *WorkingTreeCommands) RemoveUntrackedDirFiles(node IFileNode) error {
} }
func (self *WorkingTreeCommands) DiscardUnstagedFileChanges(file *models.File) error { func (self *WorkingTreeCommands) DiscardUnstagedFileChanges(file *models.File) error {
cmdArgs := NewGitCmd("checkout").Arg("--", file.Name).ToArgv() cmdArgs := newCheckoutCommand().Arg("--", file.Name).ToArgv()
return self.cmd.New(cmdArgs).Run() return self.cmd.New(cmdArgs).Run()
} }
@ -315,7 +319,7 @@ func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reve
// CheckoutFile checks out the file for the given commit // CheckoutFile checks out the file for the given commit
func (self *WorkingTreeCommands) CheckoutFile(commitHash, fileName string) error { func (self *WorkingTreeCommands) CheckoutFile(commitHash, fileName string) error {
cmdArgs := NewGitCmd("checkout").Arg(commitHash, "--", fileName). cmdArgs := newCheckoutCommand().Arg(commitHash, "--", fileName).
ToArgv() ToArgv()
return self.cmd.New(cmdArgs).Run() return self.cmd.New(cmdArgs).Run()
@ -323,7 +327,7 @@ func (self *WorkingTreeCommands) CheckoutFile(commitHash, fileName string) error
// DiscardAnyUnstagedFileChanges discards any unstaged file changes via `git checkout -- .` // DiscardAnyUnstagedFileChanges discards any unstaged file changes via `git checkout -- .`
func (self *WorkingTreeCommands) DiscardAnyUnstagedFileChanges() error { func (self *WorkingTreeCommands) DiscardAnyUnstagedFileChanges() error {
cmdArgs := NewGitCmd("checkout").Arg("--", "."). cmdArgs := newCheckoutCommand().Arg("--", ".").
ToArgv() ToArgv()
return self.cmd.New(cmdArgs).Run() return self.cmd.New(cmdArgs).Run()

View file

@ -1,6 +1,8 @@
package git_commands package git_commands
import ( import (
"fmt"
"os"
"testing" "testing"
"github.com/go-errors/errors" "github.com/go-errors/errors"
@ -10,6 +12,8 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
var disableHooksFlag = fmt.Sprintf("core.hooksPath=%s", os.DevNull)
func TestWorkingTreeStageFile(t *testing.T) { func TestWorkingTreeStageFile(t *testing.T) {
runner := oscommands.NewFakeRunner(t). runner := oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"add", "--", "test.txt"}, "", nil) ExpectGitArgs([]string{"add", "--", "test.txt"}, "", nil)
@ -113,7 +117,7 @@ func TestWorkingTreeDiscardAllFileChanges(t *testing.T) {
}, },
removeFile: func(string) error { return nil }, removeFile: func(string) error { return nil },
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"checkout", "--", "test"}, "", errors.New("error")), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "--", "test"}, "", errors.New("error")),
expectedError: "error", expectedError: "error",
}, },
{ {
@ -125,7 +129,7 @@ func TestWorkingTreeDiscardAllFileChanges(t *testing.T) {
}, },
removeFile: func(string) error { return nil }, removeFile: func(string) error { return nil },
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"checkout", "--", "test"}, "", nil), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "--", "test"}, "", nil),
expectedError: "", expectedError: "",
}, },
{ {
@ -138,7 +142,7 @@ func TestWorkingTreeDiscardAllFileChanges(t *testing.T) {
removeFile: func(string) error { return nil }, removeFile: func(string) error { return nil },
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"reset", "--", "test"}, "", nil). ExpectGitArgs([]string{"reset", "--", "test"}, "", nil).
ExpectGitArgs([]string{"checkout", "--", "test"}, "", nil), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "--", "test"}, "", nil),
expectedError: "", expectedError: "",
}, },
{ {
@ -151,7 +155,7 @@ func TestWorkingTreeDiscardAllFileChanges(t *testing.T) {
removeFile: func(string) error { return nil }, removeFile: func(string) error { return nil },
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"reset", "--", "test"}, "", nil). ExpectGitArgs([]string{"reset", "--", "test"}, "", nil).
ExpectGitArgs([]string{"checkout", "--", "test"}, "", nil), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "--", "test"}, "", nil),
expectedError: "", expectedError: "",
}, },
{ {
@ -428,7 +432,7 @@ func TestWorkingTreeCheckoutFile(t *testing.T) {
commitHash: "11af912", commitHash: "11af912",
fileName: "test999.txt", fileName: "test999.txt",
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"checkout", "11af912", "--", "test999.txt"}, "", nil), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "11af912", "--", "test999.txt"}, "", nil),
test: func(err error) { test: func(err error) {
assert.NoError(t, err) assert.NoError(t, err)
}, },
@ -438,7 +442,7 @@ func TestWorkingTreeCheckoutFile(t *testing.T) {
commitHash: "11af912", commitHash: "11af912",
fileName: "test999.txt", fileName: "test999.txt",
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"checkout", "11af912", "--", "test999.txt"}, "", errors.New("error")), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "11af912", "--", "test999.txt"}, "", errors.New("error")),
test: func(err error) { test: func(err error) {
assert.Error(t, err) assert.Error(t, err)
}, },
@ -468,7 +472,7 @@ func TestWorkingTreeDiscardUnstagedFileChanges(t *testing.T) {
testName: "valid case", testName: "valid case",
file: &models.File{Name: "test.txt"}, file: &models.File{Name: "test.txt"},
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"checkout", "--", "test.txt"}, "", nil), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "--", "test.txt"}, "", nil),
test: func(err error) { test: func(err error) {
assert.NoError(t, err) assert.NoError(t, err)
}, },
@ -495,7 +499,7 @@ func TestWorkingTreeDiscardAnyUnstagedFileChanges(t *testing.T) {
{ {
testName: "valid case", testName: "valid case",
runner: oscommands.NewFakeRunner(t). runner: oscommands.NewFakeRunner(t).
ExpectGitArgs([]string{"checkout", "--", "."}, "", nil), ExpectGitArgs([]string{"-c", disableHooksFlag, "checkout", "--", "."}, "", nil),
test: func(err error) { test: func(err error) {
assert.NoError(t, err) assert.NoError(t, err)
}, },