diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000000..62f9670c83d --- /dev/null +++ b/.editorconfig @@ -0,0 +1,4 @@ +root = true + +[*.go] +indent_style = tab diff --git a/pkg/commands/git.go b/pkg/commands/git.go index b1b04a72fa0..b43c8c4e55a 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -2,12 +2,9 @@ package commands import ( "os" - "path" - "path/filepath" "strings" "github.com/go-errors/errors" - "github.com/spf13/afero" gogit "github.com/jesseduffield/go-git/v5" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" @@ -15,7 +12,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/commands/patch" "github.com/jesseduffield/lazygit/pkg/common" - "github.com/jesseduffield/lazygit/pkg/env" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -64,39 +60,14 @@ func NewGitCommand( osCommand *oscommands.OSCommand, gitConfig git_config.IGitConfig, ) (*GitCommand, error) { - currentPath, err := os.Getwd() + repoPaths, err := git_commands.GetRepoPaths(osCommand.Cmd, version) if err != nil { - return nil, utils.WrapError(err) - } - - // converting to forward slashes for the sake of windows (which uses backwards slashes). We want everything - // to have forward slashes internally - currentPath = filepath.ToSlash(currentPath) - - gitDir := env.GetGitDirEnv() - if gitDir != "" { - // we've been given the git directory explicitly so no need to navigate to it - _, err := cmn.Fs.Stat(gitDir) - if err != nil { - return nil, utils.WrapError(err) - } - } else { - // we haven't been given the git dir explicitly so we assume it's in the current working directory as `.git/` (or an ancestor directory) - - rootDirectory, err := findWorktreeRoot(cmn.Fs, currentPath) - if err != nil { - return nil, utils.WrapError(err) - } - currentPath = rootDirectory - err = os.Chdir(rootDirectory) - if err != nil { - return nil, utils.WrapError(err) - } + return nil, errors.Errorf("Error getting repo paths: %v", err) } - repoPaths, err := git_commands.GetRepoPaths(cmn.Fs, currentPath) + err = os.Chdir(repoPaths.WorktreePath()) if err != nil { - return nil, errors.Errorf("Error getting repo paths: %v", err) + return nil, utils.WrapError(err) } repository, err := gogit.PlainOpenWithOptions( @@ -208,32 +179,6 @@ func NewGitCommandAux( } } -// this returns the root of the current worktree. So if you start lazygit from within -// a subdirectory of the worktree, it will start in the context of the root of that worktree -func findWorktreeRoot(fs afero.Fs, currentPath string) (string, error) { - for { - // we don't care if .git is a directory or a file: either is okay. - _, err := fs.Stat(path.Join(currentPath, ".git")) - - if err == nil { - return currentPath, nil - } - - if !os.IsNotExist(err) { - return "", utils.WrapError(err) - } - - currentPath = path.Dir(currentPath) - - atRoot := currentPath == path.Dir(currentPath) - if atRoot { - // we should never really land here: the code that creates GitCommand should - // verify we're in a git directory - return "", errors.New("Must open lazygit in a git repository") - } - } -} - func VerifyInGitRepo(osCommand *oscommands.OSCommand) error { return osCommand.Cmd.New(git_commands.NewGitCmd("rev-parse").Arg("--git-dir").ToArgv()).DontLog().Run() } diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index e0b5b8a9aeb..dfb0b408510 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -250,6 +250,7 @@ func (self *CommitCommands) ShowCmdObj(sha string, filterPath string) oscommands Arg(sha). ArgIf(self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). ArgIf(filterPath != "", "--", filterPath). + Dir(self.repoPaths.worktreePath). ToArgv() return self.cmd.New(cmdArgs).DontLog() diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 0ade25a8de5..a2b674eebff 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -197,7 +197,7 @@ func TestCommitShowCmdObj(t *testing.T) { contextSize: 3, ignoreWhitespace: false, extDiffCmd: "", - expected: []string{"show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890"}, + expected: []string{"-C", "/path/to/worktree", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890"}, }, { testName: "Default case with filter path", @@ -205,7 +205,7 @@ func TestCommitShowCmdObj(t *testing.T) { contextSize: 3, ignoreWhitespace: false, extDiffCmd: "", - expected: []string{"show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--", "file.txt"}, + expected: []string{"-C", "/path/to/worktree", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890", "--", "file.txt"}, }, { testName: "Show diff with custom context size", @@ -213,7 +213,7 @@ func TestCommitShowCmdObj(t *testing.T) { contextSize: 77, ignoreWhitespace: false, extDiffCmd: "", - expected: []string{"show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890"}, + expected: []string{"-C", "/path/to/worktree", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890"}, }, { testName: "Show diff, ignoring whitespace", @@ -221,7 +221,7 @@ func TestCommitShowCmdObj(t *testing.T) { contextSize: 77, ignoreWhitespace: true, extDiffCmd: "", - expected: []string{"show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890", "--ignore-all-space"}, + expected: []string{"-C", "/path/to/worktree", "show", "--no-ext-diff", "--submodule", "--color=always", "--unified=77", "--stat", "--decorate", "-p", "1234567890", "--ignore-all-space"}, }, { testName: "Show diff with external diff command", @@ -229,7 +229,7 @@ func TestCommitShowCmdObj(t *testing.T) { contextSize: 3, ignoreWhitespace: false, extDiffCmd: "difft --color=always", - expected: []string{"-c", "diff.external=difft --color=always", "show", "--ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890"}, + expected: []string{"-C", "/path/to/worktree", "-c", "diff.external=difft --color=always", "show", "--ext-diff", "--submodule", "--color=always", "--unified=3", "--stat", "--decorate", "-p", "1234567890"}, }, } @@ -243,7 +243,10 @@ func TestCommitShowCmdObj(t *testing.T) { appState.DiffContextSize = s.contextSize runner := oscommands.NewFakeRunner(t).ExpectGitArgs(s.expected, "", nil) - instance := buildCommitCommands(commonDeps{userConfig: userConfig, appState: appState, runner: runner}) + repoPaths := RepoPaths{ + worktreePath: "/path/to/worktree", + } + instance := buildCommitCommands(commonDeps{userConfig: userConfig, appState: appState, runner: runner, repoPaths: &repoPaths}) assert.NoError(t, instance.ShowCmdObj("1234567890", s.filterPath).Run()) runner.CheckForMissingCalls() diff --git a/pkg/commands/git_commands/diff.go b/pkg/commands/git_commands/diff.go index 3729390241a..73b30bc4848 100644 --- a/pkg/commands/git_commands/diff.go +++ b/pkg/commands/git_commands/diff.go @@ -14,14 +14,19 @@ func NewDiffCommands(gitCommon *GitCommon) *DiffCommands { func (self *DiffCommands) DiffCmdObj(diffArgs []string) oscommands.ICmdObj { return self.cmd.New( - NewGitCmd("diff").Arg("--submodule", "--no-ext-diff", "--color").Arg(diffArgs...).ToArgv(), + NewGitCmd("diff"). + Arg("--submodule", "--no-ext-diff", "--color"). + Arg(diffArgs...). + Dir(self.repoPaths.worktreePath). + ToArgv(), ) } func (self *DiffCommands) internalDiffCmdObj(diffArgs ...string) *GitCommandBuilder { return NewGitCmd("diff"). Arg("--no-ext-diff", "--no-color"). - Arg(diffArgs...) + Arg(diffArgs...). + Dir(self.repoPaths.worktreePath) } func (self *DiffCommands) GetPathDiff(path string, staged bool) (string, error) { diff --git a/pkg/commands/git_commands/repo_paths.go b/pkg/commands/git_commands/repo_paths.go index 13cda86a451..b0e1970dbe8 100644 --- a/pkg/commands/git_commands/repo_paths.go +++ b/pkg/commands/git_commands/repo_paths.go @@ -1,21 +1,18 @@ package git_commands import ( - "fmt" ioFs "io/fs" - "os" "path" "path/filepath" "strings" "github.com/go-errors/errors" - "github.com/jesseduffield/lazygit/pkg/env" - "github.com/samber/lo" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/spf13/afero" ) type RepoPaths struct { - currentPath string worktreePath string worktreeGitDirPath string repoPath string @@ -23,12 +20,7 @@ type RepoPaths struct { repoName string } -// Current working directory of the program. Currently, this will always -// be the same as WorktreePath(), but in future we may support running -// lazygit from inside a subdirectory of the worktree. -func (self *RepoPaths) CurrentPath() string { - return self.currentPath -} +var gitPathFormatVersion GitVersion = GitVersion{2, 31, 0, ""} // Path to the current worktree. If we're in the main worktree, this will // be the same as RepoPath() @@ -65,7 +57,6 @@ func (self *RepoPaths) RepoName() string { // Returns the repo paths for a typical repo func MockRepoPaths(currentPath string) *RepoPaths { return &RepoPaths{ - currentPath: currentPath, worktreePath: currentPath, worktreeGitDirPath: path.Join(currentPath, ".git"), repoPath: currentPath, @@ -75,44 +66,41 @@ func MockRepoPaths(currentPath string) *RepoPaths { } func GetRepoPaths( - fs afero.Fs, - currentPath string, -) (*RepoPaths, error) { - return getRepoPathsAux(afero.NewOsFs(), resolveSymlink, currentPath) -} - -func getRepoPathsAux( - fs afero.Fs, - resolveSymlinkFn func(string) (string, error), - currentPath string, + cmd oscommands.ICmdObjBuilder, + version *GitVersion, ) (*RepoPaths, error) { - worktreePath := currentPath - repoGitDirPath, repoPath, err := getCurrentRepoGitDirPath(fs, resolveSymlinkFn, currentPath) + gitDirOutput, err := callGitRevParse(cmd, version, "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree") if err != nil { - return nil, errors.Errorf("failed to get repo git dir path: %v", err) + return nil, err } - var worktreeGitDirPath string - if env.GetWorkTreeEnv() != "" { - // This env is set when you pass --work-tree to lazygit. In that case, - // we're not dealing with a linked work-tree, we're dealing with a 'specified' - // worktree (for lack of a better term). In this case, the worktree has no - // .git file and it just contains a bunch of files: it has no idea it's - // pointed to by a bare repo. As such it does not have its own git dir within - // the bare repo's git dir. Instead, we just use the bare repo's git dir. - worktreeGitDirPath = repoGitDirPath - } else { - var err error - worktreeGitDirPath, err = getWorktreeGitDirPath(fs, currentPath) + gitDirResults := strings.Split(utils.NormalizeLinefeeds(gitDirOutput), "\n") + worktreePath := gitDirResults[0] + worktreeGitDirPath := gitDirResults[1] + repoGitDirPath := gitDirResults[2] + if version.IsOlderThanVersion(&gitPathFormatVersion) { + repoGitDirPath, err = filepath.Abs(repoGitDirPath) if err != nil { - return nil, errors.Errorf("failed to get worktree git dir path: %v", err) + return nil, err } } + // If we're in a submodule, --show-superproject-working-tree will return + // a value, meaning gitDirResults will be length 4. In that case + // return the worktree path as the repoPath. Otherwise we're in a + // normal repo or a worktree so return the parent of the git common + // dir (repoGitDirPath) + isSubmodule := len(gitDirResults) == 4 + + var repoPath string + if isSubmodule { + repoPath = worktreePath + } else { + repoPath = path.Dir(repoGitDirPath) + } repoName := path.Base(repoPath) return &RepoPaths{ - currentPath: currentPath, worktreePath: worktreePath, worktreeGitDirPath: worktreeGitDirPath, repoPath: repoPath, @@ -121,124 +109,31 @@ func getRepoPathsAux( }, nil } -// Returns the path of the git-dir for the worktree. For linked worktrees, the worktree has -// a .git file that points to the git-dir (which itself lives in the git-dir -// of the repo) -func getWorktreeGitDirPath(fs afero.Fs, worktreePath string) (string, error) { - // if .git is a file, we're in a linked worktree, otherwise we're in - // the main worktree - dotGitPath := path.Join(worktreePath, ".git") - gitFileInfo, err := fs.Stat(dotGitPath) - if err != nil { - return "", err - } - - if gitFileInfo.IsDir() { - return dotGitPath, nil - } - - return linkedWorktreeGitDirPath(fs, worktreePath) +func callGitRevParse( + cmd oscommands.ICmdObjBuilder, + version *GitVersion, + gitRevArgs ...string, +) (string, error) { + return callGitRevParseWithDir(cmd, version, "", gitRevArgs...) } -func linkedWorktreeGitDirPath(fs afero.Fs, worktreePath string) (string, error) { - dotGitPath := path.Join(worktreePath, ".git") - gitFileContents, err := afero.ReadFile(fs, dotGitPath) - if err != nil { - return "", err - } - - // The file will have `gitdir: /path/to/.git/worktrees/` - gitDirLine := lo.Filter(strings.Split(string(gitFileContents), "\n"), func(line string, _ int) bool { - return strings.HasPrefix(line, "gitdir: ") - }) - - if len(gitDirLine) == 0 { - return "", errors.New(fmt.Sprintf("%s is a file which suggests we are in a submodule or a worktree but the file's contents do not contain a gitdir pointing to the actual .git directory", dotGitPath)) - } - - gitDir := strings.TrimPrefix(gitDirLine[0], "gitdir: ") - - gitDir = filepath.Clean(gitDir) - // For windows support - gitDir = filepath.ToSlash(gitDir) - - return gitDir, nil -} - -func getCurrentRepoGitDirPath( - fs afero.Fs, - resolveSymlinkFn func(string) (string, error), - currentPath string, -) (string, string, error) { - var unresolvedGitPath string - if env.GetGitDirEnv() != "" { - unresolvedGitPath = env.GetGitDirEnv() - } else { - unresolvedGitPath = path.Join(currentPath, ".git") +func callGitRevParseWithDir( + cmd oscommands.ICmdObjBuilder, + version *GitVersion, + dir string, + gitRevArgs ...string, +) (string, error) { + gitRevParse := NewGitCmd("rev-parse").ArgIf(version.IsAtLeastVersion(&gitPathFormatVersion), "--path-format=absolute").Arg(gitRevArgs...) + if dir != "" { + gitRevParse.Dir(dir) } - gitPath, err := resolveSymlinkFn(unresolvedGitPath) + gitCmd := cmd.New(gitRevParse.ToArgv()).DontLog() + res, err := gitCmd.RunWithOutput() if err != nil { - return "", "", err + return "", errors.Errorf("'%s' failed: %v", gitCmd.ToString(), err) } - - // check if .git is a file or a directory - gitFileInfo, err := fs.Stat(gitPath) - if err != nil { - return "", "", err - } - - if gitFileInfo.IsDir() { - // must be in the main worktree - return gitPath, path.Dir(gitPath), nil - } - - // either in a submodule, or worktree - worktreeGitPath, err := linkedWorktreeGitDirPath(fs, currentPath) - if err != nil { - return "", "", errors.Errorf("could not find git dir for %s: %v", currentPath, err) - } - - _, err = fs.Stat(worktreeGitPath) - if err != nil { - if os.IsNotExist(err) { - // hardcoding error to get around windows-specific error message - return "", "", errors.Errorf("could not find git dir for %s. %s does not exist", currentPath, worktreeGitPath) - } - return "", "", errors.Errorf("could not find git dir for %s: %v", currentPath, err) - } - - // confirm whether the next directory up is the worktrees directory - parent := path.Dir(worktreeGitPath) - if path.Base(parent) == "worktrees" { - gitDirPath := path.Dir(parent) - return gitDirPath, path.Dir(gitDirPath), nil - } - - // Unlike worktrees, submodules can be nested arbitrarily deep, so we check - // if the `modules` directory is anywhere up the chain. - if strings.Contains(worktreeGitPath, "/modules/") { - // For submodules, we just return the path directly - return worktreeGitPath, currentPath, nil - } - - // If this error causes issues, we could relax the constraint and just always - // return the path - return "", "", errors.Errorf("could not find git dir for %s: the path '%s' is not under `worktrees` or `modules` directories", currentPath, worktreeGitPath) -} - -// takes a path containing a symlink and returns the true path -func resolveSymlink(path string) (string, error) { - l, err := os.Lstat(path) - if err != nil { - return "", err - } - - if l.Mode()&os.ModeSymlink == 0 { - return path, nil - } - - return filepath.EvalSymlinks(path) + return strings.TrimSpace(res), nil } // Returns the paths of linked worktrees diff --git a/pkg/commands/git_commands/repo_paths_test.go b/pkg/commands/git_commands/repo_paths_test.go index 5e6275522e9..ae452673771 100644 --- a/pkg/commands/git_commands/repo_paths_test.go +++ b/pkg/commands/git_commands/repo_paths_test.go @@ -1,34 +1,50 @@ package git_commands import ( + "fmt" + "strings" "testing" "github.com/go-errors/errors" - "github.com/spf13/afero" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/stretchr/testify/assert" ) -func mockResolveSymlinkFn(p string) (string, error) { return p, nil } +type ( + argFn func() []string + errFn func(getRevParseArgs argFn) error +) type Scenario struct { Name string - BeforeFunc func(fs afero.Fs) + BeforeFunc func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) Path string Expected *RepoPaths - Err error + Err errFn } -func TestGetRepoPathsAux(t *testing.T) { +func TestGetRepoPaths(t *testing.T) { scenarios := []Scenario{ { Name: "typical case", - BeforeFunc: func(fs afero.Fs) { + BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) { // setup for main worktree - _ = fs.MkdirAll("/path/to/repo/.git", 0o755) + expectedOutput := []string{ + // --show-toplevel + "/path/to/repo", + // --git-dir + "/path/to/repo/.git", + // --git-common-dir + "/path/to/repo/.git", + // --show-superproject-working-tree + } + runner.ExpectGitArgs( + append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree"), + strings.Join(expectedOutput, "\n"), + nil) }, Path: "/path/to/repo", Expected: &RepoPaths{ - currentPath: "/path/to/repo", worktreePath: "/path/to/repo", worktreeGitDirPath: "/path/to/repo/.git", repoPath: "/path/to/repo", @@ -37,71 +53,26 @@ func TestGetRepoPathsAux(t *testing.T) { }, Err: nil, }, - { - Name: "linked worktree", - BeforeFunc: func(fs afero.Fs) { - // setup for linked worktree - _ = fs.MkdirAll("/path/to/repo/.git/worktrees/worktree1", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/worktree1/.git", []byte("gitdir: /path/to/repo/.git/worktrees/worktree1"), 0o644) - }, - Path: "/path/to/repo/worktree1", - Expected: &RepoPaths{ - currentPath: "/path/to/repo/worktree1", - worktreePath: "/path/to/repo/worktree1", - worktreeGitDirPath: "/path/to/repo/.git/worktrees/worktree1", - repoPath: "/path/to/repo", - repoGitDirPath: "/path/to/repo/.git", - repoName: "repo", - }, - Err: nil, - }, - { - Name: "worktree with trailing separator in path", - BeforeFunc: func(fs afero.Fs) { - // setup for linked worktree - _ = fs.MkdirAll("/path/to/repo/.git/worktrees/worktree1", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/worktree1/.git", []byte("gitdir: /path/to/repo/.git/worktrees/worktree1/"), 0o644) - }, - Path: "/path/to/repo/worktree1", - Expected: &RepoPaths{ - currentPath: "/path/to/repo/worktree1", - worktreePath: "/path/to/repo/worktree1", - worktreeGitDirPath: "/path/to/repo/.git/worktrees/worktree1", - repoPath: "/path/to/repo", - repoGitDirPath: "/path/to/repo/.git", - repoName: "repo", - }, - Err: nil, - }, - { - Name: "worktree .git file missing gitdir directive", - BeforeFunc: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/repo/.git/worktrees/worktree2", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/worktree2/.git", []byte("blah"), 0o644) - }, - Path: "/path/to/repo/worktree2", - Expected: nil, - Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/worktree2: /path/to/repo/worktree2/.git is a file which suggests we are in a submodule or a worktree but the file's contents do not contain a gitdir pointing to the actual .git directory"), - }, - { - Name: "worktree .git file gitdir directive points to a non-existing directory", - BeforeFunc: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/repo/.git/worktrees/worktree2", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/worktree2/.git", []byte("gitdir: /nonexistant"), 0o644) - }, - Path: "/path/to/repo/worktree2", - Expected: nil, - Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/worktree2. /nonexistant does not exist"), - }, { Name: "submodule", - BeforeFunc: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/repo/.git/modules/submodule1", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/submodule1/.git", []byte("gitdir: /path/to/repo/.git/modules/submodule1"), 0o644) + BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) { + expectedOutput := []string{ + // --show-toplevel + "/path/to/repo/submodule1", + // --git-dir + "/path/to/repo/.git/modules/submodule1", + // --git-common-dir + "/path/to/repo/.git/modules/submodule1", + // --show-superproject-working-tree + "/path/to/repo", + } + runner.ExpectGitArgs( + append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree"), + strings.Join(expectedOutput, "\n"), + nil) }, Path: "/path/to/repo/submodule1", Expected: &RepoPaths{ - currentPath: "/path/to/repo/submodule1", worktreePath: "/path/to/repo/submodule1", worktreeGitDirPath: "/path/to/repo/.git/modules/submodule1", repoPath: "/path/to/repo/submodule1", @@ -111,49 +82,52 @@ func TestGetRepoPathsAux(t *testing.T) { Err: nil, }, { - Name: "submodule in nested directory", - BeforeFunc: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/repo/.git/modules/my/submodule1", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/my/submodule1/.git", []byte("gitdir: /path/to/repo/.git/modules/my/submodule1"), 0o644) - }, - Path: "/path/to/repo/my/submodule1", - Expected: &RepoPaths{ - currentPath: "/path/to/repo/my/submodule1", - worktreePath: "/path/to/repo/my/submodule1", - worktreeGitDirPath: "/path/to/repo/.git/modules/my/submodule1", - repoPath: "/path/to/repo/my/submodule1", - repoGitDirPath: "/path/to/repo/.git/modules/my/submodule1", - repoName: "submodule1", - }, - Err: nil, - }, - { - Name: "submodule git dir not under .git/modules", - BeforeFunc: func(fs afero.Fs) { - _ = fs.MkdirAll("/random/submodule1", 0o755) - _ = afero.WriteFile(fs, "/path/to/repo/my/submodule1/.git", []byte("gitdir: /random/submodule1"), 0o644) + Name: "git rev-parse returns an error", + BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) { + runner.ExpectGitArgs( + append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree"), + "", + errors.New("fatal: invalid gitfile format: /path/to/repo/worktree2/.git")) }, - Path: "/path/to/repo/my/submodule1", + Path: "/path/to/repo/worktree2", Expected: nil, - Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/my/submodule1: the path '/random/submodule1' is not under `worktrees` or `modules` directories"), + Err: func(getRevParseArgs argFn) error { + args := strings.Join(getRevParseArgs(), " ") + return errors.New( + fmt.Sprintf("'git %v --show-toplevel --absolute-git-dir --git-common-dir --show-superproject-working-tree' failed: fatal: invalid gitfile format: /path/to/repo/worktree2/.git", args), + ) + }, }, } for _, s := range scenarios { s := s t.Run(s.Name, func(t *testing.T) { - fs := afero.NewMemMapFs() + runner := oscommands.NewFakeRunner(t) + cmd := oscommands.NewDummyCmdObjBuilder(runner) + version, err := GetGitVersion(oscommands.NewDummyOSCommand()) + if err != nil { + t.Fatal(err) + } + + getRevParseArgs := func() []string { + args := []string{"rev-parse"} + if version.IsAtLeast(2, 31, 0) { + args = append(args, "--path-format=absolute") + } + return args + } // prepare the filesystem for the scenario - s.BeforeFunc(fs) + s.BeforeFunc(runner, getRevParseArgs) - // run the function with the scenario path - repoPaths, err := getRepoPathsAux(fs, mockResolveSymlinkFn, s.Path) + repoPaths, err := GetRepoPaths(cmd, version) // check the error and the paths if s.Err != nil { + scenarioErr := s.Err(getRevParseArgs) assert.Error(t, err) - assert.EqualError(t, err, s.Err.Error()) + assert.EqualError(t, err, scenarioErr.Error()) } else { assert.Nil(t, err) assert.Equal(t, s.Expected, repoPaths) diff --git a/pkg/commands/git_commands/stash.go b/pkg/commands/git_commands/stash.go index fad73a472a9..29140e68e76 100644 --- a/pkg/commands/git_commands/stash.go +++ b/pkg/commands/git_commands/stash.go @@ -88,6 +88,7 @@ func (self *StashCommands) ShowStashEntryCmdObj(index int) oscommands.ICmdObj { Arg(fmt.Sprintf("--unified=%d", self.AppState.DiffContextSize)). ArgIf(self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). Arg(fmt.Sprintf("stash@{%d}", index)). + Dir(self.repoPaths.worktreePath). ToArgv() return self.cmd.New(cmdArgs).DontLog() diff --git a/pkg/commands/git_commands/stash_test.go b/pkg/commands/git_commands/stash_test.go index 846c493ee54..6954a3cf817 100644 --- a/pkg/commands/git_commands/stash_test.go +++ b/pkg/commands/git_commands/stash_test.go @@ -112,21 +112,21 @@ func TestStashStashEntryCmdObj(t *testing.T) { index: 5, contextSize: 3, ignoreWhitespace: false, - expected: []string{"git", "stash", "show", "-p", "--stat", "--color=always", "--unified=3", "stash@{5}"}, + expected: []string{"git", "-C", "/path/to/worktree", "stash", "show", "-p", "--stat", "--color=always", "--unified=3", "stash@{5}"}, }, { testName: "Show diff with custom context size", index: 5, contextSize: 77, ignoreWhitespace: false, - expected: []string{"git", "stash", "show", "-p", "--stat", "--color=always", "--unified=77", "stash@{5}"}, + expected: []string{"git", "-C", "/path/to/worktree", "stash", "show", "-p", "--stat", "--color=always", "--unified=77", "stash@{5}"}, }, { testName: "Default case", index: 5, contextSize: 3, ignoreWhitespace: true, - expected: []string{"git", "stash", "show", "-p", "--stat", "--color=always", "--unified=3", "--ignore-all-space", "stash@{5}"}, + expected: []string{"git", "-C", "/path/to/worktree", "stash", "show", "-p", "--stat", "--color=always", "--unified=3", "--ignore-all-space", "stash@{5}"}, }, } @@ -137,7 +137,10 @@ func TestStashStashEntryCmdObj(t *testing.T) { appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace appState.DiffContextSize = s.contextSize - instance := buildStashCommands(commonDeps{userConfig: userConfig, appState: appState}) + repoPaths := RepoPaths{ + worktreePath: "/path/to/worktree", + } + instance := buildStashCommands(commonDeps{userConfig: userConfig, appState: appState, repoPaths: &repoPaths}) cmdStr := instance.ShowStashEntryCmdObj(s.index).Args() assert.Equal(t, s.expected, cmdStr) diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index 9630e2870ee..51ad417aa1c 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -255,6 +255,7 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain ArgIf(noIndex, "/dev/null"). Arg(node.GetPath()). ArgIf(prevPath != "", prevPath). + Dir(self.repoPaths.worktreePath). ToArgv() return self.cmd.New(cmdArgs).DontLog() @@ -290,6 +291,7 @@ func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reve ArgIf(!plain && self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space"). Arg("--"). Arg(fileName). + Dir(self.repoPaths.worktreePath). ToArgv() return self.cmd.New(cmdArgs).DontLog() diff --git a/pkg/commands/git_commands/working_tree_test.go b/pkg/commands/git_commands/working_tree_test.go index cc46d4994b6..51fa88b7ff2 100644 --- a/pkg/commands/git_commands/working_tree_test.go +++ b/pkg/commands/git_commands/working_tree_test.go @@ -231,7 +231,7 @@ func TestWorkingTreeDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--", "test.txt"}, expectedResult, nil), }, { testName: "cached", @@ -245,7 +245,7 @@ func TestWorkingTreeDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--cached", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--cached", "--", "test.txt"}, expectedResult, nil), }, { testName: "plain", @@ -259,7 +259,7 @@ func TestWorkingTreeDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=never", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=never", "--", "test.txt"}, expectedResult, nil), }, { testName: "File not tracked and file has no staged changes", @@ -273,7 +273,7 @@ func TestWorkingTreeDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--no-index", "--", "/dev/null", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--no-index", "--", "/dev/null", "test.txt"}, expectedResult, nil), }, { testName: "Default case (ignore whitespace)", @@ -287,7 +287,7 @@ func TestWorkingTreeDiff(t *testing.T) { ignoreWhitespace: true, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--ignore-all-space", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--color=always", "--ignore-all-space", "--", "test.txt"}, expectedResult, nil), }, { testName: "Show diff with custom context size", @@ -301,7 +301,7 @@ func TestWorkingTreeDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 17, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=17", "--color=always", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=17", "--color=always", "--", "test.txt"}, expectedResult, nil), }, } @@ -312,8 +312,11 @@ func TestWorkingTreeDiff(t *testing.T) { appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace appState.DiffContextSize = s.contextSize + repoPaths := RepoPaths{ + worktreePath: "/path/to/worktree", + } - instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState}) + instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState, repoPaths: &repoPaths}) result := instance.WorktreeFileDiff(s.file, s.plain, s.cached) assert.Equal(t, expectedResult, result) s.runner.CheckForMissingCalls() @@ -345,7 +348,7 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--no-renames", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--no-renames", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), }, { testName: "Show diff with custom context size", @@ -356,7 +359,7 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { ignoreWhitespace: false, contextSize: 123, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=123", "--no-renames", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=123", "--no-renames", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), }, { testName: "Default case (ignore whitespace)", @@ -367,7 +370,7 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { ignoreWhitespace: true, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"diff", "--no-ext-diff", "--submodule", "--unified=3", "--no-renames", "--color=always", "1234567890", "0987654321", "--ignore-all-space", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--no-renames", "--color=always", "1234567890", "0987654321", "--ignore-all-space", "--", "test.txt"}, expectedResult, nil), }, } @@ -378,8 +381,11 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { appState := &config.AppState{} appState.IgnoreWhitespaceInDiffView = s.ignoreWhitespace appState.DiffContextSize = s.contextSize + repoPaths := RepoPaths{ + worktreePath: "/path/to/worktree", + } - instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState}) + instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: appState, repoPaths: &repoPaths}) result, err := instance.ShowFileDiff(s.from, s.to, s.reverse, "test.txt", s.plain) assert.NoError(t, err) diff --git a/pkg/commands/git_commands/worktree_loader.go b/pkg/commands/git_commands/worktree_loader.go index 0d5f01e3433..97839662cab 100644 --- a/pkg/commands/git_commands/worktree_loader.go +++ b/pkg/commands/git_commands/worktree_loader.go @@ -4,6 +4,7 @@ import ( iofs "io/fs" "path/filepath" "strings" + "sync" "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/models" @@ -57,18 +58,14 @@ func (self *WorktreeLoader) GetWorktrees() ([]*models.Worktree, error) { isCurrent := path == worktreePath isPathMissing := self.pathExists(path) - var gitDir string - gitDir, err := getWorktreeGitDirPath(self.Fs, path) - if err != nil { - self.Log.Warnf("Could not find git dir for worktree %s: %v", path, err) - } - current = &models.Worktree{ IsMain: isMain, IsCurrent: isCurrent, IsPathMissing: isPathMissing, Path: path, - GitDir: gitDir, + // we defer populating GitDir until a loop below so that + // we can parallelize the calls to git rev-parse + GitDir: "", } } else if strings.HasPrefix(splitLine, "branch ") { branch := strings.SplitN(splitLine, " ", 2)[1] @@ -76,6 +73,28 @@ func (self *WorktreeLoader) GetWorktrees() ([]*models.Worktree, error) { } } + wg := sync.WaitGroup{} + wg.Add(len(worktrees)) + for _, worktree := range worktrees { + worktree := worktree + + go utils.Safe(func() { + defer wg.Done() + + if worktree.IsPathMissing { + return + } + gitDir, err := callGitRevParseWithDir(self.cmd, self.version, worktree.Path, "--absolute-git-dir") + if err != nil { + self.Log.Warnf("Could not find git dir for worktree %s: %v", worktree.Path, err) + return + } + + worktree.GitDir = gitDir + }) + } + wg.Wait() + names := getUniqueNamesFromPaths(lo.Map(worktrees, func(worktree *models.Worktree, _ int) string { return worktree.Path })) diff --git a/pkg/commands/git_commands/worktree_loader_test.go b/pkg/commands/git_commands/worktree_loader_test.go index 12b30807496..02ed73e865b 100644 --- a/pkg/commands/git_commands/worktree_loader_test.go +++ b/pkg/commands/git_commands/worktree_loader_test.go @@ -14,7 +14,7 @@ func TestGetWorktrees(t *testing.T) { type scenario struct { testName string repoPaths *RepoPaths - before func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs) + before func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs, getRevParseArgs argFn) expectedWorktrees []*models.Worktree expectedErr string } @@ -26,7 +26,7 @@ func TestGetWorktrees(t *testing.T) { repoPath: "/path/to/repo", worktreePath: "/path/to/repo", }, - before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs) { + before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs, getRevParseArgs argFn) { runner.ExpectGitArgs([]string{"worktree", "list", "--porcelain"}, `worktree /path/to/repo HEAD d85cc9d281fa6ae1665c68365fc70e75e82a042d @@ -34,6 +34,8 @@ branch refs/heads/mybranch `, nil) + gitArgsMainWorktree := append(append([]string{"-C", "/path/to/repo"}, getRevParseArgs()...), "--absolute-git-dir") + runner.ExpectGitArgs(gitArgsMainWorktree, "/path/to/repo/.git", nil) _ = fs.MkdirAll("/path/to/repo/.git", 0o755) }, expectedWorktrees: []*models.Worktree{ @@ -55,7 +57,7 @@ branch refs/heads/mybranch repoPath: "/path/to/repo", worktreePath: "/path/to/repo", }, - before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs) { + before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs, getRevParseArgs argFn) { runner.ExpectGitArgs([]string{"worktree", "list", "--porcelain"}, `worktree /path/to/repo HEAD d85cc9d281fa6ae1665c68365fc70e75e82a042d @@ -66,6 +68,10 @@ HEAD 775955775e79b8f5b4c4b56f82fbf657e2d5e4de branch refs/heads/mybranch-worktree `, nil) + gitArgsMainWorktree := append(append([]string{"-C", "/path/to/repo"}, getRevParseArgs()...), "--absolute-git-dir") + runner.ExpectGitArgs(gitArgsMainWorktree, "/path/to/repo/.git", nil) + gitArgsLinkedWorktree := append(append([]string{"-C", "/path/to/repo-worktree"}, getRevParseArgs()...), "--absolute-git-dir") + runner.ExpectGitArgs(gitArgsLinkedWorktree, "/path/to/repo/.git/worktrees/repo-worktree", nil) _ = fs.MkdirAll("/path/to/repo/.git", 0o755) _ = fs.MkdirAll("/path/to/repo-worktree", 0o755) @@ -100,7 +106,7 @@ branch refs/heads/mybranch-worktree repoPath: "/path/to/repo", worktreePath: "/path/to/repo", }, - before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs) { + before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs, getRevParseArgs argFn) { runner.ExpectGitArgs([]string{"worktree", "list", "--porcelain"}, `worktree /path/to/worktree HEAD 775955775e79b8f5b4c4b56f82fbf657e2d5e4de @@ -129,7 +135,7 @@ branch refs/heads/missingbranch repoPath: "/path/to/repo", worktreePath: "/path/to/repo-worktree", }, - before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs) { + before: func(runner *oscommands.FakeCmdObjRunner, fs afero.Fs, getRevParseArgs argFn) { runner.ExpectGitArgs([]string{"worktree", "list", "--porcelain"}, `worktree /path/to/repo HEAD d85cc9d281fa6ae1665c68365fc70e75e82a042d @@ -140,6 +146,10 @@ HEAD 775955775e79b8f5b4c4b56f82fbf657e2d5e4de branch refs/heads/mybranch-worktree `, nil) + gitArgsMainWorktree := append(append([]string{"-C", "/path/to/repo"}, getRevParseArgs()...), "--absolute-git-dir") + runner.ExpectGitArgs(gitArgsMainWorktree, "/path/to/repo/.git", nil) + gitArgsLinkedWorktree := append(append([]string{"-C", "/path/to/repo-worktree"}, getRevParseArgs()...), "--absolute-git-dir") + runner.ExpectGitArgs(gitArgsLinkedWorktree, "/path/to/repo/.git/worktrees/repo-worktree", nil) _ = fs.MkdirAll("/path/to/repo/.git", 0o755) _ = fs.MkdirAll("/path/to/repo-worktree", 0o755) @@ -175,10 +185,23 @@ branch refs/heads/mybranch-worktree t.Run(s.testName, func(t *testing.T) { runner := oscommands.NewFakeRunner(t) fs := afero.NewMemMapFs() - s.before(runner, fs) + version, err := GetGitVersion(oscommands.NewDummyOSCommand()) + if err != nil { + t.Fatal(err) + } + + getRevParseArgs := func() []string { + args := []string{"rev-parse"} + if version.IsAtLeast(2, 31, 0) { + args = append(args, "--path-format=absolute") + } + return args + } + + s.before(runner, fs, getRevParseArgs) loader := &WorktreeLoader{ - GitCommon: buildGitCommon(commonDeps{runner: runner, fs: fs, repoPaths: s.repoPaths}), + GitCommon: buildGitCommon(commonDeps{runner: runner, fs: fs, repoPaths: s.repoPaths, gitVersion: version}), } worktrees, err := loader.GetWorktrees() diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go deleted file mode 100644 index f4dc27dedbe..00000000000 --- a/pkg/commands/git_test.go +++ /dev/null @@ -1,74 +0,0 @@ -package commands - -import ( - "testing" - - "github.com/go-errors/errors" - "github.com/spf13/afero" - "github.com/stretchr/testify/assert" -) - -func TestFindWorktreeRoot(t *testing.T) { - type scenario struct { - testName string - currentPath string - before func(fs afero.Fs) - expectedPath string - expectedErr string - } - - scenarios := []scenario{ - { - testName: "at root of worktree", - currentPath: "/path/to/repo", - before: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/repo/.git", 0o755) - }, - expectedPath: "/path/to/repo", - expectedErr: "", - }, - { - testName: "inside worktree", - currentPath: "/path/to/repo/subdir", - before: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/repo/.git", 0o755) - _ = fs.MkdirAll("/path/to/repo/subdir", 0o755) - }, - expectedPath: "/path/to/repo", - expectedErr: "", - }, - { - testName: "not in a git repo", - currentPath: "/path/to/dir", - before: func(fs afero.Fs) {}, - expectedPath: "", - expectedErr: "Must open lazygit in a git repository", - }, - { - testName: "In linked worktree", - currentPath: "/path/to/worktree", - before: func(fs afero.Fs) { - _ = fs.MkdirAll("/path/to/worktree", 0o755) - _ = afero.WriteFile(fs, "/path/to/worktree/.git", []byte("blah"), 0o755) - }, - expectedPath: "/path/to/worktree", - expectedErr: "", - }, - } - - for _, s := range scenarios { - s := s - t.Run(s.testName, func(t *testing.T) { - fs := afero.NewMemMapFs() - s.before(fs) - - root, err := findWorktreeRoot(fs, s.currentPath) - if s.expectedErr != "" { - assert.EqualError(t, errors.New(s.expectedErr), err.Error()) - } else { - assert.NoError(t, err) - assert.Equal(t, s.expectedPath, root) - } - }) - } -} diff --git a/pkg/commands/oscommands/cmd_obj_builder.go b/pkg/commands/oscommands/cmd_obj_builder.go index 77fb7e7c964..540487ed46e 100644 --- a/pkg/commands/oscommands/cmd_obj_builder.go +++ b/pkg/commands/oscommands/cmd_obj_builder.go @@ -27,8 +27,14 @@ type CmdObjBuilder struct { var _ ICmdObjBuilder = &CmdObjBuilder{} func (self *CmdObjBuilder) New(args []string) ICmdObj { + cmdObj := self.NewWithEnviron(args, os.Environ()) + return cmdObj +} + +// A command with explicit environment from env +func (self *CmdObjBuilder) NewWithEnviron(args []string, env []string) ICmdObj { cmd := exec.Command(args[0], args[1:]...) - cmd.Env = os.Environ() + cmd.Env = env return &CmdObj{ cmd: cmd, diff --git a/pkg/integration/components/env.go b/pkg/integration/components/env.go new file mode 100644 index 00000000000..d3cdf467f60 --- /dev/null +++ b/pkg/integration/components/env.go @@ -0,0 +1,65 @@ +package components + +import ( + "fmt" + "os" +) + +const ( + // These values will be passed to lazygit + LAZYGIT_ROOT_DIR = "LAZYGIT_ROOT_DIR" + SANDBOX_ENV_VAR = "SANDBOX" + TEST_NAME_ENV_VAR = "TEST_NAME" + WAIT_FOR_DEBUGGER_ENV_VAR = "WAIT_FOR_DEBUGGER" + + // These values will be passed to both lazygit and shell commands + GIT_CONFIG_GLOBAL_ENV_VAR = "GIT_CONFIG_GLOBAL" + // We pass PWD because if it's defined, Go will use it as the working directory + // rather than make a syscall to the OS, and that means symlinks won't be resolved, + // which is good to test for. + PWD = "PWD" + + // We set $HOME and $GIT_CONFIG_NOGLOBAL during integrationt tests so + // that older versions of git that don't respect $GIT_CONFIG_GLOBAL + // will find the correct global config file for testing + HOME = "HOME" + GIT_CONFIG_NOGLOBAL = "GIT_CONFIG_NOGLOBAL" + + // These values will be passed through to lazygit and shell commands, with their + // values inherited from the host environment + PATH = "PATH" + TERM = "TERM" +) + +// Tests will inherit these environment variables from the host environment, rather +// than the test runner deciding the values itself. +// All other environment variables present in the host environment will be ignored. +// Having such a minimal list ensures that lazygit behaves the same across different test environments. +var hostEnvironmentAllowlist = [...]string{ + PATH, + TERM, +} + +// Returns a copy of the environment filtered by +// hostEnvironmentAllowlist +func allowedHostEnvironment() []string { + env := []string{} + for _, envVar := range hostEnvironmentAllowlist { + env = append(env, fmt.Sprintf("%s=%s", envVar, os.Getenv(envVar))) + } + return env +} + +func NewTestEnvironment(rootDir string) []string { + env := allowedHostEnvironment() + + // Set $HOME to control the global git config location for git + // versions <= 2.31.8 + env = append(env, fmt.Sprintf("%s=%s", HOME, testPath(rootDir))) + + // $GIT_CONFIG_GLOBAL controls global git config location for git + // versions >= 2.32.0 + env = append(env, fmt.Sprintf("%s=%s", GIT_CONFIG_GLOBAL_ENV_VAR, globalGitConfigPath(rootDir))) + + return env +} diff --git a/pkg/integration/components/runner.go b/pkg/integration/components/runner.go index 064dd5c5b26..c148dc14163 100644 --- a/pkg/integration/components/runner.go +++ b/pkg/integration/components/runner.go @@ -13,14 +13,6 @@ import ( "github.com/samber/lo" ) -const ( - LAZYGIT_ROOT_DIR = "LAZYGIT_ROOT_DIR" - TEST_NAME_ENV_VAR = "TEST_NAME" - SANDBOX_ENV_VAR = "SANDBOX" - WAIT_FOR_DEBUGGER_ENV_VAR = "WAIT_FOR_DEBUGGER" - GIT_CONFIG_GLOBAL_ENV_VAR = "GIT_CONFIG_GLOBAL" -) - type RunTestArgs struct { Tests []*IntegrationTest Logf func(format string, formatArgs ...interface{}) @@ -161,18 +153,27 @@ func buildLazygit(testArgs RunTestArgs) error { // Sets up the fixture for test and returns the working directory to invoke // lazygit in. func createFixture(test *IntegrationTest, paths Paths, rootDir string) string { - shell := NewShell(paths.ActualRepo(), func(errorMsg string) { panic(errorMsg) }) + env := NewTestEnvironment(rootDir) + + env = append(env, fmt.Sprintf("%s=%s", PWD, paths.ActualRepo())) + shell := NewShell( + paths.ActualRepo(), + env, + func(errorMsg string) { panic(errorMsg) }, + ) shell.Init() - os.Setenv(GIT_CONFIG_GLOBAL_ENV_VAR, globalGitConfigPath(rootDir)) - test.SetupRepo(shell) return shell.dir } +func testPath(rootdir string) string { + return filepath.Join(rootdir, "test") +} + func globalGitConfigPath(rootDir string) string { - return filepath.Join(rootDir, "test", "global_git_config") + return filepath.Join(testPath(rootDir), "global_git_config") } func getGitVersion() (*git_commands.GitVersion, error) { @@ -215,9 +216,16 @@ func getLazygitCommand( }) cmdArgs = append(cmdArgs, resolvedExtraArgs...) - cmdObj := osCommand.Cmd.New(cmdArgs) + // Use a limited environment for test isolation, including pass through + // of just allowed host environment variables + cmdObj := osCommand.Cmd.NewWithEnviron(cmdArgs, NewTestEnvironment(rootDir)) + // Integration tests related to symlink behavior need a PWD that + // preserves symlinks. By default, SetWd will set a symlink-resolved + // value for PWD. Here, we override that with the path (that may) + // contain a symlink to simulate behavior in a user's shell correctly. cmdObj.SetWd(workingDir) + cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", PWD, workingDir)) cmdObj.AddEnvVars(fmt.Sprintf("%s=%s", LAZYGIT_ROOT_DIR, rootDir)) diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index 5a3bbaa29a8..48ff3fdf734 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -19,6 +19,9 @@ import ( type Shell struct { // working directory the shell is invoked in dir string + // passed into each command + env []string + // when running the shell outside the gui we can directly panic on failure, // but inside the gui we need to close the gui before panicking fail func(string) @@ -26,14 +29,15 @@ type Shell struct { randomFileContentIndex int } -func NewShell(dir string, fail func(string)) *Shell { - return &Shell{dir: dir, fail: fail} +func NewShell(dir string, env []string, fail func(string)) *Shell { + return &Shell{dir: dir, env: env, fail: fail} } func (self *Shell) RunCommand(args []string) *Shell { return self.RunCommandWithEnv(args, []string{}) } +// Run a command with additional environment variables set func (self *Shell) RunCommandWithEnv(args []string, env []string) *Shell { output, err := self.runCommandWithOutputAndEnv(args, env) if err != nil { @@ -58,7 +62,7 @@ func (self *Shell) runCommandWithOutput(args []string) (string, error) { func (self *Shell) runCommandWithOutputAndEnv(args []string, env []string) (string, error) { cmd := exec.Command(args[0], args[1:]...) - cmd.Env = append(os.Environ(), env...) + cmd.Env = append(self.env, env...) cmd.Dir = self.dir output, err := cmd.CombinedOutput() @@ -461,8 +465,8 @@ func (self *Shell) CopyFile(source string, destination string) *Shell { return self } -// NOTE: this only takes effect before running the test; -// the test will still run in the original directory +// The final value passed to Chdir() during setup +// will be the directory the test is run from. func (self *Shell) Chdir(path string) *Shell { self.dir = filepath.Join(self.dir, path) diff --git a/pkg/integration/components/test.go b/pkg/integration/components/test.go index 203436ae755..6eab23ae04a 100644 --- a/pkg/integration/components/test.go +++ b/pkg/integration/components/test.go @@ -182,7 +182,13 @@ func (self *IntegrationTest) Run(gui integrationTypes.GuiDriver) { panic(err) } - shell := NewShell(pwd, func(errorMsg string) { gui.Fail(errorMsg) }) + shell := NewShell( + pwd, + // passing the full environment because it's already been filtered down + // in the parent process. + os.Environ(), + func(errorMsg string) { gui.Fail(errorMsg) }, + ) keys := gui.Keys() testDriver := NewTestDriver(gui, shell, keys, InputDelay()) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index f0437b30da6..1acc97fb4b2 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -274,13 +274,16 @@ var tests = []*components.IntegrationTest{ worktree.AssociateBranchBisect, worktree.AssociateBranchRebase, worktree.BareRepo, + worktree.BareRepoWorktreeConfig, worktree.Crud, worktree.CustomCommand, worktree.DetachWorktreeFromBranch, worktree.DotfileBareRepo, + worktree.DoubleNestedLinkedSubmodule, worktree.FastForwardWorktreeBranch, worktree.ForceRemoveWorktree, worktree.RemoveWorktreeFromBranch, worktree.ResetWindowTabs, + worktree.SymlinkIntoRepoSubdir, worktree.WorktreeInRepo, } diff --git a/pkg/integration/tests/worktree/bare_repo_worktree_config.go b/pkg/integration/tests/worktree/bare_repo_worktree_config.go new file mode 100644 index 00000000000..ae468145521 --- /dev/null +++ b/pkg/integration/tests/worktree/bare_repo_worktree_config.go @@ -0,0 +1,92 @@ +package worktree + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// This case is identical to dotfile_bare_repo.go, except +// that it invokes lazygit with $GIT_DIR set but not +// $GIT_WORK_TREE. Instead, the repo uses the core.worktree +// config to identify the main worktre. + +var BareRepoWorktreeConfig = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Open lazygit in the worktree of a vcsh-style bare repo and add a file and commit", + ExtraCmdArgs: []string{"--git-dir={{.actualPath}}/.bare"}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.UserConfig.Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + // we're going to have a directory structure like this: + // project + // - .bare + // - . (a worktree at the same path as .bare) + // + // + // 'repo' is the repository/directory that all lazygit tests start in + + shell.CreateFileAndAdd("a/b/c/blah", "blah\n") + shell.Commit("initial commit") + + shell.CreateFileAndAdd(".gitignore", ".bare/\n/repo\n") + shell.Commit("add .gitignore") + + shell.Chdir("..") + + // configure this "fake bare"" repo using the vcsh convention + // of core.bare=false and core.worktree set to the actual + // worktree path (a homedir root). This allows $GIT_DIR + // alone to make this repo "self worktree identifying" + shell.RunCommand([]string{"git", "--git-dir=./.bare", "init", "--shared=false"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "config", "core.bare", "false"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "config", "core.worktree", ".."}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "remote", "add", "origin", "./repo"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "checkout", "-b", "main"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "config", "branch.main.remote", "origin"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "config", "branch.main.merge", "refs/heads/master"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "fetch", "origin", "master"}) + shell.RunCommand([]string{"git", "--git-dir=./.bare", "-c", "merge.ff=true", "merge", "origin/master"}) + + // we no longer need the original repo so remove it + shell.DeleteFile("repo") + + shell.UpdateFile("a/b/c/blah", "updated content\n") + shell.Chdir("a/b/c") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Lines( + Contains("main"), + ) + + t.Views().Commits(). + Lines( + Contains("add .gitignore"), + Contains("initial commit"), + ) + + t.Views().Files(). + IsFocused(). + Lines( + Contains(" M a/b/c/blah"), // shows as modified + ). + PressPrimaryAction(). + Press(keys.Files.CommitChanges) + + t.ExpectPopup().CommitMessagePanel(). + Title(Equals("Commit summary")). + Type("Add blah"). + Confirm() + + t.Views().Files(). + IsEmpty() + + t.Views().Commits(). + Lines( + Contains("Add blah"), + Contains("add .gitignore"), + Contains("initial commit"), + ) + }, +}) diff --git a/pkg/integration/tests/worktree/double_nested_linked_submodule.go b/pkg/integration/tests/worktree/double_nested_linked_submodule.go new file mode 100644 index 00000000000..c964251a966 --- /dev/null +++ b/pkg/integration/tests/worktree/double_nested_linked_submodule.go @@ -0,0 +1,93 @@ +package worktree + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// Even though this involves submodules, it's a worktree test since +// it's really exercising lazygit's ability to correctly do pathfinding +// in a complex use case. +var DoubleNestedLinkedSubmodule = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Open lazygit in a link to a repo's double nested submodules", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.UserConfig.Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + // we're going to have a directory structure like this: + // project + // - repo/outerSubmodule/innerSubmodule/a/b/c + // - link (symlink to repo/outerSubmodule/innerSubmodule/a/b/c) + // + shell.CreateFileAndAdd("rootFile", "rootStuff") + shell.Commit("initial repo commit") + + shell.Chdir("..") + shell.CreateDir("innerSubmodule") + shell.Chdir("innerSubmodule") + shell.Init() + shell.CreateFileAndAdd("a/b/c/blah", "blah\n") + shell.Commit("initial inner commit") + + shell.Chdir("..") + shell.CreateDir("outerSubmodule") + shell.Chdir("outerSubmodule") + shell.Init() + shell.CreateFileAndAdd("foo", "foo") + shell.Commit("initial outer commit") + // the git config (-c) parameter below is required + // to let git create a file-protocol/path submodule + shell.RunCommand([]string{"git", "-c", "protocol.file.allow=always", "submodule", "add", "../innerSubmodule"}) + shell.Commit("add dependency as innerSubmodule") + + shell.Chdir("../repo") + shell.RunCommand([]string{"git", "-c", "protocol.file.allow=always", "submodule", "add", "../outerSubmodule"}) + shell.Commit("add dependency as outerSubmodule") + shell.Chdir("outerSubmodule") + shell.RunCommand([]string{"git", "-c", "protocol.file.allow=always", "submodule", "update", "--init", "--recursive"}) + + shell.Chdir("innerSubmodule") + shell.UpdateFile("a/b/c/blah", "updated content\n") + + shell.Chdir("../../..") + shell.RunCommand([]string{"ln", "-s", "repo/outerSubmodule/innerSubmodule/a/b/c", "link"}) + + shell.Chdir("link") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Lines( + Contains("HEAD detached"), + Contains("master"), + ) + + t.Views().Commits(). + Lines( + Contains("initial inner commit"), + ) + + t.Views().Files(). + IsFocused(). + Lines( + Contains(" M a/b/c/blah"), // shows as modified + ). + PressPrimaryAction(). + Press(keys.Files.CommitChanges) + + t.ExpectPopup().CommitMessagePanel(). + Title(Equals("Commit summary")). + Type("Update blah"). + Confirm() + + t.Views().Files(). + IsEmpty() + + t.Views().Commits(). + Lines( + Contains("Update blah"), + Contains("initial inner commit"), + ) + }, +}) diff --git a/pkg/integration/tests/worktree/symlink_into_repo_subdir.go b/pkg/integration/tests/worktree/symlink_into_repo_subdir.go new file mode 100644 index 00000000000..a77a95d72b5 --- /dev/null +++ b/pkg/integration/tests/worktree/symlink_into_repo_subdir.go @@ -0,0 +1,63 @@ +package worktree + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SymlinkIntoRepoSubdir = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Open lazygit in a symlink into a repo's subdirectory", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.UserConfig.Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + // we're going to have a directory structure like this: + // project + // - repo/a/b/c (main worktree with subdirs) + // - link (symlink to repo/a/b/c) + // + shell.CreateFileAndAdd("a/b/c/blah", "blah\n") + shell.Commit("initial commit") + shell.UpdateFile("a/b/c/blah", "updated content\n") + + shell.Chdir("..") + shell.RunCommand([]string{"ln", "-s", "repo/a/b/c", "link"}) + + shell.Chdir("link") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Lines( + Contains("master"), + ) + + t.Views().Commits(). + Lines( + Contains("initial commit"), + ) + + t.Views().Files(). + IsFocused(). + Lines( + Contains(" M a/b/c/blah"), // shows as modified + ). + PressPrimaryAction(). + Press(keys.Files.CommitChanges) + + t.ExpectPopup().CommitMessagePanel(). + Title(Equals("Commit summary")). + Type("Add blah"). + Confirm() + + t.Views().Files(). + IsEmpty() + + t.Views().Commits(). + Lines( + Contains("Add blah"), + Contains("initial commit"), + ) + }, +}) diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 88ce3cfcec5..202c3f23a68 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -271,7 +271,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p if err := cmd.Wait(); err != nil { // it's fine if we've killed this program ourselves if !strings.Contains(err.Error(), "signal: killed") { - self.Log.Errorf("Unexpected error when running cmd task: %v", err) + self.Log.Errorf("Unexpected error when running cmd task: %v; Failed command: %v %v", err, cmd.Path, cmd.Args) } } diff --git a/test/.gitconfig b/test/.gitconfig new file mode 120000 index 00000000000..b2405f3cd79 --- /dev/null +++ b/test/.gitconfig @@ -0,0 +1 @@ +global_git_config \ No newline at end of file