Skip to content

Fix startup error when .git file is a symlink (e.g. when using repo tool) #5478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
howettl wants to merge 1 commit into
base: master
Choose a base branch
from
+104 −23
Open

Fix startup error when .git file is a symlink (e.g. when using repo tool) #5478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions pkg/commands/git_commands/repo_paths.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,33 @@ func GetRepoPathsForDir(
dir string,
cmd oscommands.ICmdObjBuilder,
) (*RepoPaths, error) {
gitDirOutput, err := callGitRevParseWithDir(cmd, dir, "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree")
// Do not pass --show-superproject-working-tree in the same rev-parse as the path flags:
// some Git versions abort the whole command (BUG: submodule.c) when they are combined,
// so we would get no worktree or git-dir output at all (e.g. repo tool, symlinked .git).
//
// We run a second rev-parse for the superproject path only. That call can still fail or
// abort on the same bug when run alone; we treat failure or empty output like "not a
// submodule" for repoPath (see below).
gitDirOutput, err := callGitRevParseWithDir(cmd, dir, "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository")
if err != nil {
return nil, err
}

gitDirResults := strings.Split(utils.NormalizeLinefeeds(gitDirOutput), "\n")
if len(gitDirResults) < 4 {
return nil, errors.Errorf("unexpected rev-parse output (expected 4 lines): %q", gitDirOutput)
}
worktreePath := gitDirResults[0]
worktreeGitDirPath := gitDirResults[1]
repoGitDirPath := gitDirResults[2]
isBareRepo := gitDirResults[3] == "true"

// If we're in a submodule, --show-superproject-working-tree will return
// a value, meaning gitDirResults will be length 5. 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) == 5
superprojectOut, superErr := callGitRevParseWithDir(cmd, dir, "--show-superproject-working-tree")
isSubmodule := superErr == nil && strings.TrimSpace(superprojectOut) != ""

// If we're in a submodule, --show-superproject-working-tree returns a non-empty path; use
// the worktree path as repoPath. Otherwise we're in a normal repo or a worktree, so use
// the parent of the git common dir (repoGitDirPath).
var repoPath string
if isSubmodule {
repoPath = worktreePath
Expand Down
104 changes: 88 additions & 16 deletions pkg/commands/git_commands/repo_paths_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ type Scenario struct {
Err errFn
}

// primaryRevParseArgs matches the first GetRepoPathsForDir rev-parse (path flags only).
func primaryRevParseArgs(getRevParseArgs argFn) []string {
return append(getRevParseArgs(),
"--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository")
}

// superprojectRevParseArgs matches the second rev-parse (--show-superproject-working-tree alone).
func superprojectRevParseArgs(getRevParseArgs argFn) []string {
return append(getRevParseArgs(), "--show-superproject-working-tree")
}

func TestGetRepoPaths(t *testing.T) {
scenarios := []Scenario{
{
Expand All @@ -40,7 +51,6 @@ func TestGetRepoPaths(t *testing.T) {
`C:\path\to\repo\.git`,
// --is-bare-repository
"false",
// --show-superproject-working-tree
}, []string{
// --show-toplevel
"/path/to/repo",
Expand All @@ -50,12 +60,16 @@ func TestGetRepoPaths(t *testing.T) {
"/path/to/repo/.git",
// --is-bare-repository
"false",
// --show-superproject-working-tree
})
runner.ExpectGitArgs(
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
primaryRevParseArgs(getRevParseArgs),
strings.Join(mockOutput, "\n"),
nil)
// --show-superproject-working-tree (empty: not inside a submodule checkout)
runner.ExpectGitArgs(
superprojectRevParseArgs(getRevParseArgs),
"",
nil)
},
Path: "/path/to/repo",
Expected: lo.Ternary(runtime.GOOS == "windows", &RepoPaths{
Expand All @@ -78,7 +92,7 @@ func TestGetRepoPaths(t *testing.T) {
{
Name: "bare repo",
BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) {
// setup for main worktree
// setup for main worktree with a separate bare git dir
mockOutput := lo.Ternary(runtime.GOOS == "windows", []string{
// --show-toplevel
`C:\path\to\repo`,
Expand All @@ -88,7 +102,6 @@ func TestGetRepoPaths(t *testing.T) {
`C:\path\to\bare_repo\bare.git`,
// --is-bare-repository
`true`,
// --show-superproject-working-tree
}, []string{
// --show-toplevel
"/path/to/repo",
Expand All @@ -98,12 +111,16 @@ func TestGetRepoPaths(t *testing.T) {
"/path/to/bare_repo/bare.git",
// --is-bare-repository
"true",
// --show-superproject-working-tree
})
runner.ExpectGitArgs(
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
primaryRevParseArgs(getRevParseArgs),
strings.Join(mockOutput, "\n"),
nil)
// --show-superproject-working-tree (empty)
runner.ExpectGitArgs(
superprojectRevParseArgs(getRevParseArgs),
"",
nil)
},
Path: "/path/to/repo",
Expected: lo.Ternary(runtime.GOOS == "windows", &RepoPaths{
Expand All @@ -126,7 +143,7 @@ func TestGetRepoPaths(t *testing.T) {
{
Name: "submodule",
BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) {
mockOutput := lo.Ternary(runtime.GOOS == "windows", []string{
mockPrimary := lo.Ternary(runtime.GOOS == "windows", []string{
// --show-toplevel
`C:\path\to\repo\submodule1`,
// --git-dir
Expand All @@ -135,8 +152,6 @@ func TestGetRepoPaths(t *testing.T) {
`C:\path\to\repo\.git\modules\submodule1`,
// --is-bare-repository
`false`,
// --show-superproject-working-tree
`C:\path\to\repo`,
}, []string{
// --show-toplevel
"/path/to/repo/submodule1",
Expand All @@ -146,12 +161,16 @@ func TestGetRepoPaths(t *testing.T) {
"/path/to/repo/.git/modules/submodule1",
// --is-bare-repository
"false",
// --show-superproject-working-tree
"/path/to/repo",
})
// --show-superproject-working-tree (superproject worktree path)
superOut := lo.Ternary(runtime.GOOS == "windows", `C:\path\to\repo`, "/path/to/repo")
runner.ExpectGitArgs(
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
strings.Join(mockOutput, "\n"),
primaryRevParseArgs(getRevParseArgs),
strings.Join(mockPrimary, "\n"),
nil)
runner.ExpectGitArgs(
superprojectRevParseArgs(getRevParseArgs),
superOut,
nil)
},
Path: "/path/to/repo/submodule1",
Expand All @@ -172,19 +191,72 @@ func TestGetRepoPaths(t *testing.T) {
}),
Err: nil,
},
{
Name: "superproject rev-parse fails (fallback to non-submodule repoPath)",
BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) {
// Primary rev-parse succeeds (e.g. repo-tool symlinked .git); secondary can still error
// (e.g. BUG: submodule.c) — we ignore superproject failure and use repoPath from common-dir.
mockOutput := lo.Ternary(runtime.GOOS == "windows", []string{
// --show-toplevel
`C:\path\to\repo`,
// --absolute-git-dir
`C:\path\to\repo\.git`,
// --git-common-dir
`C:\path\to\repo\.git`,
// --is-bare-repository
"false",
}, []string{
// --show-toplevel
"/path/to/repo",
// --absolute-git-dir
"/path/to/repo/.git",
// --git-common-dir
"/path/to/repo/.git",
// --is-bare-repository
"false",
})
runner.ExpectGitArgs(
primaryRevParseArgs(getRevParseArgs),
strings.Join(mockOutput, "\n"),
nil)
// --show-superproject-working-tree (Git errors, e.g. submodule.c internal BUG)
runner.ExpectGitArgs(
superprojectRevParseArgs(getRevParseArgs),
"",
errors.New("BUG: submodule.c:2455: returned path string doesn't match cwd?"))
},
Path: "/path/to/repo",
Expected: lo.Ternary(runtime.GOOS == "windows", &RepoPaths{
worktreePath: `C:\path\to\repo`,
worktreeGitDirPath: `C:\path\to\repo\.git`,
repoPath: `C:\path\to\repo`,
repoGitDirPath: `C:\path\to\repo\.git`,
repoName: `repo`,
isBareRepo: false,
}, &RepoPaths{
worktreePath: "/path/to/repo",
worktreeGitDirPath: "/path/to/repo/.git",
repoPath: "/path/to/repo",
repoGitDirPath: "/path/to/repo/.git",
repoName: "repo",
isBareRepo: false,
}),
Err: nil,
},
{
Name: "git rev-parse returns an error",
BeforeFunc: func(runner *oscommands.FakeCmdObjRunner, getRevParseArgs argFn) {
// Primary rev-parse fails; superproject call is never run
runner.ExpectGitArgs(
append(getRevParseArgs(), "--show-toplevel", "--absolute-git-dir", "--git-common-dir", "--is-bare-repository", "--show-superproject-working-tree"),
primaryRevParseArgs(getRevParseArgs),
"",
errors.New("fatal: invalid gitfile format: /path/to/repo/worktree2/.git"))
},
Path: "/path/to/repo/worktree2",
Expected: nil,
Err: func(getRevParseArgs argFn) error {
args := strings.Join(getRevParseArgs(), " ")
return fmt.Errorf("'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)
return fmt.Errorf("'git %v --show-toplevel --absolute-git-dir --git-common-dir --is-bare-repository' failed: fatal: invalid gitfile format: /path/to/repo/worktree2/.git", args)
},
},
}
Expand Down