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

Bazel fails to create symlinks on slightly older versions of Windows #13169

Closed
konste opened this issue Mar 6, 2021 · 7 comments
Closed

Bazel fails to create symlinks on slightly older versions of Windows #13169

konste opened this issue Mar 6, 2021 · 7 comments
Labels
area-Windows Windows-specific issues and feature requests P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request

Comments

@konste
Copy link

konste commented Mar 6, 2021

Description of the problem / feature request:

When symlinks support is enabled for Windows platform with the startup option --windows_enable_symlinks it only works on Windows version 1703 (Creators Update, build 15063) and above. Symlink creation fails on earlier versions of Windows even though symlinks are supported since Windows Vista and the process is elevated.

Feature requests: what underlying problem are you trying to solve with this feature?

While dev machines tend to be rather up to date, the build servers are often stale. In our company we use Windows Server 2016 version 1607 and Bazel build fails to create symlinks there despite the feature is fully supported in that version of Windows.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

This is the place in the Bazel code which causes the problem. The code at fault looks like this:

  if (!CreateSymbolicLinkW(name.c_str(), target.c_str(),
                           SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE)) {

While symlinks are supported in Windows for 15 years (since Vista) that particular flag SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE is relatively new and recognized by Windows only starting from version 1703 (Creators Update, build 15063). On previous versions it causes CreateSymbolicLinkW to fail with GetLastError() returning 0x57 ERROR_INVALID_PARAMETER.

It would be great if Bazel checks the version of the OS and only apply SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE when it is supported by the underlying platform.

What operating system are you running Bazel on?

Windows, Linux, Mac.

What's the output of bazel info release?

4.0.0

@aiuto aiuto added area-Windows Windows-specific issues and feature requests team-Remote-Exec Issues and PRs for the Execution (Remote) team untriaged labels Mar 31, 2021
@coeuvre
Copy link
Member

coeuvre commented May 10, 2021

@meteorcloudy Can you please have a look?

@meteorcloudy meteorcloudy added P2 We'll consider working on this in future. (Assignee optional) type: feature request and removed untriaged labels May 10, 2021
@meteorcloudy
Copy link
Member

The change implemented --windows_enable_symlinks assumes "the user has developer mode enabled and is running a Windows 10 build 14972 or greater."

This will be a nice thing to have for Bazel to support older version of Windows, but I don't have capacity to work on this right now. Contribution is welcome and we do have an implementation in our build-runfiles-widnows binary as an example here: https://cs.opensource.google/bazel/bazel/+/master:src/main/tools/build-runfiles-windows.cc;l=250?q=CreateSymbolicLinkW&ss=bazel
The problem here is how to elegantly check if the current Windows version support SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE or not.

@konste
Copy link
Author

konste commented May 10, 2021

@meteorcloudy the problem looks to me easy to fix. Here is the problematic code. At this time we don't check what CreateSymbolicLinkW returns, but we should. If it returns 0x57 ERROR_INVALID_PARAMETER this is a direct indication that we are on Windows which does not understand SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE attribute and we just need to re-run the same API without that attribute. That decision needs to be stored in the local static variable and that variable would control which version of CreateSymbolicLinkW to use for each subsequent invocation. Easy enough?
Do you think you may find the capacity to add those few lines?

@meteorcloudy
Copy link
Member

@konste Thanks for the suggestion! I will look into this as soon as I have time. If it's very urgent for you, I'm happy to review a PR as well.

@khogeland
Copy link

khogeland commented May 14, 2021

@konste A cleaner approach would be to reuse IsDeveloperModeEnabled and not pass the flag if it's not enabled, that handles both the case where we're on an old version of Windows and the case where we're admin but dev mode is disabled.

@khogeland
Copy link

Hey @meteorcloudy I opened a PR to fix this. Please give it a look. 🙂

@meteorcloudy
Copy link
Member

@khogeland Thank you!

katre pushed a commit that referenced this issue Jul 12, 2021
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
katre pushed a commit to katre/bazel that referenced this issue Jul 13, 2021
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 bazelbuild#13169 - tested on Windows Server 2016.

Closes bazelbuild#13488.

PiperOrigin-RevId: 375035407
katre pushed a commit to katre/bazel that referenced this issue Jul 13, 2021
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 bazelbuild#13169 - tested on Windows Server 2016.

Closes bazelbuild#13488.

PiperOrigin-RevId: 375035407
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants