Skip to content

Commit 7a67096

Browse files
jwhitleyjwhitley
committed
Optimize number of early calls to GetRepoPaths
This change reduces the number of calls during application startup to one, calling GetRepoPaths() earlier than previously and plumbing the repoPaths struct around to achieve this end.
1 parent a138a31 commit 7a67096

File tree

6 files changed

+85
-48
lines changed

6 files changed

+85
-48
lines changed

‎pkg/app/app.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/spf13/afero"
1515

1616
appTypes "github.com/jesseduffield/lazygit/pkg/app/types"
17-
"github.com/jesseduffield/lazygit/pkg/commands"
1817
"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
1918
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
2019
"github.com/jesseduffield/lazygit/pkg/common"
@@ -119,7 +118,14 @@ func NewApp(config config.AppConfigurer, test integrationTypes.IntegrationTest,
119118
return app, err
120119
}
121120

122-
showRecentRepos, err := app.setupRepo()
121+
// If we're not in a repo, repoPaths will be nil. The error is moot for us
122+
// at this stage, since we'll try to init a new repo in setupRepo(), below
123+
repoPaths, err := git_commands.GetRepoPaths(app.OSCommand.Cmd, gitVersion)
124+
if err != nil {
125+
return app, err
126+
}
127+
128+
showRecentRepos, err := app.setupRepo(repoPaths)
123129
if err != nil {
124130
return app, err
125131
}
@@ -168,14 +174,16 @@ func openRecentRepo(app *App) bool {
168174
return false
169175
}
170176

171-
func (app *App) setupRepo() (bool, error) {
177+
func (app *App) setupRepo(
178+
repoPaths *git_commands.RepoPaths,
179+
) (bool, error) {
172180
if env.GetGitDirEnv() != "" {
173-
// we've been given the git dir directly. We'll verify this dir when initializing our Git object
181+
// we've been given the git dir directly. Skip setup
174182
return false, nil
175183
}
176184

177185
// if we are not in a git repo, we ask if we want to `git init`
178-
if err := commands.VerifyInGitRepo(app.OSCommand); err != nil {
186+
if repoPaths == nil {
179187
cwd, err := os.Getwd()
180188
if err != nil {
181189
return false, err
@@ -221,6 +229,7 @@ func (app *App) setupRepo() (bool, error) {
221229
if err := app.OSCommand.Cmd.New(args).Run(); err != nil {
222230
return false, err
223231
}
232+
224233
return false, nil
225234
}
226235

@@ -238,10 +247,7 @@ func (app *App) setupRepo() (bool, error) {
238247
}
239248

240249
// Run this afterward so that the previous repo creation steps can run without this interfering
241-
if isBare, err := git_commands.IsBareRepo(app.OSCommand); isBare {
242-
if err != nil {
243-
return false, err
244-
}
250+
if repoPaths.IsBareRepo() {
245251

246252
fmt.Print(app.Tr.BareRepo)
247253

‎pkg/commands/git_commands/repo_paths.go‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package git_commands
22

33
import (
44
ioFs "io/fs"
5+
"os"
56
"path"
67
"path/filepath"
78
"strings"
@@ -18,6 +19,7 @@ type RepoPaths struct {
1819
repoPath string
1920
repoGitDirPath string
2021
repoName string
22+
isBareRepo bool
2123
}
2224

2325
var gitPathFormatVersion GitVersion = GitVersion{2, 31, 0, ""}
@@ -54,6 +56,10 @@ func (self *RepoPaths) RepoName() string {
5456
return self.repoName
5557
}
5658

59+
func (self *RepoPaths) IsBareRepo() bool {
60+
return self.isBareRepo
61+
}
62+
5763
// Returns the repo paths for a typical repo
5864
func MockRepoPaths(currentPath string) *RepoPaths {
5965
return &RepoPaths{
@@ -62,14 +68,27 @@ func MockRepoPaths(currentPath string) *RepoPaths {
6268
repoPath: currentPath,
6369
repoGitDirPath: path.Join(currentPath, ".git"),
6470
repoName: "lazygit",
71+
isBareRepo: false,
6572
}
6673
}
6774

6875
func GetRepoPaths(
6976
cmd oscommands.ICmdObjBuilder,
7077
version *GitVersion,
7178
) (*RepoPaths, error) {
72-
gitDirOutput, err := callGitRevParse(cmd, version, "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree")
79+
cwd, err := os.Getwd()
80+
if err != nil {
81+
return nil, err
82+
}
83+
return GetRepoPathsForDir(cwd, cmd, version)
84+
}
85+
86+
func GetRepoPathsForDir(
87+
dir string,
88+
cmd oscommands.ICmdObjBuilder,
89+
version *GitVersion,
90+
) (*RepoPaths, error) {
91+
gitDirOutput, err := callGitRevParseWithDir(cmd, version, dir, "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree")
7392
if err != nil {
7493
return nil, err
7594
}
@@ -84,13 +103,14 @@ func GetRepoPaths(
84103
return nil, err
85104
}
86105
}
106+
isBareRepo := gitDirResults[3] == "true"
87107

88108
// If we're in a submodule, --show-superproject-working-tree will return
89-
// a value, meaning gitDirResults will be length 4. In that case
109+
// a value, meaning gitDirResults will be length 5. In that case
90110
// return the worktree path as the repoPath. Otherwise we're in a
91111
// normal repo or a worktree so return the parent of the git common
92112
// dir (repoGitDirPath)
93-
isSubmodule := len(gitDirResults) == 4
113+
isSubmodule := len(gitDirResults) == 5
94114

95115
var repoPath string
96116
if isSubmodule {
@@ -106,17 +126,10 @@ func GetRepoPaths(
106126
repoPath: repoPath,
107127
repoGitDirPath: repoGitDirPath,
108128
repoName: repoName,
129+
isBareRepo: isBareRepo,
109130
}, nil
110131
}
111132

112-
func callGitRevParse(
113-
cmd oscommands.ICmdObjBuilder,
114-
version *GitVersion,
115-
gitRevArgs ...string,
116-
) (string, error) {
117-
return callGitRevParseWithDir(cmd, version, "", gitRevArgs...)
118-
}
119-
120133
func callGitRevParseWithDir(
121134
cmd oscommands.ICmdObjBuilder,
122135
version *GitVersion,

‎pkg/commands/git_commands/repo_paths_test.go‎

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ func TestGetRepoPaths(t *testing.T) {
3636
"/path/to/repo/.git",
3737
// --git-common-dir
3838
"/path/to/repo/.git",
39+
// --is-bare-repository
40+
"false",
3941
// --show-superproject-working-tree
4042
}
4143
runner.ExpectGitArgs(
42-
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree"),
44+
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
4345
strings.Join(expectedOutput, "\n"),
4446
nil)
4547
},
@@ -50,6 +52,38 @@ func TestGetRepoPaths(t *testing.T) {
5052
repoPath: "/path/to/repo",
5153
repoGitDirPath: "/path/to/repo/.git",
5254
repoName: "repo",
55+
isBareRepo: false,
56+
},
57+
Err: nil,
58+
},
59+
{
60+
Name: "bare repo",
61+
BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) {
62+
// setup for main worktree
63+
expectedOutput := []string{
64+
// --show-toplevel
65+
"/path/to/repo",
66+
// --git-dir
67+
"/path/to/bare_repo/bare.git",
68+
// --git-common-dir
69+
"/path/to/bare_repo/bare.git",
70+
// --is-bare-repository
71+
"true",
72+
// --show-superproject-working-tree
73+
}
74+
runner.ExpectGitArgs(
75+
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
76+
strings.Join(expectedOutput, "\n"),
77+
nil)
78+
},
79+
Path: "/path/to/repo",
80+
Expected: &RepoPaths{
81+
worktreePath: "/path/to/repo",
82+
worktreeGitDirPath: "/path/to/bare_repo/bare.git",
83+
repoPath: "/path/to/bare_repo",
84+
repoGitDirPath: "/path/to/bare_repo/bare.git",
85+
repoName: "bare_repo",
86+
isBareRepo: true,
5387
},
5488
Err: nil,
5589
},
@@ -63,11 +97,13 @@ func TestGetRepoPaths(t *testing.T) {
6397
"/path/to/repo/.git/modules/submodule1",
6498
// --git-common-dir
6599
"/path/to/repo/.git/modules/submodule1",
100+
// --is-bare-repository
101+
"false",
66102
// --show-superproject-working-tree
67103
"/path/to/repo",
68104
}
69105
runner.ExpectGitArgs(
70-
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree"),
106+
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
71107
strings.Join(expectedOutput, "\n"),
72108
nil)
73109
},
@@ -78,14 +114,15 @@ func TestGetRepoPaths(t *testing.T) {
78114
repoPath: "/path/to/repo/submodule1",
79115
repoGitDirPath: "/path/to/repo/.git/modules/submodule1",
80116
repoName: "submodule1",
117+
isBareRepo: false,
81118
},
82119
Err: nil,
83120
},
84121
{
85122
Name: "git rev-parse returns an error",
86123
BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) {
87124
runner.ExpectGitArgs(
88-
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--show-superproject-working-tree"),
125+
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
89126
"",
90127
errors.New("fatal: invalid gitfile format: /path/to/repo/worktree2/.git"))
91128
},
@@ -94,7 +131,7 @@ func TestGetRepoPaths(t *testing.T) {
94131
Err: func(getRevParseArgs argFn) error {
95132
args := strings.Join(getRevParseArgs(), " ")
96133
return errors.New(
97-
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),
134+
fmt.Sprintf("'git %v --show-toplevel --absolute-git-dir --git-common-dir --is-bare-repository --show-superproject-working-tree' failed: fatal: invalid gitfile format: /path/to/repo/worktree2/.git", args),
98135
)
99136
},
100137
},
@@ -120,7 +157,7 @@ func TestGetRepoPaths(t *testing.T) {
120157
// prepare the filesystem for the scenario
121158
s.BeforeFunc(runner, getRevParseArgs)
122159

123-
repoPaths, err := GetRepoPaths(cmd, version)
160+
repoPaths, err := GetRepoPathsForDir("", cmd, version)
124161

125162
// check the error and the paths
126163
if s.Err != nil {

‎pkg/commands/git_commands/status.go‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@ package git_commands
33
import (
44
"os"
55
"path/filepath"
6-
"strconv"
76
"strings"
87

9-
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
108
"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
119
)
1210

@@ -49,20 +47,8 @@ func (self *StatusCommands) WorkingTreeState() enums.RebaseMode {
4947
return enums.REBASE_MODE_NONE
5048
}
5149

52-
func (self *StatusCommands) IsBareRepo() (bool, error) {
53-
return IsBareRepo(self.os)
54-
}
55-
56-
func IsBareRepo(osCommand *oscommands.OSCommand) (bool, error) {
57-
res, err := osCommand.Cmd.New(
58-
NewGitCmd("rev-parse").Arg("--is-bare-repository").ToArgv(),
59-
).DontLog().RunWithOutput()
60-
if err != nil {
61-
return false, err
62-
}
63-
64-
// The command returns output with a newline, so we need to strip
65-
return strconv.ParseBool(strings.TrimSpace(res))
50+
func (self *StatusCommands) IsBareRepo() bool {
51+
return self.repoPaths.isBareRepo
6652
}
6753

6854
func (self *StatusCommands) IsInNormalRebase() (bool, error) {

‎pkg/gui/dummies.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ func NewDummyUpdater() *updates.Updater {
1717
// NewDummyGui creates a new dummy GUI for testing
1818
func NewDummyGui() *Gui {
1919
newAppConfig := config.NewDummyAppConfig()
20-
dummyGui, _ := NewGui(utils.NewDummyCommon(), newAppConfig, &git_commands.GitVersion{}, NewDummyUpdater(), false, "", nil)
20+
dummyGui, _ := NewGui(utils.NewDummyCommon(), newAppConfig, &git_commands.GitVersion{Major: 2, Minor: 0, Patch: 0}, NewDummyUpdater(), false, "", nil)
2121
return dummyGui
2222
}

‎pkg/gui/recent_repos_panel.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,7 @@ import (
88
// updateRecentRepoList registers the fact that we opened lazygit in this repo,
99
// so that we can open the same repo via the 'recent repos' menu
1010
func (gui *Gui) updateRecentRepoList() error {
11-
isBareRepo, err := gui.git.Status.IsBareRepo()
12-
if err != nil {
13-
return err
14-
}
15-
16-
if isBareRepo {
11+
if gui.git.Status.IsBareRepo() {
1712
// we could totally do this but it would require storing both the git-dir and the
1813
// worktree in our recent repos list, which is a change that would need to be
1914
// backwards compatible

0 commit comments

Comments
 (0)