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

--build_python_zip=false broken on Windows #1840

Closed
KoltesDigital opened this issue Apr 11, 2024 · 10 comments · Fixed by #2015
Closed

--build_python_zip=false broken on Windows #1840

KoltesDigital opened this issue Apr 11, 2024 · 10 comments · Fixed by #2015

Comments

@KoltesDigital
Copy link
Contributor

🐞 bug report

Affected Rule

The issue is caused by the function **_transition_py_impl** so impacts **py_binary** and **py_test**.

Is this a regression?

The bug was not present with WORKSPACE method.

Description

I can't use --build_python_zip=false anymore.

However this might not be a bad thing. I've put that in .bazelrc a long time ago, when I had some troubles with caches. Maybe this is not needed anyway.

🔬 Minimal Reproduction

Any repo using rules_python with bzlmod on Windows, I guess.

🔥 Exception or Error


ERROR: C:/.../BUILD:X:X: in _transition_py_binary rule //:requirements.update:
Traceback (most recent call last):
        File "C:/.../external/rules_python~/python/config_settings/transition.bzl", line 57, column 28, in _transition_py_impl
                ctx.actions.symlink(
Error in symlink: Exactly one of "target_file" or "target_path" is required
ERROR: C:/.../BUILD:X:X: Analysis of target '//:requirements.update' failed

From what I understand, because zipfile is still None, because the previous loop didn't find any zip file in the runfiles.

🌍 Your Environment

Operating System: Windows 10 x64
Output of bazel version:

  
Build label: 7.1.1
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Thu Mar 21 18:11:32 2024 (1711044692)
Build timestamp: 1711044692
Build timestamp as int: 1711044692
  

Rules_python version: 0.31.0

@KoltesDigital
Copy link
Contributor Author

Well, now binaries/tests are incredibly slow. Good reminder why I was using it.

@KoltesDigital
Copy link
Contributor Author

It may not be linked to bzlmod after all, because it only adds an extension of top of previous functions. Surely this is due to upgrading from 0.19.0 😄

@KoltesDigital KoltesDigital changed the title bzlmod doesn't support --build_python_zip=false on Windows --build_python_zip=false broken on Windows Apr 26, 2024
@KoltesDigital
Copy link
Contributor Author

Still not working with 0.32.2.

@aignas
Copy link
Collaborator

aignas commented May 15, 2024

Could you test if this is still not working well with 0.29.0 please? That is before we switched to starlark implementation of the rules.

@KoltesDigital
Copy link
Contributor Author

@aignas Well, I tried to bisect. The flag still works in version 0.25.0, with only bazel_dep in MODULE.bazel and the pip deps kept in WORKSPACE. But 0.26.0 had a weird dep cycle so I moved everything into MODULE.bazel, and then the flag doesn't work. So I couldn't tell if it's related to the version in itself, or to the switch to bzlmod.

As a side note, the code dealing with the zips has been added in 0.15.0.

@aignas
Copy link
Collaborator

aignas commented Jun 3, 2024

Does head work for you? The latest commits may be of interest to you.

@KoltesDigital
Copy link
Contributor Author

@aignas Still not working as of ae1e1a0 .

@aignas
Copy link
Collaborator

aignas commented Jun 4, 2024

#1929 with the optional features may be of interest. The features are off by default, so you have to enable them as per https://rules-python.readthedocs.io/en/latest/api/python/config_settings/index.html#bootstrap_impl

I thought that this would have had some effect, but it might be the case that it is unrelated.

Since none of the maintainers are developing on Windows, PRs are welcome to address this issue.

@KoltesDigital
Copy link
Contributor Author

@aignas I'm willing to contribute, but I need some guidance. I've tried to handle the case where the error is raised (quite simple with an extra if), but it then fails at execution.

Say there's a pytest_test(name = "foobar", ...) in some path/BUILD file. Running bazel test ... then produces foobar.exe and _foobar.exe, with apparently the same runfiles. I'm not sure of what happens here, I believe foobar.exe is responsible for spawning _foobar.exe with backed arguments, the latter launches the actual tests. Anyway, the test fails and the log contains:

Executing tests from //path:foobar
-----------------------------------------------------------------------------
X:/external/rules_python~~python~python_3_10_x86_64-pc-windows-msvc/python.exe: can't open file 'X:\\execroot\\_main\\bazel-out\\x64_windows-fastbuild-ST-eaf12dacd369\\bin\\path\\foobar.exe.runfiles\\_main\\path\\foobar': [Errno 2] No such file or directory

In .../foobar.exe.runfiles/_main/path there's indeed no foobar file. However, there are foobar.exe, _foobar, and _foobar.exe. These three files are symlinks back to generated files just before the runfiles tree (e.g. X:/execroot/_main/bazel-out/x64_windows-fastbuild-ST-eaf12dacd369/bin/path/foobar.exe.runfiles/_main/path/_foobar -> X:/execroot/_main/bazel-out/x64_windows-fastbuild-ST-eaf12dacd369/bin/path/_foobar).

If my guess is correct, foobar.exe should spawn / be looking for ...runfiles/_main/path/_foobar.exe, rather than ...runfiles//main/path/foobar. The problem is, I have no idea where to change that.

Any explanation is welcome.

@KoltesDigital
Copy link
Contributor Author

Well I had time to look at it again. @aignas please have a look at the PR!

github-merge-queue bot pushed a commit that referenced this issue Jul 4, 2024
This fixes #1840 by supporting again `--build_python_zip=false` on
Windows.

When the zip file isn't build, the transition executable looks for the
eponymous bootstrap script but the latter doesn't exist. I've just added
a symlink, and refactored a bit because logic would have been
duplicated.

It seems you don't run CICD on Windows. FWIW I've manually tested it,
both with `build_python_zip` as `true` and `false`.

---------

Co-authored-by: Richard Levasseur <rlevasseur@google.com>
github-merge-queue bot pushed a commit that referenced this issue Jul 10, 2024
This is a more comprehensive regression test for verifying
`--build_python_zip` is
actually working
(#1840)

This also creates a small framework to make it easier to write
integration tests that
need to customize the environment bazel runs in and check the output of
bazel itself.
I figure this will be helpful for writing simple verification tests for
repository/bzlmod
phase logic (i.e. set the debug env vars and grep the output). While we
should avoid heavy
usage of these bazel-in-bazel tests, a bit of grepping logs would go a
long way for covering
edge cases that examples don't cover.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants