-
Notifications
You must be signed in to change notification settings - Fork 536
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
rules_python 0.29.0 started enforcing symlink usage on Windows (was: broke Bazel on Windows) #1723
Comments
Looks like this doesn't happen with 0.28.0 |
Weird. 0.29.0 doesn't have many changes: 0.28.0...0.29.0 This commit looks like a likely culprit: 4fe0db3 It has some mentions of some windows-specific hacks that had to be done. Error code -1073741515 seems to be something about corrupt files or missing files. The error says its happening during the pip phase, i.e when it's trying to go download the packages and install them. Being unable to find some dll's because it's confused by the symlink/realpath-of-the-symlink seems plausible |
I'm curious in the env differences between rules_python CI and bazel. Are there any? What kind of Windows python is installed/setup on bazel CI? Or is it on local? I agree with the judgment that that commit is the most likely culprit, but curious why our CI did not complain. |
Yes, for Bazel, we did use the local python due to #1356, see On CI Windows VMs, Python3 is installed by this script: |
I don't have access to a Windows machine, but I think it would be interesting to see how the get interpreter function which does .realpath to resolve the interpreter in your case. In both cases, CI and local the resolution should use the host interpreter toolchain and I am not sure why it works only in CI. Is the Windows version different? Are we missing some .bazelrc flags on the bazel end that may affect symlink behaviour? |
@aignas Can you help me compose a single command that I can use to reproduce this issue? Basically, what's been executed at rules_python/python/pip_install/pip_repository.bzl Lines 776 to 780 in 0ff94b1
|
… third party deps Use this by running `bazel build @repro//..` in the `rules_python` module. The build will fail all the time because I do `sys.exit(1)` in the script but the expectation that on Windows we can reproduce the #1723 and the python script will exit before `sys.exit(1)` is called with a different error.
@meteorcloudy, from what I can understand, #1736 might reproduce the error seen here, but if it doesn't, let me know. |
@aignas Thank you very much, here is the output on a Windows VM:
|
Hmm, but I didn't see the exact error code. Let me know if you want me to test any python command with the host Python. |
Use the same binary path that the regular toolchain is using in an attempt to see if the behaviour is any different. Attempt to fix #1723
I was reading the code that is executed once more and I was wondering if the need for #1745 might be causing the errors. I don't have a Windows machine but the CI still says that it is green: https://buildkite.com/bazel/rules-python-python/builds/7106#018d7924-16c6-4995-9995-e91fbdd7f9ac @meteorcloudy, if you have time, could you try and see if the following patch fixes $ gh pr diff 1745 Gives the output diff --git a/python/private/toolchains_repo.bzl b/python/private/toolchains_repo.bzl
index c586208a7..4c039ea84 100644
--- a/python/private/toolchains_repo.bzl
+++ b/python/private/toolchains_repo.bzl
@@ -238,11 +238,16 @@ exports_files(["python"], visibility = ["//visibility:public"])
(os_name, arch) = get_host_os_arch(rctx)
host_platform = get_host_platform(os_name, arch)
+
+ is_windows = (os_name == WINDOWS_NAME)
+ python3_binary_path = "python.exe" if is_windows else "bin/python3"
+
host_python = rctx.path(
Label(
- "@@{py_repository}_{host_platform}//:python".format(
+ "@@{py_repository}_{host_platform}//:{python}".format(
py_repository = rctx.attr.name[:-len("_host")],
host_platform = host_platform,
+ python = python3_binary_path,
),
),
) |
I'm overriding rules_python to a version 0.29.0 with this patch applied, but still got:
|
The failing command is
|
Manually running the python binary resulted:
|
I think I know what's going on. I assume If I do So basically, 4fe0db3 requires all windows users to turn on symlink support (https://bazel.build/configure/windows#symlink), which requires admin privilege, is there any way around this? |
Also explains why CI is green for rules_python: rules_python/examples/bzlmod/.bazelrc Line 10 in 0ff94b1
|
Thanks for the digging. That finally makes sense, I have one workaround in mind which I'll try. |
@meteorcloudy, I think #1745 should fix your issue, could you test |
@aignas I can confirm #1745 fixes the issue for Bazel! Thanks!
|
…1745) Previously we would only symlink the interpreter binary itself when creating the hermetic host toolchain used in setting up the `whl_library` repositories. This works on UNIX platforms and Windows if they have the following in their `.bazelrc`: ``` startup --windows_enable_symlinks ``` In our CI we had the same lines but the users did not need to add them until the `0.29.0` forced them to have them because we actually started using symlinks on Windows. If the symlinks are not enabled on the host platform `bazel` tries to be helpful and copies the files over instead of making the links. We are leveraging this to just symlink all of the contents of the python interpreter repository for the host platform to the `_host` toolchain repository. Fixes #1723
🐞 bug report
Affected Rule
rules_python 0.29.0
Is this a regression?
Yes, rules_python 0.26.0 worked fine for Bazel
Description
I tried to upgrade rules_python for Bazel, which caused the following breakage on Windows:
https://buildkite.com/bazel/google-bazel-presubmit/builds/76496#018d45ca-4b9c-4572-83dc-9c605e169298
🔬 Minimal Reproduction
bazel test //src/test/py/bazel:first_time_use_test
🔥 Exception or Error
🌍 Your Environment
Operating System:
Output of
bazel version
:Rules_python version:
Anything else relevant?
The text was updated successfully, but these errors were encountered: