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

Support creating symlinks outside elevated sessions #1188

Merged
merged 2 commits into from
May 31, 2017

Conversation

dscho
Copy link
Member

@dscho dscho commented May 30, 2017

This Pull Request is intended to supersede #1184.

The main difference is that I fixed a rather serious bug (the return value was declared incorrectly, possibly fooling Git into believing that it could create a symbolic link when it really failed, and that really happened here during my tests), and that the file/directory flags are modified only once. I also use the GetVersion() API instead of the POSIX-y uname().

@dscho
Copy link
Member Author

dscho commented May 30, 2017

Oh, and I also decided to test for the build number of the first Insider build that supported the flag...

dscho added 2 commits May 30, 2017 22:44
…only)

This patch fixes signature of the CreateSymbolicLinkW() function's
declaratoin. The previous declaration claimed that said function returns
a BOOL, while it really returns a BOOLEAN.

This is not an academic distinction: BOOL is defined as an int (i.e.
32-bit) and BOOLEAN as an unsigned char (i.e. 8-bit). Therefore, the
return value 0 (meaning, the least-significant 8 bits are all zero)
could be mistaken to indicate a successful creation of the symbolic
link (because the remaining 24 bits are undefined, and quite likely
non-zero).

Signed-off-by: Johannes Schindelin <[email protected]>
With Windows 10 Build 14972 in Developer Mode, a new flag is supported
by CreateSymbolicLink() to create symbolic links even when running
outside of an elevated session (which was previously required).

This new flag is called SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE and
has the numeric value 0x02.

Previous Windows 10 versions will not understand that flag and return an
ERROR_INVALID_PARAMETER, therefore we have to be careful to try passing
that flag only when the build number indicates that it is supported.

For more information about the new flag, see this blog post:
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/

This patch is loosely based on the patch submitted by Samuel D. Leslie
as git-for-windows#1184.

Signed-off-by: Johannes Schindelin <[email protected]>
@ralish
Copy link

ralish commented May 31, 2017

Looks good to me and nice pick-up on the return type of CreateSymbolicLinkW!

Only comment is I think it'd be nice to preserve the #ifndefs corresponding to the CreateSymbolicLinkW flags for readability in the absence of importing the actual header. Otherwise we do end-up with a fair few magic numbers that are going to be potentially quite non-obvious to others. A minor point for sure, but in general I think the less magic numbers the better.

@dscho
Copy link
Member Author

dscho commented May 31, 2017

in general I think the less magic numbers the better.

I agree. However, the header files available in Git for Windows' SDK do not even have that flag yet. So even if we bumped _WINNT_VER to 0x600 (i.e. require Windows Vista or later, which leads to all kinds of unrelated compile errors e.g. because of the pollfd declarations), we would not be able to include the header and use the symbol.

As to #define'ing it, we also have to consider Git's pretty strict formatting rules: 80 columns per line, no more. And that symbol is really long (I thought only Germans use stupidly long names, but apparently I was wrong).

As the symbol's name has been mentioned in the code comment above, I am a bit less concerned about the magic-ness of the 2, too ;-)

Thanks for the look-over!

@dscho dscho merged commit a4d0d5f into git-for-windows:master May 31, 2017
@dscho dscho deleted the unprivileged-symlinks branch May 31, 2017 09:47
@dscho dscho added this to the v2.13.0(1) milestone May 31, 2017
@ralish
Copy link

ralish commented May 31, 2017

Excellent, thanks for the explanation as to the rationale. As for the missing flag, I addressed that in a separate patch a few days ago to the Mingw-w64 developers so it should be available in a future release (though doesn't solve the 80 columns per line issue, but indicating anyway just for reference).

Thanks for merging. Looking forward to finally having 100% usable symlinks on Windows in Git!

dscho added a commit to git-for-windows/build-extra that referenced this pull request May 31, 2017
Git [now uses the flag introduced with Windows 10 Creators
Update to create symbolic links without requiring elevated
privileges](git-for-windows/git#1188) in
Developer Mode.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented May 31, 2017

Looking forward to finally having 100% usable symlinks on Windows in Git!

Yesss! And thanks for all your help!

dscho added a commit that referenced this pull request Jun 5, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 7, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 7, 2017
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jun 7, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 7, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 19, 2017
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jun 19, 2017
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jun 20, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 21, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 21, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jun 21, 2017
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jun 25, 2017
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jan 18, 2018
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jan 20, 2018
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jan 22, 2018
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Jan 22, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Jan 22, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Feb 16, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Mar 23, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Apr 3, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request May 29, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request May 29, 2018
Support creating symlinks outside elevated sessions
PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018
Support creating symlinks outside elevated sessions
PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018
Support creating symlinks outside elevated sessions
PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Aug 22, 2018
Support creating symlinks outside elevated sessions
dscho added a commit to dscho/git that referenced this pull request Aug 22, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Aug 23, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Aug 23, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Aug 23, 2018
Support creating symlinks outside elevated sessions
jamill pushed a commit to jamill/git that referenced this pull request Aug 28, 2018
Support creating symlinks outside elevated sessions
jamill pushed a commit to jamill/git that referenced this pull request Sep 5, 2018
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Sep 10, 2018
Support creating symlinks outside elevated sessions
jamill pushed a commit to jamill/git that referenced this pull request Sep 11, 2018
Support creating symlinks outside elevated sessions
git-for-windows-ci pushed a commit that referenced this pull request Sep 24, 2018
Support creating symlinks outside elevated sessions
dscho added a commit that referenced this pull request Oct 10, 2018
Support creating symlinks outside elevated sessions
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