Cannot use `runfiles.python` import path when running `bazel coverage`

[michaelboyd2]

🐞 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]

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]

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]

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.

[kilian-funk-wayve]

@aignas Are you aware of any activity on this issue? Is there an effective workaround?

My workaround right now is to look through the sys.path and use the rules_python path to manually load the runfiles module in case the regular import fails.

I would be happy to support, but am unsure if this should be solved through a special coverage solution or if this is more of a basic Bazel/Python import issue.

My observation so far is that in the coverage case I find /.../external/python_3_10_x86_64-unknown-linux-gnu_coverage/coverage on the path. However, I would have expected to rather find the parent directory in the path. I was not able to understand where this path is actually added to sys.path.

[aignas]

To be honest, I think that the best way to fix this would be to exclude the test from being executed when running bazel coverage. There is a way to have a select statement to generate the right target_compatible_with attribute value.

Similar to how https://github.com/bazel-contrib/rules_python/blob/d08cf537541ffc99bbd98dd71949a507516a4ff8/python/private/hermetic_runtime_repo_setup.bzl#L225-L229 is done.

[kilian-funk-wayve]

@aignas Thanks for the hint. However, that will exclude any code using the idiomatic Bazel way to depend on runtime resources, which may be a really large portion of the code base...

Or am I missing something here?

[aignas]

I am only suggesting we do this for the requirements.test that is documented in the first post of the issue. @kilian-funk-wayve, are you experiencing this with regular tests?

If that is the case, for you the current workaround might be to install the bazel-runfiles library via PyPI.