Skip to content

Commit

Permalink
Use git rev-parse to obtain repository and worktree paths (#3183)
Browse files Browse the repository at this point in the history
- **PR Description**
This changes GetRepoPaths() to pull information from `git rev-parse`
instead of partially reimplementing git's logic for pathfinding. This
change fixes issues with bare repos, esp. versioned homedir use cases,
by aligning lazygit's path handling to what git itself does. I believe
it also paves the way for lazygit to run from any subdirectory of a
working tree with relatively minor changes.

Addresses #1294 and #3175.

- **Please check if the PR fulfills these requirements**

* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [x] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [x] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [x] Docs (specifically `docs/Config.md`) have been updated if
necessary
* [x] You've read through your own file changes for silly mistakes etc

<!--
Be sure to name your PR with an imperative e.g. 'Add worktrees view'
see https://github.com/jesseduffield/lazygit/releases/tag/v0.40.0 for
examples
-->
  • Loading branch information
stefanhaller authored Jan 24, 2024
2 parents 74d9378 + 3d9f1e0 commit b7d4db2
Show file tree
Hide file tree
Showing 25 changed files with 586 additions and 438 deletions.
4 changes: 4 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
root = true

[*.go]
indent_style = tab
63 changes: 4 additions & 59 deletions pkg/commands/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@ 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"
"github.com/jesseduffield/lazygit/pkg/commands/git_config"
"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"
)

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
}
1 change: 1 addition & 0 deletions pkg/commands/git_commands/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
15 changes: 9 additions & 6 deletions pkg/commands/git_commands/commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,39 +197,39 @@ 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",
filterPath: "file.txt",
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",
filterPath: "",
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",
filterPath: "",
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",
filterPath: "",
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"},
},
}

Expand All @@ -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()
Expand Down
9 changes: 7 additions & 2 deletions pkg/commands/git_commands/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit b7d4db2

Please sign in to comment.