Skip to content
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

git rev-parse --show-toplevel fails to resolve all junctions #2114

Closed
1 task done
bk2204 opened this issue Mar 7, 2019 · 11 comments
Closed
1 task done

git rev-parse --show-toplevel fails to resolve all junctions #2114

bk2204 opened this issue Mar 7, 2019 · 11 comments
Labels

Comments

@bk2204
Copy link

bk2204 commented Mar 7, 2019

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options
git version 2.19.0.gvfs.1.34.gc7fb556
cpu: x86_64
built from commit: c7fb556beb07c47abba7a4c31d03e56f14ced358
sizeof-long: 4
sizeof-size_t: 8
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd /c ver
Microsoft Windows [Version 10.0.17763.316]
(c) 2018 Microsoft Corporation. All rights reserved.

This is a 64-bit VM

  • What options did you set as part of the installation? Or did you choose the
    defaults?
$ cat /etc/install-options.txt
Editor Option: VIM
Custom Editor Path:
Path Option: Cmd
SSH Option: OpenSSH
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Enabled
Enable Builtin Rebase: Enabled
Enable Builtin Stash: Enabled

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

Bash

# In CMD
mklink /J c:\junction "c:\Users\brian m. carlson"
# In Git Bash
cd /c/Junction
git init test
cd test
git rev-parse --show-toplevel
  • What did you expect to occur after running these commands?

I expected Git to provide an absolute, canonical path, resolving all junctions (since they are a kind of symlink).

  • What actually happened instead?

The junction was not resolved:

$ git rev-parse --show-toplevel
C:/junction/test

This happens to cause a problem for Git LFS, because Git LFS does indeed resolve all junctions. However, when relativizing a path based on git rev-parse --show-toplevel, which is documented as being absolute, the path that's created doesn't share the proper components in common, leading to locking being broken because the lock path contains ... This is the cause of git-lfs/git-lfs#3354.

@dscho
Copy link
Member

dscho commented Mar 8, 2019

Hmm. The way I understand junction points is more like a bind mount point. And if you mounted, say, mount --bind /some/where /else/where, then paths inside /some/where would not be "resolved" to /else/where paths.

The reason why I say this is that junction points are resolved in the low-level filesystem layer, unlike symbolic links.

So I would argue that Git LFS should not resolve junction points, much like it does not resolve bind mounts.

@dscho dscho added the question label Mar 8, 2019
@JFLarvoire
Copy link

Junctions are often used as symbolic links to directories in Windows, for several reason:

  1. They existed since Windows 2000, whereas symbolic links were introduced in Windows Vista.
  2. Junctions can be created without any special right, whereas creating a symlinkd requires the SeCreateSymbolicLinkPrivilege right, which is not granted by default, even to system administrators.

See: https://docs.microsoft.com/en-us/sysinternals/downloads/junction

As using junctions instead of symlinkds was long recommended by Microsoft themselves, I'd argue that I'm surely not the only one doing this, and that you should resolve junctions as well. :-)

@dscho
Copy link
Member

dscho commented Mar 8, 2019

@JFLarvoire Thank you for re-iterating those arguments. Of course, as you might imagine, I am fully aware of that.

And let me take the time to re-iterate the really, really important argument that junction points are equivalent to bind mounts, not to symbolic links.

@JFLarvoire
Copy link

I don't care if they're real symbolic links or not.
All I ask is that it should work in the presence of a junction, and currently it does not.

@dscho
Copy link
Member

dscho commented Mar 8, 2019

I don't care if they're real symbolic links or not.

It is quite clear that you don't care.

Since Git for Windows is not your personal pet project, it is however important that it keeps working for pretty much everybody else as well as possible, too.

And just because you do not want to acknowledge that junction points are not, in fact, symbolic links (even if they were used as such for a long time due to the absence of real symbolic links) does not make it less of a fact.

Junction points are not symbolic links. It would be a mistake to treat them as such.

See also this comment by somebody way smarter than you or I: #437 (comment)

@dscho
Copy link
Member

dscho commented Mar 8, 2019

@bk2204 I am starting to come to the conclusion that Git LFS should do its best to treat junction points the same way it would treat bind mounts on Linux: by not resolving them.

@JFLarvoire
Copy link

Agreed, the important thing is that all git components resolve paths consistently, and work well together in the presence of junctions, symlinks, and symlinkds.

@bk2204
Copy link
Author

bk2204 commented Mar 9, 2019

@dscho We unfortunately can't do that. The Go function filepath.EvalSymlinks resolves junction points (apparently via GetFinalPathNameByHandle), and we don't have any alternative functions to use. (Writing our own is not going to happen.)

Wikipedia claims that junction points are symbolic links, which is why I opened this issue. WSL represents them that way as well.

The Git LFS developers don't typically use Windows, but we would of course like things to be as functional as possible there. We'll put in a change that prevents the underlying issue from reoccurring with an error message, but that won't let users use junctions in this case. We need to produce a path relative to the repo root for locking purposes, and canonicalization is required because Windows is not case-sensitive and paths can have multiple representations.

I'm willing to entertain other suggestions as to how to fix this, but canonicalization is definitely required.

@dscho
Copy link
Member

dscho commented Mar 10, 2019

@bk2204 in another ticket about Cygwin symlinks (which are yet a different way to emulate symlinks on Windows), I suggested enhancing the Windows part of core.symlinks to support a different value than true or false and handling it accordingly.

We could do the same here (with the caveat that junctions really are not symlinks, so quite a few restrictions apply): implement core.symlinks = junctions.

I will gladly guide any volunteer who wants to implement this.

@dscho
Copy link
Member

dscho commented Mar 8, 2020

@bk2204 Git for Windows was recently switched to use GetFinalPathByHandle() to resolve symlinks. Would you be able to test whether v2.25.1 fixes this here issue?

@dscho
Copy link
Member

dscho commented Oct 15, 2021

Closing this as stale.

@dscho dscho closed this as completed Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants