Fix symlink creation on older Windows versions#13488
Fix symlink creation on older Windows versions#13488khogeland wants to merge 7 commits intobazelbuild:masterfrom
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
khogeland
force-pushed
the
windows-symlink-patch
branch
from
9816cf0 to
b376ea5
Compare
khogeland
force-pushed
the
windows-symlink-patch
branch
from
f38831f to
cea93b5
Compare
jin
added
area-Windows
There was a problem hiding this comment.
Thanks! This indeed looks better than the previous check overall.
But one corner case is that: what if you are on a Windows version prior to Windows 10 Creators Update (1703), and you also have developer enabled. In this case, the implementation in this PR will try to create the symlink with the SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE flag, which the system doesn't support. However it should be able to create the symlink if the user has admin right.
Can you please verify if this is a problem on Windows Server 2016?
|
I was going to say that Windows 1703 (even Enterprise) is long EOL since 2019, but indeed Windows Server 2016 is still supported until 2022 😳 |
|
Ah, alright, I misinterpreted the Bazel docs and thought those features were introduced at the same time, but that is a possible situation. |
khogeland
requested review from
aiuto,
floriographygoth,
gregestren and
jin
as code owners
|
@meteorcloudy I've added an explicit OS version check to make sure Bazel never passes the flag to an incompatible version of Windows. |
meteorcloudy
left a comment
meteorcloudy
left a comment
There was a problem hiding this comment.
Thank you so much! Getting rid of the dummy link approach is really nice!
|
@khogeland Can you please sign the CLA so that I can merge this PR? 😃 |
|
@meteorcloudy I see the |
|
@khogeland Oh, my mistake, everything is good to go! |
The OS version check in #13488 breaks the developer mode symlink behavior. `IsWindowsVersionOrGreater` does not work as advertised, and returns false on Windows 10 if not called from an executable with an associated application manifest declaring its compatibility for Windows 10. (Very cool, Microsoft.) The other methods of checking OS version are far more verbose and complicated, which doesn't seem warranted here. As an alternative workaround, this PR replaces the ahead-of-time version check with a retry without the flag if the function reports an invalid argument exception. @meteorcloudy Closes #13629. PiperOrigin-RevId: 382734470
This patch ensures that the `SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE` flag is not passed to `CreateSymbolicLinkW` when the developer mode registry key is not present and enabled on a supported version of Windows. This allows symlinks to be created when Bazel is run with elevated privileges, regardless of Windows version or developer mode. I also removed the dummy file creation check while refactoring that code for reuse. It seemed overly complicated vs. simply checking the registry and failing during runfiles creation if we're not admin. Please let me know if there's some subtle reason it needed to be done that way. Fixes #13169 - tested on Windows Server 2016. Closes #13488. PiperOrigin-RevId: 375035407
4 participants
This patch ensures that the
SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATEflag is not passed toCreateSymbolicLinkWwhen the developer mode registry key is not present and enabled on a supported version of Windows. This allows symlinks to be created when Bazel is run with elevated privileges, regardless of Windows version or developer mode.I also removed the dummy file creation check while refactoring that code for reuse. It seemed overly complicated vs. simply checking the registry and failing during runfiles creation if we're not admin. Please let me know if there's some subtle reason it needed to be done that way.
Fixes #13169 - tested on Windows Server 2016.