bzlmod + build_python_zip breaks runfiles resolution

[coquelicot]

Description of the bug:

When building a py_binary with --build_python_zip and bzlmod enabled, the runfiles cannot be correctly resolved.
Things would work if I remove the --build_python_zip option or switch back to the old WORKSPACE style.
I suspect this has something to do with the way bzlmod handle repo mapping.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I have a very small example to reproduce the issue: https://github.com/coquelicot/broken-bzlmod-pyzip

Here's the log from my machine:

 » bazel run example
INFO: Build option --build_python_zip has changed, discarding analysis cache.
INFO: Analyzed target //:example (0 packages loaded, 539 targets configured).
INFO: Found 1 target...
Target //:example up-to-date:
  bazel-bin/example
INFO: Elapsed time: 0.280s, Critical Path: 0.01s
INFO: 4 processes: 4 internal.
INFO: Build completed successfully, 4 total actions
INFO: Running command line: bazel-bin/example
sample

 » bazel run --build_python_zip example
INFO: Build option --build_python_zip has changed, discarding analysis cache.
INFO: Analyzed target //:example (0 packages loaded, 539 targets configured).
INFO: Found 1 target...
Target //:example up-to-date:
  bazel-bin/example
  bazel-bin/example.zip
INFO: Elapsed time: 0.274s, Critical Path: 0.01s
INFO: 4 processes: 3 internal, 1 linux-sandbox.
INFO: Build completed successfully, 4 total actions
INFO: Running command line: bazel-bin/example
Traceback (most recent call last):
  File "/tmp/Bazel.runfiles_tuww8rqk/runfiles/_main/example.py", line 11, in <module>
    main()
  File "/tmp/Bazel.runfiles_tuww8rqk/runfiles/_main/example.py", line 6, in main
    with open(path, "r") as fin:
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/Bazel.runfiles_tuww8rqk/runfiles/pyzip/data.txt'

Which operating system are you running Bazel on?

linux

What is the output of bazel info release?

release 6.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

The way runfiles are implemented seems to be different with bzlmod: #16124

Any other information, logs, or outputs that you want to share?

No response

[fmeum]

Looks like the Python rules do not add symlinks and root_symlinks to the ZIP file, which causes the _repo_mapping file to be missing. I will try to fix this.

[fmeum]

#17942 is the easy part, but this is unfortunately complicated on a deeper level:

The _repo_mapping file is currently injected directly in https://cs.opensource.google/bazel/bazel/+/6f12510e39153e316f35ab0a1ec376ece7f1b0a9:src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java;l=421, which makes it invisible to the runfiles Starlark API.

We could instead add it in a way similar to https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java;l=125, but that would construct a new Runfiles object for every target.

An alternative could be to add the artifact to every Runfiles object directly upon construction (as long as Bzlmod is enabled) and then only register the generating action in RunfilesSupport. Edit: This doesn't work as those Runfiles objects wouldn't be mergeable anymore.

@Wyverald @lberki I could use your help here as I don't have a good grasp on the potential performance implications of these approaches.

[fmeum]

Looked into this some more. Even if RunfilesSupport adds the repo mapping artifact to root symlinks, the Python rules still wouldn't see it as it is added only after the rule implementation has run. Maybe we need to expose it on RuleContext?

Edit: Cleared up some confusion I had, which led me to this insight: Computing the repo mapping requires the runfiles, so it simply isn't known before the rule commits to its runfiles. Getting it in the context of the rule only seems to be necessary if the rule wants to build a runfiles structure manually for itself - if it did this for some other rule, that rule's repo mapping would be in its runfiles. So unless the Python rules can build the ZIP as part of a separate rule using a macro, maybe RepoMappingManifestAction would need to be exposed to Starlark so that users can build their own manifests for arbitrary runfiles objects?

[lberki]

Let me see if I understand this correctly: the current way Python rules build ZIP files is that they dissect a Runfiles object and put the path to artifact mapping on a command line. But repo mappings make it impossible to determine that mapping solely from the Runfiles object. Is that correct?

Shooting from the hip, with the current toolbox, I could imagine creating a "fake" Python binary which is then put on the inputs of an action and then that action does not execute it, but simply extracts its runfiles tree. Would that work?

[fmeum]

@lberki That's exactly correct. If forcing such rules to become macros that obtain the runfiles from an actual target is acceptable, then that is certainly the easiest solution.

@rickeylev Could the executable Python rules go through a separate target to generate the runfiles that are then packaged up into the executable ZIP file?

[lberki]

Is the limitation that makes a macro necessary that FilesToRunProvider can only be created once per rule and outside the control of Starlark code and that the only way one can currently add a runfiles tree to the inputs of an action is by using ctx.actions.run(tools=) ?