Skip to content

git: worktree, canonicalize pathnames when calling Worktree.Add(), Fixes #1460 #1491

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.

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

0xDEC0DE
Copy link
Contributor

@0xDEC0DE 0xDEC0DE commented Apr 7, 2025

Ensure that adding files off the filesystem root gets the filenames right.

Fixes: Issue #1460

@0xDEC0DE 0xDEC0DE changed the title fix: canonicalize pathnames when calling Worktree.Add() worktree: canonicalize pathnames when calling Worktree.Add(), Fixes #1460 Apr 7, 2025
@0xDEC0DE 0xDEC0DE force-pushed the issue/1460 branch 2 times, most recently from 8361313 to 4727c64 Compare April 13, 2025 23:43
@0xDEC0DE 0xDEC0DE changed the title worktree: canonicalize pathnames when calling Worktree.Add(), Fixes #1460 git: worktree, canonicalize pathnames when calling Worktree.Add(), Fixes #1460 Apr 13, 2025
 go-git#1460

Ensure that adding files off the filesystem root gets the filenames
right.

Fixes: Issue go-git#1460
Comment on lines +376 to +379
if string(path[0]) == string(filepath.Separator) {
root := w.Filesystem.Root()
path, _ = filepath.Rel(root, filepath.Join(root, ".", path))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xDEC0DE thanks for looking into this. This logic seems better suited in go-billy, potentially as an util that receives a Filesystem and a path, returning a canonical path if the file exists.

We'd need to test this (in go-billy) for all built-in FS types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure I understand, as the use case here is creating a new file; but there's already util.getUnderlyingAndPath, which I suppose could use an exported version, would that be sufficient for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants