Skip to content

Commit

Permalink
Fix seg-fault when opening submodule in nested folder (#2903)
Browse files Browse the repository at this point in the history
  • Loading branch information
jesseduffield committed Aug 7, 2023
2 parents 579791e + 8e7958f commit 4c89f9f
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 8 deletions.
31 changes: 24 additions & 7 deletions pkg/commands/git_commands/repo_paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ func linkedWorktreeGitDirPath(fs afero.Fs, worktreePath string) (string, error)
}

gitDir := strings.TrimPrefix(gitDirLine[0], "gitdir: ")

// For windows support
gitDir = filepath.ToSlash(gitDir)

return gitDir, nil
}

Expand Down Expand Up @@ -194,19 +198,32 @@ func getCurrentRepoGitDirPath(
return "", "", errors.Errorf("could not find git dir for %s: %v", currentPath, err)
}

// confirm whether the next directory up is the worktrees/submodules directory
_, 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" && path.Base(parent) != "modules" {
return "", "", errors.Errorf("could not find git dir for %s", currentPath)
if path.Base(parent) == "worktrees" {
gitDirPath := path.Dir(parent)
return gitDirPath, path.Dir(gitDirPath), nil
}

// if it's a submodule, we treat it as its own repo
if path.Base(parent) == "modules" {
// 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
}

gitDirPath := path.Dir(parent)
return gitDirPath, path.Dir(gitDirPath), 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: path is not under `worktrees` or `modules` directories", currentPath)
}

// takes a path containing a symlink and returns the true path
Expand Down
29 changes: 28 additions & 1 deletion pkg/commands/git_commands/repo_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestGetRepoPathsAux(t *testing.T) {
},
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"),
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",
Expand All @@ -92,6 +92,33 @@ 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)
},
Path: "/path/to/repo/my/submodule1",
Expected: nil,
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"),
},
}

for _, s := range scenarios {
Expand Down

0 comments on commit 4c89f9f

Please sign in to comment.