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

Native symlinks support without UAC disable #13

Closed

Conversation

Karlson2k
Copy link

This PR add support for directory junction creation as symlink.
Directory junction is supported since Win2k and do not require admin rights or UAC disable.
File symlinks is not supported this way, but can be emulated by hardlinks.

Note: Cygwin already is able to read junction, this PR add only support for creation.
Upstream PR: Alexpux#11

@dscho
Copy link
Member

dscho commented Oct 5, 2015

Thank you for your contribution! I will have a look after resting for a while (Git for Windows 2.6.1 was just released).

@dscho
Copy link
Member

dscho commented Feb 17, 2016

Okay, it took a long while. Sorry about that. In my defence: I keep having to work on tickets with incomplete and incorrect reports (for example, yesterday I chased an encoding bug in Git GUI for hours, only to find out that the bug report led me in the wrong direction and it was not an encoding bug at all).

Hopefully you are still willing to work on this PR?

The main problem I see is that this patch pretends that all symlinks can be emulated through reparse mount points. Yet IIRC reparse mount points can only represent symlinks pointing to directories, and only to absolute paths, not relative ones.

The second problem is that it looks as if the default allow_winsymlnks is changed, quite possibly breaking tons of people's expectations in backwards compatibility.

Can these two concerns be addressed?

@Karlson2k
Copy link
Author

Yes, I'm still willing to work on this PR. 😄

On windows both "Directory Junctions" and "Symbolic links" are kinds of reparse mount points. As well as directory-mounted disk volumes.
Junctions can point only to directories, right. For files we must use symlink which required admin rights or hardlinks (which have other drawbacks). So this will solve only one part of symlinks problem on Windows, but I think most important part.
And yes, only absolute ones. So moved/copied local repo must "modified" for git as links destinations were changed.

Symlinks default is changed but this shouldn't be a problem as new setting is backward compatible. Currently dir symlink creation just resulting in dir copy, I don't think that many users depend on this, because for copy better to use copy. 😉

I think that patch will be accepted soon in Cygwin (need paper work and few details). Cygwin default for symlinks will not be changed, but I hope that msys2 default will be changed (as msys2 already differs from Cygwin). If not, than we can just use local patch or variable for enabling symlinks.

@dscho
Copy link
Member

dscho commented Feb 17, 2016

I think that patch will be accepted soon in Cygwin (need paper work and few details).

Great! We will inherit this automatically, then!

Cygwin default for symlinks will not be changed, but I hope that msys2 default will be changed (as msys2 already differs from Cygwin). If not, than we can just use local patch or variable for enabling symlinks.

I'd rather stay with the current default. Remember: whenever you ship with any setting for an extended amount of time, users will start to rely on it. Scripts will expect that behavior, and developers will be upset when they have to "fix" systems that worked well so far, thankyouverymuch.

But it should not be too much to ask the user who desires safenative to be enabled (and understands its serious limitations when it comes to the promises of POSIX symlinks) to set their MSYS environment variable accordingly.

Given that we inherit any improvement in Cygwin's runtime, I'd favor closing this here ticket. Okay?

@Karlson2k
Copy link
Author

OK, closed PR as expecting it will be accepted upstream.

@Karlson2k Karlson2k closed this Feb 17, 2016
@dscho
Copy link
Member

dscho commented Feb 17, 2016

Perfect, thank you so much for your contribution!

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