path.c: translate WSL and Cygwin paths when resolving worktrees#6234
path.c: translate WSL and Cygwin paths when resolving worktrees#6234johnnyshields wants to merge 1 commit intogit-for-windows:mainfrom
Conversation
60f3381 to
555e8fa
Compare
|
This strongly reminds me of #6187, and my serious concerns about supporting/advertising a scenario that cannot ever be performant are unchanged. I understand what you're trying to do, I just think it is a bad idea and will only lead to complaints "why is this so slow, can't you make this faster? LOL". |
| size_t prefix_len; | ||
| } posix_prefixes[] = { | ||
| { "/mnt/", 5 }, | ||
| { "/cygdrive/", 10 }, |
There was a problem hiding this comment.
That works for Cygwin. But Git for Windows is based on MSYS2, where the drives are mapped as /c/, /d/, etc. And by translating those always, you run now create ambiguities that cannot be overcome: In WSL, /c/ is a valid path that you could now no longer access. Sure, you can argue that "nobody maps those", but here is a person speaking to you who did that in the past, and in general it is not a good sign when your code introduces ambiguities where there were none before the patch.
There was a problem hiding this comment.
Firstly, WSL /mnt/c/ and Cygwin /cygwin/c/ paths are unambiguous and are major wins--especially WSL--even if we can't agree what to do with MSYS2.
Re: MSYS2, I can think of 4 possibilities:
- Alias
/c/,/d/toC:\,D:\etc--I think this is arguably safe, see below. - Do nothing (or defer implementation pending further discussion)
- Opt-in via config--something like
git config --global core.translateMsysPaths true - Solve it write-side--have MSYS2 write
gitdir = /c/...asC:\...and then read it back as/c/. This is defensible IMHO because MSYS2 is highly specialized to Windows, and storing paths in Windows format is not wrong, per se.
Why I think MSYS2's /c/, /d/, paths etc. are safe to alias too
Recall that two gates limit where the new logic runs:
- Compile-time: the helper is
#ifdef GIT_WINDOWS_NATIVE, so it's compiled in only on the mingw64 native build of Git for Windows. WSL git, Cygwin git, and the MSYS2-runtime git from pacman are separate builds and unaffected. Your "person who mkdir /c in WSL" continues to use a WSL git binary that has none of this code. - Call site: the helper fires only on three specific worktree-related files — /.git, /worktrees//gitdir, and /commondir. It is not part of general path resolution.
For an ambiguity to actually bite a user, ALL of the following must coincide:
- The user runs Git-for-Windows's mingw64 git from within Windows OS
- They are operating on a worktree (not a normal repo).
- The user will have had to created a specific single-letter path
/c/within WSL Linux's root dir. - The main repo must lives within this special WSL single-letter path (e.g.
/c/foo, not visible to native Windows). - The worktree lives at a separate Windows-visible path (e.g.
/mnt/c/elsewhere/wt), i.e. not co-located with its parent repo, which would be exceedingly uncommon. - The user also must have a real Windows repo at
C:/foowhose exact path mirrors/clashes with the WSL one/c/foo(otherwise its a no-op)
Even with all six aligned, the failure mode is bounded: Git-for-Windows follows the translated c:/foo, finds either nothing (and does a no-op, as it does today) or a real repo at that path that the user explicitly created. AFAIK Git has protection mechanisms that prevent worktrees from pointing to the wrong copy of their repo (based on bi-directional registration.)
TLDR; I just don't see this as something that's seriously going to affect anyone, meanwhile the convenience of Windows-and-MSYS2 interop is a tangible quality-of-life win.
There was a problem hiding this comment.
The path /mnt/ on Linux can be anything, from a mount to a regular directory. Nothing, nothing at all guarantees that /mnt/c refers to the mounted C: drive iff this is a WSL instance. But as I had pointed out already (which you ignored), the same code runs also on regular Linux, where it definitely does not refer to a host's C: drive because there is none.
I understand that it is in fashion nowadays to double-down instead of fixing designs, but this is not going to work here. There are fundamental problems with the proposed design that won't stand a chance of being addressed merely by doubling down on a flawed idea.
There was a problem hiding this comment.
Hold on.
Yes, /mnt/ can technically be anything. However, 99% of Linux distros use /mnt/ as part of the Linux FHS--even FHS-deviants like NixOS use /mnt/--and doubly-so for the subset of distros that are used on WSL.
Again, this PR is for Git for Windows exclusively. It is simply saying, "If Git runs on Windows, and it sees a worktree with gitdir = /mnt/c/foo/bar, it is reasonable to try to map it to C:\foo\bar", because in 99.99% of cases this arises from creating the worktree inside WSL, exactly as was reported in #6187.
So we're talking cross-purposes here. Of course we can sit around all day and say "what ifs" i.e. "what if someone renames /mnt/ in Linux?" And in fact, those cases are fine, the behavior simply degrades to the no-op that is is today. So users who want Windows-WSL git interop need to live with not renaming /mnt/ to something screwy (which by the way is going to break a whole lot more than git :)
Perhaps a compromise here is to have feature flags:
git config --global core.enableWindowsWSLCompatibility truegit config --global core.enableWindowsCygwinCompatibility truegit config --global core.enableWindowsMSYSCompatibility true
These would enable compatibility shims (gitdir for now, maybe other things in the future) that come with certain assumptions, i.e. that you haven't renamed /mnt/. For WSL specifically we can add the caveat about performance to the documentation.
Would you be willing to accept a PR with those feature flags added?
There was a problem hiding this comment.
You are elevating a fragile heuristic to a fundamental design decision. I find that highly unconvincing, and am unwilling to accept it into Git for Windows. Maybe you will have more luck getting it into Git for Windows via the Git project.
There was a problem hiding this comment.
@dscho that /mnt/ is used by 100% of WSL-available Git distros as the mountpoint dir is not a random "fragile heuristic", its a direct consequence of the FHS standard. Anyone who renames it must reasonably expect that various things will break.
We are getting lost in the weeds here.
This PR is simply proposing to have Git on Windows interpret gitdir = /mnt/c/<path> as gitdir = C:\
- This will benefit the large number of Windows devs using Windows and WSL with a shared filesystem.
- If the user is not using WSL, there is no impact, because their
gitdirpaths will not include/mnt/ - If the user insists to rename
/mnt/then this feature will gracefully degrade to a no-op--the exact same as it is today. - MSYS2 and Cygwin can be omitted for now; WSL is the priority as per Worktrees don't work when the worktree is created on Windows but used from WSL #6187.
- I'd be happy to feature flag this in some form.
I think we need to honestly ask who is Git for Windows intended for:
- Is it the hypothetical Windows Mega-Wizard who never uses WSL, and when he does use WSL he insists on renaming
/mnt/? - Or the average Windows dev who installs generic WSL Ubuntu, TortoiseGit, and is just happy if his worktrees are accessible from both under the "out-of-the-box" config--i.e. "it just works" under the standard 99% happy path.
|
It would probably be a much better idea to design something a lot more robust, a feature where users can say specifically "hey Git, if you encounter a specific absolute path in |
When `git worktree add` is run from inside WSL2 (or Cygwin/MSYS), git
records each worktree's path using the POSIX-mounted view of the
Windows filesystem - e.g. `/mnt/c/repo/.git/worktrees/wt` or
`/cygdrive/c/repo/.git/worktrees/wt`. Reading those files back from
native-Windows git fails because the paths are not meaningful outside
of the WSL/Cygwin namespace, so the worktree appears broken even
though every byte of it is reachable from Windows.
Add a small helper `translate_wsl_path()` that rewrites the recognised
POSIX-drive prefixes into Windows drive-letter form in place, and call
it at the three places where Windows-native git reads a recorded
worktree-related path back from disk:
* `read_gitfile_gently()` - the `gitdir:` line in a worktree's
`.git` file.
* `get_common_dir_noenv()` - the `commondir` file inside a
worktree's git directory, which points at the main repo.
* `get_linked_worktree()` - the `gitdir` file inside
`<commondir>/worktrees/<id>/`, which points at the worktree's
`.git` link.
Translation only happens for `/mnt/<x>/` and `/cygdrive/<x>/` where
`<x>` is a single ASCII letter and the next character is either a
separator or end-of-string. Anything else (e.g. `/mnt/storage`) is
left alone. The helper compiles to a no-op outside `GIT_WINDOWS_NATIVE`,
so a `/mnt/c/...` path on a Linux host - where it might really exist -
is still treated literally.
Add t/t0042-wsl-mnt-path.sh covering all three call sites plus the
"do not translate" cases. Tests are gated on the MINGW prereq.
Signed-off-by: johnnyshields <27655+johnnyshields@users.noreply.github.com>
555e8fa to
af0a7b2
Compare
|
First thing to mention is that as per #6187 -- this is a real issue which would be a significant quality-of-life win for users, including myself. Let's set aside biases such as "I don't use WSL / Cygwin / MSYS2, therefore
The goal of this PR--and WSL in general--is not performance, it is convenience. We shouldn't confuse the two goals. WSL users know that WSL comes with a baked-in performance penalty--especially when accessing the mounted Windows filesystems. If we want performance on Linux, we know to install Linux directly. No WSL user in their right mind is going to hold git responsible for WSL's own architectural tradeoffs. (IIRC git already displays some warning when installing or running on WSL.) Also, the perf issues are not on the Git-for-Windows side, since Windows accessing Windows-native filesystems is fine. The perf issues are entirely on the WSL side (again WSL's architecture itself, not "git" inside WSL.) That being said--I use WSL + Windows with an SSD, and the perf is actually totally tolerable for me, even working on 1 million+ LOC repos. |
This would technically work, but it's an unopinionated approach to a problem whose specificity lends itself to an opinionated solution. The problem is specifically that of worktree Suppose we added a config
TLDR; the opinionated solution is a sure-shot to do what users expect (and nothing more), the unopinionated solution is a Pandora's box. (If the proposal is to do something like a Boolean |
Fixes #6187
The Problem
If I create a git worktree from within WSL, then Windows Git cannot resolve it, because the
gitdirspecifier will be a POSIX-style path/mnt/c/repo...rather than Windows-styleC:\repo\...(I use a hybrid Windows and WSL Ubuntu setup on the same machine; it works nearly perfectly except for the above issue, specifically with worktrees.)
Related PRs
The Root Cause
Worktrees use a bi-directional link that include absolute paths in the
gitdirvalue to point from the main repo to the worktree repo, and vice versa. It is specifically thisgitdirvalue that needs Windows-to-POSIX path translation handling. (Nothing else besides this path requires additional logic for worktrees to work across WSL and Windows native.)When
git worktree addis run from inside WSL2 (or Cygwin/MSYS), git records each worktree's path in agitdirfile using the POSIX-mounted path of the Windows filesystem - e.g./mnt/c/repo/.git/worktrees/wtor/cygdrive/c/repo/.git/worktrees/wt. Reading those files back from native-Windows git fails because the paths are not the expected Windows path formatC:\repo\..., so the worktree appears broken even though it is actually reachable from Windows.The Solution
This PR adds a small helper
translate_wsl_path()that rewrites the recognised POSIX-drive prefixes into Windows drive-letter form in place, and call it at the three places where Windows-native git reads a recorded worktree-related path back from disk:read_gitfile_gently()- thegitdir:line in a worktree's.gitfile.get_common_dir_noenv()- thecommondirfile inside a worktree's git directory, which points at the main repo.get_linked_worktree()- thegitdirfile inside<commondir>/worktrees/<id>/, which points at the worktree's.gitlink.Why this is Safe
This PR is safe because:
GIT_WINDOWS_NATIVE, so a/mnt/c/...path on a Linux host - where it might really exist - is still treated literally./mnt/<x>/and/cygdrive/<x>/where<x>is a single ASCII letter and the next character is either a separator or end-of-string. Anything else (e.g./mnt/storage) is left alone. The helper also doesn't evaluate/collapse relative paths, so it doesn't introduce any path-traversal vectors that aren't existing today.