bazel-runfiles module always throws ValueError

[AndrewGuenther]

🐞 bug report

Affected Rule

bazel-runfiles ackage

Is this a regression?

Appears to be, we were using bazel-runfiles on v0.12.0 with no issue

Description

When running a py_binary target, the main file is executed from within the runfiles tree, but everything imported points back to the original workspace. As a result of this, if you try to use bazel-runfiles, you're always met with this ValueError:

ValueError: /home/andrew/workspace/rules_python_runfiles_repro/my_module/__init__.py does not lie under the runfiles root /home/andrew/.cache/bazel/_bazel_andrew/43e798d9071ddd30271aac70168abdc4/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles

🔬 Minimal Reproduction

https://github.com/AndrewGuenther/rules_python_1631_repro

Minimal reproduction repository linked above.

🔥 Exception or Error

ValueError: /home/andrew/workspace/rules_python_runfiles_repro/my_module/__init__.py does not lie under the runfiles root /home/andrew/.cache/bazel/_bazel_andrew/43e798d9071ddd30271aac70168abdc4/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles

🌍 Your Environment

Operating System:

  
Ubuntu
  

Output of bazel version:

  
6.4.0
  

Rules_python version:

  
0.27.1
  

Anything else relevant?

[fmeum]

@rickeylev Happy to help, but I would need to know whether there have been any intentional changes to the way Python imports are resolved.

[rickeylev]

The only thing I can think of relating to imports is some changes to py_proto_library. It was changed to add "virtual imports" (basically the directory where files are put when strip_prefix is used) to the path.

[AndrewGuenther]

I can confirm that this issue only occurs when bzlmod is enabled. The imports still point to the local workspace, but the ValueError is not thrown from bazel-runfiles

[AndrewGuenther]

Just pushed a branch called workspace to the repro repo to demonstrate.

[AndrewGuenther]

Sorry for the comment spam, but I've added some additional logging and have a few more findings to share:

The import behavior is the same in both WORKSPACE and MODULE.bazel modes, what differs however is the result of this check:

https://github.com/bazelbuild/rules_python/blob/main/python/runfiles/runfiles.py#L137-L141

        if source_repo is None and self._repo_mapping:
            # Look up runfiles using the repository mapping of the caller of the
            # current method. If the repo mapping is empty, determining this
            # name is not necessary.
            source_repo = self.CurrentRepository(frame=2)

In both WORKSPACE and MODULE.bazel mode, source_repo is None, but self._repo_mapping is only non-empty in MODULE.bazel mode. The ValueError being thrown is coming from inside this check. Commenting out this check (not saying that's the right fix) causes both WORKSPACE and MODULE.bazel modes to return the same result. So this is something that was valid behavior before, but is not when bzlmod is enabled.

I'm not sure what the "right" thing to do is here since I presume there's an argument that the behavior in WORKSPACE mode is also wrong and the import behavior could be a regression.

[AndrewGuenther]

Just verified that this import behavior can be observed as far back as the 0.5.0 release of rules_python. I couldn't get earlier versions to work at all, but the point is more that this is not new behavior.

I also verified that the ValueError is thrown in every version of rules_python that supports bzlmod (starting in 0.14.0)

[aignas]

Thanks for this investigation!

[fmeum]

When I run the reproducer, the sys.path looks like this:

['/home/fhenneke/tmp/rules_python_1631_repro', '/home/fhenneke/.cache/bazel/_bazel_fhenneke/8c09f903aff88728a492063a5dce0a79/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles', ...]

This is why the import in hello_world.py resolves as it does: into the source root, not the runfiles directory.

Traceback (most recent call last):
  File "/home/fhenneke/.cache/bazel/_bazel_fhenneke/8c09f903aff88728a492063a5dce0a79/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles/_main/hello_world.py", line 6, in <module>
    import my_module
  File "/home/fhenneke/tmp/rules_python_1631_repro/my_module/__init__.py", line 6, in <module>
    r.Rlocation("_main/my_module/test.txt")
  File "/home/fhenneke/.cache/bazel/_bazel_fhenneke/8c09f903aff88728a492063a5dce0a79/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles/rules_python~0.27.1~pip~pip_38_bazel_runfiles/site-packages/runfiles/runfiles.py", line 141, in Rlocation
    source_repo = self.CurrentRepository(frame=2)
  File "/home/fhenneke/.cache/bazel/_bazel_fhenneke/8c09f903aff88728a492063a5dce0a79/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles/rules_python~0.27.1~pip~pip_38_bazel_runfiles/site-packages/runfiles/runfiles.py", line 215, in CurrentRepository
    raise ValueError(
ValueError: /home/fhenneke/tmp/rules_python_1631_repro/my_module/__init__.py does not lie under the runfiles root /home/fhenneke/.cache/bazel/_bazel_fhenneke/8c09f903aff88728a492063a5dce0a79/execroot/_main/bazel-out/k8-fastbuild/bin/hello_world.runfiles

@aignas @rickeylev Is this expected? I always assumed that rules_python would not add the source root to sys.path as that could result in non-hermetic behavior (imports resolving when they shouldn't). I could adapt the runfiles logic to this if this really is the expected behavior.

[rickeylev]

What version of Python is being used? This sounds like the "script dir is added to sys.path" behavior: https://docs.python.org/3/using/cmdline.html#cmdoption-P

Not doing that behavior requires Python 3.11+

[fmeum]

I was using Python 3.10. That makes sense.

@rickeylev I can look into handling this case in the runfiles library for compatibility with Python 3.10 and lower if those versions are meant to be supported.

[github-actions]

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

[github-actions]

This issue was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"