Skip to content

Commit f5f8090

Browse files
committed
Fix seg-fault when opening submodule in nested folder
1 parent 579791e commit f5f8090

File tree

2 files changed

+48
-8
lines changed

2 files changed

+48
-8
lines changed

pkg/commands/git_commands/repo_paths.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ func linkedWorktreeGitDirPath(fs afero.Fs, worktreePath string) (string, error)
157157
}
158158

159159
gitDir := strings.TrimPrefix(gitDirLine[0], "gitdir: ")
160+
161+
// For windows support
162+
gitDir = filepath.ToSlash(gitDir)
163+
160164
return gitDir, nil
161165
}
162166

@@ -194,19 +198,28 @@ func getCurrentRepoGitDirPath(
194198
return "", "", errors.Errorf("could not find git dir for %s: %v", currentPath, err)
195199
}
196200

197-
// confirm whether the next directory up is the worktrees/submodules directory
201+
_, err = fs.Stat(worktreeGitPath)
202+
if err != nil {
203+
return "", "", errors.Errorf("could not find git dir for %s: %v", currentPath, err)
204+
}
205+
206+
// confirm whether the next directory up is the worktrees directory
198207
parent := path.Dir(worktreeGitPath)
199-
if path.Base(parent) != "worktrees" && path.Base(parent) != "modules" {
200-
return "", "", errors.Errorf("could not find git dir for %s", currentPath)
208+
if path.Base(parent) == "worktrees" {
209+
gitDirPath := path.Dir(parent)
210+
return gitDirPath, path.Dir(gitDirPath), nil
201211
}
202212

203-
// if it's a submodule, we treat it as its own repo
204-
if path.Base(parent) == "modules" {
213+
// Unlike worktrees, submodules can be nested arbitrarily deep, so we check
214+
// if the `modules` directory is anywhere up the chain.
215+
if strings.Contains(worktreeGitPath, "/modules/") {
216+
// For submodules, we just return the path directly
205217
return worktreeGitPath, currentPath, nil
206218
}
207219

208-
gitDirPath := path.Dir(parent)
209-
return gitDirPath, path.Dir(gitDirPath), nil
220+
// If this error causes issues, we could relax the constraint and just always
221+
// return the path
222+
return "", "", errors.Errorf("could not find git dir for %s: path is not under `worktrees` or `modules` directories", currentPath)
210223
}
211224

212225
// takes a path containing a symlink and returns the true path

pkg/commands/git_commands/repo_paths_test.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func TestGetRepoPathsAux(t *testing.T) {
7373
},
7474
Path: "/path/to/repo/worktree2",
7575
Expected: nil,
76-
Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/worktree2"),
76+
Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/worktree2: open /nonexistant: file does not exist"),
7777
},
7878
{
7979
Name: "submodule",
@@ -92,6 +92,33 @@ func TestGetRepoPathsAux(t *testing.T) {
9292
},
9393
Err: nil,
9494
},
95+
{
96+
Name: "submodule in nested directory",
97+
BeforeFunc: func(fs afero.Fs) {
98+
_ = fs.MkdirAll("/path/to/repo/.git/modules/my/submodule1", 0o755)
99+
_ = afero.WriteFile(fs, "/path/to/repo/my/submodule1/.git", []byte("gitdir: /path/to/repo/.git/modules/my/submodule1"), 0o644)
100+
},
101+
Path: "/path/to/repo/my/submodule1",
102+
Expected: &RepoPaths{
103+
currentPath: "/path/to/repo/my/submodule1",
104+
worktreePath: "/path/to/repo/my/submodule1",
105+
worktreeGitDirPath: "/path/to/repo/.git/modules/my/submodule1",
106+
repoPath: "/path/to/repo/my/submodule1",
107+
repoGitDirPath: "/path/to/repo/.git/modules/my/submodule1",
108+
repoName: "submodule1",
109+
},
110+
Err: nil,
111+
},
112+
{
113+
Name: "submodule git dir not under .git/modules",
114+
BeforeFunc: func(fs afero.Fs) {
115+
_ = fs.MkdirAll("/random/submodule1", 0o755)
116+
_ = afero.WriteFile(fs, "/path/to/repo/my/submodule1/.git", []byte("gitdir: /random/submodule1"), 0o644)
117+
},
118+
Path: "/path/to/repo/my/submodule1",
119+
Expected: nil,
120+
Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/my/submodule1: path is not under `worktrees` or `modules` directories"),
121+
},
95122
}
96123

97124
for _, s := range scenarios {

0 commit comments

Comments
 (0)