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

Cannot use runfiles.python import path when running bazel coverage #2009

Open
michaelboyd2 opened this issue Jun 24, 2024 · 3 comments
Open

Comments

@michaelboyd2
Copy link

🐞 bug report

Affected Rule

compile_pip_requirements

Is this a regression?

Not sure.

Description

In some situations it is possible to get a conflict between the python module from the coverage package and the python package from the runfiles library. I would say this is due to the slightly risky use of "python" as a module/package name and the import from python.runfiles import runfiles in dependency_resolver.py.

🔬 Minimal Reproduction

cd examples/multi_python_versions
bazel coverage --incompatible_default_to_explicit_init_py //requirements:requirements_3_10_test

🔥 Exception or Error

michaelboyd multi_python_versions $ bazel coverage --incompatible_default_to_explicit_init_py //requirements:requirements_3_10_test
INFO: Using default value for --instrumentation_filter: "^//requirements[/:]".
INFO: Override the above default with --instrumentation_filter
INFO: Analyzed target //requirements:requirements_3_10_test (0 packages loaded, 0 targets configured).
FAIL: //requirements:requirements_3_10_test (see /private/var/tmp/_bazel_michaelboyd/52019027f6106b6e52388536c05b77eb/execroot/_main/bazel-out/darwin_arm64-fastbuild-ST-eaf12dacd369/testlogs/requirements/requirements_3_10_test/test.log)
INFO: From Testing //requirements:requirements_3_10_test:
==================== Test output for //requirements:requirements_3_10_test:
Traceback (most recent call last):
  File "/private/var/tmp/_bazel_michaelboyd/52019027f6106b6e52388536c05b77eb/execroot/_main/bazel-out/darwin_arm64-fastbuild-ST-eaf12dacd369/bin/requirements/requirements_3_10_test.runfiles/rules_python~/python/private/pypi/dependency_resolver/dependency_resolver.py", line 28, in <module>
    from python.runfiles import runfiles
ModuleNotFoundError: No module named 'python.runfiles'; 'python' is not a package

🌍 Your Environment

Operating System:

  
MacOS
  

Output of bazel version:

  
Bazelisk version: development
Build label: 7.2.0
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Mon Jun 10 13:04:55 2024 (1718024695)
Build timestamp: 1718024695
Build timestamp as int: 1718024695  

Rules_python version:

  
Tested on main: ea49937782fb0b969c72625f3b397f4bab53d412
  
@michaelboyd2
Copy link
Author

michaelboyd2 commented Jun 24, 2024

I feel like this patch fixes the immediate issue:

--- python/pip_install/tools/dependency_resolver/dependency_resolver.py
+++ python/pip_install/tools/dependency_resolver/dependency_resolver.py
@@ -25,7 +25,7 @@ import click
 import piptools.writer as piptools_writer
 from piptools.scripts.compile import cli

-from python.runfiles import runfiles
+from rules_python.python.runfiles import runfiles

 # Replace the os.replace function with shutil.copy to work around os.replace not being able to
 # replace or move files across filesystems.

I could raise a PR with this change if it was thought to be a good idea.

Edit: Now think this may not work with bazlmod where the directory is rules_python~

@michaelboyd2
Copy link
Author

michaelboyd2 commented Jun 24, 2024

It is also possible to fix with legacy_create_init = True but I feel this shouldn't be needed? This issue implies that this will not be an option long term (although I imagine this may not change quickly).

@aignas
Copy link
Collaborator

aignas commented Aug 6, 2024

I think this is the same issue that I saw below:

I found this when working on an unrelated change. This means that technically, a root module could break the non-root module if a toolchain with coverage is added and the non-root module has not verified that everything works with and without coverage, which itself would be annoying. This would only manifest in breakage when executing bazel coverage.

@aignas aignas changed the title Fix unsafe import statement in dependency_resolver Cannot use runfiles.python import path when running bazel coverage Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants