Repository: bazelbuild/bazel
Issue: Implement "Locating runfiles with Bzlmod" proposal
Labels: type: feature request
P2
team-ExternalDeps
area-Bzlmod

The ["Locating runfiles with Bzlmod" proposal](https://github.com/bazelbuild/proposals/blob/main/designs/2022-07-21-locating-runfiles-with-bzlmod.md) consists of the following individual steps:

* [x] ~Add the `RunfilesLibraryInfo` provider (#16125)~
* [x] Collect repository names and mappings of the transitive closure of a target (#16278)
* [x] Emit a repository mapping manifest for every executable (#16321 and https://github.com/bazelbuild/bazel/pull/16652)
* [x] Revert `RunfilesLibraryInfo`
* [x] Expose the current repository name in rules
  * [x] Bash (#16693)
  * [x] C++ (#16216)
  * [x] Java (#16534)
  * [x] Python (#16341)
* [x] Parse and use the repository mapping manifest in runfiles libraries (https://github.com/bazelbuild/bazel/issues/15029)
  * [x] Bash (#16693)
  * [x] C++ (#16623 and https://github.com/bazelbuild/bazel/pull/16701)
  * [x] Java (#16549)
  * [X] ~Python~ has been updated in rules_python
* [x] Use the Java runfiles library to make Stardoc work with repository mappings (https://github.com/bazelbuild/bazel/issues/14140)

Status of runfiles libraries in third-party rulesets:

* [x] rules_go (https://github.com/bazelbuild/rules_go/pull/3347)
* [ ] [rules_rust](https://github.com/bazelbuild/rules_rust/blob/main/tools/runfiles/runfiles.rs) (???)
* [ ] [rules_haskell](https://github.com/tweag/rules_haskell/blob/master/tools/runfiles/README.md) (???)

Issue comments:
@Wyverald Could you assign me and add the appropriate labels?
~~@comius Do you expect `java_binary` to be starlarkified in time for Bazel 6? That would simplify the changes to the Java rules.~~ Most likely no longer necessary, as the changes will apply to `buildjar` itself.
@Wyverald Oops, I missed the use of `Closes ...` in the PR description - we should use `Work towards ...` instead. Could you reopen? For some I lack the permission to do so.
It's the "To fix #NNN" line... GitHub trying to be clever here
The most likely future seems to be that we'll standardize on Python imports _not_ containing the repository name, in which case bzlmod is a no-op.

It's not an optimal solution because it requires an entry in `$PYTHONPATH` for each repository in the transitive closure of the Python binary, but it's the status quo and I'd much prefer thinking up a solution to that (e.g. binary-specific symlink trees that do not look like runfiles trees, dunno) than to standardize on having the repository names in imports because the concept of "repository" is specific to Bazel.

Put differently: the only way for bzlmod to cause trouble for Python is if we decide to require repository names in imports, which seems to be unlikely and it looks like the decision on that will take a while and I prefer not blocking either Bazel 6.0 or the removal of bzlmod from the experimental domain to avoid an unlikely failure mode. And should that happen, we can always paper over it with some sort of import hook.

/cc @rickeylev 
@oquenchil I'm thinking about whether we should use a macro to pass `BAZEL_CURRENT_REPOSITORY` to the `Rlocation` function automatically (e.g. do `#define Rlocation(path) RlocationReal(path, BAZEL_CURRENT_REPOSITORY)`). Do you think that would be too intrusive/magical?
Would users have to write that line or is it part of a file that is automatically generated?

I don't mind adding whatever will make it easier but if it gets too magical it should maybe be wrapped so that the details aren't visible to the user unless they want to look.
> Would users have to write that line or is it part of a file that is automatically generated?
> 
> I don't mind adding whatever will make it easier but if it gets too magical it should maybe be wrapped so that the details aren't visible to the user unless they want to look.

@oquenchil I would want to make that magic part of the runfiles library header so that users don't have to write it themselves.

What I am worried about is having an `Rlocation` macro mess with people's existing codebase since there doesn't seem to be a way to put it in a namespace or restrict it to member functions only.
Aren't we in control of the name of the macro? Can we rename it to something that would be very unlikely to clash with anything?
> Aren't we in control of the name of the macro? Can we rename it to something that would be very unlikely to clash with anything?

We can call it something else, we just won't be able to have existing C++ codebases use the new repo mapping-aware lookup automatically if we do so. But of course having to replace all `runfiles::create()` calls with something else won't be too bad.
If the macro name is Rlocation and that may start clashing with people's code in C++ files then I think it would be prudent to rename it if we decide to go with the macro.

I guess the call to make here is how much of a breaking change this is compared to the benefit of passing BAZEL_CURRENT_REPOSITORY automatically for users. I would say that the benefit is not too great but as far as I understood the use of Bazel runfiles in C++ code wasn't too wide-spread either?

>  But of course having to replace all runfiles::create() calls with something else won't be too bad.

How do you know? Is it based on search results from GH? If a release note would be enough for it not to be too disruptive then let's rename it.
I have experimented a bit more with APIs while working on the corresponding change for Java (https://github.com/bazelbuild/bazel/pull/16549).

What I now like more than the macro approach is to have users (optionally) pass in a canonical repository name when creating the `Runfiles` instance and default to that repo in `Rlocation` calls, while providing ways to override the default with an overload of `Rlocation` that accepts a repository name as a second parameter. In this way, users only have to pass in `BAZEL_CURRENT_REPOSITORY` only once in the common case, which should render a macro unnecessary.
Update: All changes relevant for Bazel 6.0.0 are now in review.
This is mostly done now, with only the bugfix https://github.com/bazelbuild/bazel/pull/16755 and a number of cherry-picks to 6.0.0 missing.
@Wyverald You can remove this from the "6.0.0 nice to have" milestone as the only remaining issue is not tied to a Bazel release (Stardoc).
Downgrading because the most important points have been addressed; the only outstanding issue is Stardoc, which is undergoing a rewrite currently by @tetromino.

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
index d9933d60b19561..00a4fe5f6a486f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
@@ -386,11 +386,14 @@ static Map<PathFragment, Artifact> filterListForObscuringSymlinks(
    *     normal source tree entries, or runfile conflicts. May be null, in which case obscuring
    *     symlinks are silently discarded, and conflicts are overwritten.
    * @param location Location for eventHandler warnings. Ignored if eventHandler is null.
+   * @param repoMappingManifest repository mapping manifest to add as a root symlink. This manifest
+   *     has to be added automatically for every executable and is thus not part of the Runfiles
+   *     advertised by a configured target.
    * @return Map<PathFragment, Artifact> path fragment to artifact, of normal source tree entries
    *     and elements that live outside the source tree. Null values represent empty input files.
    */
   public Map<PathFragment, Artifact> getRunfilesInputs(
-      EventHandler eventHandler, Location location) {
+      EventHandler eventHandler, Location location, @Nullable Artifact repoMappingManifest) {
     ConflictChecker checker = new ConflictChecker(conflictPolicy, eventHandler, location);
     Map<PathFragment, Artifact> manifest = getSymlinksAsMap(checker);
     // Add artifacts (committed to inclusion on construction of runfiles).
@@ -417,6 +420,9 @@ public Map<PathFragment, Artifact> getRunfilesInputs(
       checker = new ConflictChecker(ConflictPolicy.WARN, eventHandler, location);
     }
     builder.add(getRootSymlinksAsMap(checker), checker);
+    if (repoMappingManifest != null) {
+      checker.put(builder.manifest, PathFragment.create("_repo_mapping"), repoMappingManifest);
+    }
     return builder.build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index 189cb882fe15d7..3772cf7cb700da 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -132,18 +132,20 @@ private static RunfilesSupport create(
     }
     Preconditions.checkState(!runfiles.isEmpty());
 
+    Artifact repoMappingManifest =
+        createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable);
+
     Artifact runfilesInputManifest;
     Artifact runfilesManifest;
     if (createManifest) {
       runfilesInputManifest = createRunfilesInputManifestArtifact(ruleContext, owningExecutable);
       runfilesManifest =
-          createRunfilesAction(ruleContext, runfiles, buildRunfileLinks, runfilesInputManifest);
+          createRunfilesAction(
+              ruleContext, runfiles, buildRunfileLinks, runfilesInputManifest, repoMappingManifest);
     } else {
       runfilesInputManifest = null;
       runfilesManifest = null;
     }
-    Artifact repoMappingManifest =
-        createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable);
     Artifact runfilesMiddleman =
         createRunfilesMiddleman(
             ruleContext, owningExecutable, runfiles, runfilesManifest, repoMappingManifest);
@@ -387,7 +389,8 @@ private static Artifact createRunfilesAction(
       ActionConstructionContext context,
       Runfiles runfiles,
       boolean createSymlinks,
-      Artifact inputManifest) {
+      Artifact inputManifest,
+      @Nullable Artifact repoMappingManifest) {
     // Compute the names of the runfiles directory and its MANIFEST file.
     context
         .getAnalysisEnvironment()
@@ -397,6 +400,7 @@ private static Artifact createRunfilesAction(
                 context.getActionOwner(),
                 inputManifest,
                 runfiles,
+                repoMappingManifest,
                 context.getConfiguration().remotableSourceManifestActions()));
 
     if (!createSymlinks) {
@@ -423,6 +427,7 @@ private static Artifact createRunfilesAction(
                 inputManifest,
                 runfiles,
                 outputManifest,
+                repoMappingManifest,
                 /*filesetRoot=*/ null));
     return outputManifest;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java
index 126c0e98548430..379c875b3d2141 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java
@@ -34,10 +34,12 @@
 /** {@link RunfilesSupplier} implementation wrapping a single {@link Runfiles} directory mapping. */
 @AutoCodec
 public final class SingleRunfilesSupplier implements RunfilesSupplier {
+
   private final PathFragment runfilesDir;
   private final Runfiles runfiles;
   private final Supplier<Map<PathFragment, Artifact>> runfilesInputs;
   @Nullable private final Artifact manifest;
+  @Nullable private final Artifact repoMappingManifest;
   private final boolean buildRunfileLinks;
   private final boolean runfileLinksEnabled;
 
@@ -50,14 +52,15 @@ public static SingleRunfilesSupplier create(RunfilesSupport runfilesSupport) {
         runfilesSupport.getRunfiles(),
         /*runfilesCachingEnabled=*/ false,
         /*manifest=*/ null,
+        runfilesSupport.getRepoMappingManifest(),
         runfilesSupport.isBuildRunfileLinks(),
         runfilesSupport.isRunfilesEnabled());
   }
 
   /**
    * Same as {@link SingleRunfilesSupplier#SingleRunfilesSupplier(PathFragment, Runfiles, Artifact,
-   * boolean, boolean)}, except adds caching for {@linkplain Runfiles#getRunfilesInputs runfiles
-   * inputs}.
+   * Artifact, boolean, boolean)}, except adds caching for {@linkplain Runfiles#getRunfilesInputs
+   * runfiles inputs}.
    *
    * <p>The runfiles inputs are computed lazily and softly cached. Caching is shared across
    * instances created via {@link #withOverriddenRunfilesDir}.
@@ -65,6 +68,7 @@ public static SingleRunfilesSupplier create(RunfilesSupport runfilesSupport) {
   public static SingleRunfilesSupplier createCaching(
       PathFragment runfilesDir,
       Runfiles runfiles,
+      @Nullable Artifact repoMappingManifest,
       boolean buildRunfileLinks,
       boolean runfileLinksEnabled) {
     return new SingleRunfilesSupplier(
@@ -72,6 +76,7 @@ public static SingleRunfilesSupplier createCaching(
         runfiles,
         /*runfilesCachingEnabled=*/ true,
         /*manifest=*/ null,
+        repoMappingManifest,
         buildRunfileLinks,
         runfileLinksEnabled);
   }
@@ -92,6 +97,7 @@ public SingleRunfilesSupplier(
       PathFragment runfilesDir,
       Runfiles runfiles,
       @Nullable Artifact manifest,
+      @Nullable Artifact repoMappingManifest,
       boolean buildRunfileLinks,
       boolean runfileLinksEnabled) {
     this(
@@ -99,6 +105,7 @@ public SingleRunfilesSupplier(
         runfiles,
         /*runfilesCachingEnabled=*/ false,
         manifest,
+        repoMappingManifest,
         buildRunfileLinks,
         runfileLinksEnabled);
   }
@@ -108,15 +115,19 @@ private SingleRunfilesSupplier(
       Runfiles runfiles,
       boolean runfilesCachingEnabled,
       @Nullable Artifact manifest,
+      @Nullable Artifact repoMappingManifest,
       boolean buildRunfileLinks,
       boolean runfileLinksEnabled) {
     this(
         runfilesDir,
         runfiles,
         runfilesCachingEnabled
-            ? new RunfilesCacher(runfiles)
-            : () -> runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null),
+            ? new RunfilesCacher(runfiles, repoMappingManifest)
+            : () ->
+                runfiles.getRunfilesInputs(
+                    /*eventHandler=*/ null, /*location=*/ null, repoMappingManifest),
         manifest,
+        repoMappingManifest,
         buildRunfileLinks,
         runfileLinksEnabled);
   }
@@ -126,6 +137,7 @@ private SingleRunfilesSupplier(
       Runfiles runfiles,
       Supplier<Map<PathFragment, Artifact>> runfilesInputs,
       @Nullable Artifact manifest,
+      @Nullable Artifact repoMappingManifest,
       boolean buildRunfileLinks,
       boolean runfileLinksEnabled) {
     checkArgument(!runfilesDir.isAbsolute());
@@ -133,6 +145,7 @@ private SingleRunfilesSupplier(
     this.runfiles = checkNotNull(runfiles);
     this.runfilesInputs = checkNotNull(runfilesInputs);
     this.manifest = manifest;
+    this.repoMappingManifest = repoMappingManifest;
     this.buildRunfileLinks = buildRunfileLinks;
     this.runfileLinksEnabled = runfileLinksEnabled;
   }
@@ -199,17 +212,21 @@ public SingleRunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfiles
             runfiles,
             runfilesInputs,
             manifest,
+            repoMappingManifest,
             buildRunfileLinks,
             runfileLinksEnabled);
   }
 
   /** Softly caches the result of {@link Runfiles#getRunfilesInputs}. */
   private static final class RunfilesCacher implements Supplier<Map<PathFragment, Artifact>> {
+
     private final Runfiles runfiles;
+    @Nullable private final Artifact repoMappingManifest;
     private volatile SoftReference<Map<PathFragment, Artifact>> ref = new SoftReference<>(null);
 
-    RunfilesCacher(Runfiles runfiles) {
+    RunfilesCacher(Runfiles runfiles, @Nullable Artifact repoMappingManifest) {
       this.runfiles = runfiles;
+      this.repoMappingManifest = repoMappingManifest;
     }
 
     @Override
@@ -221,7 +238,9 @@ public Map<PathFragment, Artifact> get() {
       synchronized (this) {
         result = ref.get();
         if (result == null) {
-          result = runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null);
+          result =
+              runfiles.getRunfilesInputs(
+                  /*eventHandler=*/ null, /*location=*/ null, repoMappingManifest);
           ref = new SoftReference<>(result);
         }
       }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
index 8ca38d37c50658..6feb2b91321df8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
@@ -61,7 +61,7 @@ public final class SourceManifestAction extends AbstractFileWriteAction {
 
   private static final Comparator<Map.Entry<PathFragment, Artifact>> ENTRY_COMPARATOR =
       (path1, path2) -> path1.getKey().getPathString().compareTo(path2.getKey().getPathString());
-
+  private final Artifact repoMappingManifest;
   /**
    * Interface for defining manifest formatting and reporting specifics. Implementations must be
    * immutable.
@@ -118,7 +118,7 @@ void writeEntry(
   @VisibleForTesting
   SourceManifestAction(
       ManifestWriter manifestWriter, ActionOwner owner, Artifact primaryOutput, Runfiles runfiles) {
-    this(manifestWriter, owner, primaryOutput, runfiles, /*remotableSourceManifestActions=*/ false);
+    this(manifestWriter, owner, primaryOutput, runfiles, null, false);
   }
 
   /**
@@ -129,17 +129,20 @@ void writeEntry(
    * @param owner the action owner
    * @param primaryOutput the file to which to write the manifest
    * @param runfiles runfiles
+   * @param repoMappingManifest the repository mapping manifest for runfiles
    */
   public SourceManifestAction(
       ManifestWriter manifestWriter,
       ActionOwner owner,
       Artifact primaryOutput,
       Runfiles runfiles,
+      @Nullable Artifact repoMappingManifest,
       boolean remotableSourceManifestActions) {
     // The real set of inputs is computed in #getInputs().
     super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), primaryOutput, false);
     this.manifestWriter = manifestWriter;
     this.runfiles = runfiles;
+    this.repoMappingManifest = repoMappingManifest;
     this.remotableSourceManifestActions = remotableSourceManifestActions;
   }
 
@@ -180,7 +183,9 @@ public synchronized NestedSet<Artifact> getInputs() {
   @VisibleForTesting
   public void writeOutputFile(OutputStream out, @Nullable EventHandler eventHandler)
       throws IOException {
-    writeFile(out, runfiles.getRunfilesInputs(eventHandler, getOwner().getLocation()));
+    writeFile(
+        out,
+        runfiles.getRunfilesInputs(eventHandler, getOwner().getLocation(), repoMappingManifest));
   }
 
   /**
@@ -202,7 +207,8 @@ public String getStarlarkContent() throws IOException {
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
     final Map<PathFragment, Artifact> runfilesInputs =
-        runfiles.getRunfilesInputs(ctx.getEventHandler(), getOwner().getLocation());
+        runfiles.getRunfilesInputs(
+            ctx.getEventHandler(), getOwner().getLocation(), repoMappingManifest);
     return out -> writeFile(out, runfilesInputs);
   }
 
@@ -247,6 +253,10 @@ protected void computeKey(
     fp.addString(GUID);
     fp.addBoolean(remotableSourceManifestActions);
     runfiles.fingerprint(fp);
+    fp.addBoolean(repoMappingManifest != null);
+    if (repoMappingManifest != null) {
+      fp.addPath(repoMappingManifest.getExecPath());
+    }
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
index 14a06ca1511f5c..4a819506532e24 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeAction.java
@@ -49,6 +49,7 @@ public final class SymlinkTreeAction extends AbstractAction {
   private final boolean enableRunfiles;
   private final boolean inprocessSymlinkCreation;
   private final boolean skipRunfilesManifests;
+  private final Artifact repoMappingManifest;
 
   /**
    * Creates SymlinkTreeAction instance.
@@ -59,6 +60,7 @@ public final class SymlinkTreeAction extends AbstractAction {
    * @param runfiles the input runfiles
    * @param outputManifest the generated symlink tree manifest (must have "MANIFEST" base name).
    *     Symlink tree root will be set to the artifact's parent directory.
+   * @param repoMappingManifest the repository mapping manifest
    * @param filesetRoot non-null if this is a fileset symlink tree
    */
   public SymlinkTreeAction(
@@ -67,12 +69,14 @@ public SymlinkTreeAction(
       Artifact inputManifest,
       @Nullable Runfiles runfiles,
       Artifact outputManifest,
+      @Nullable Artifact repoMappingManifest,
       String filesetRoot) {
     this(
         owner,
         inputManifest,
         runfiles,
         outputManifest,
+        repoMappingManifest,
         filesetRoot,
         config.getActionEnvironment(),
         config.runfilesEnabled(),
@@ -90,6 +94,7 @@ public SymlinkTreeAction(
    * @param runfiles the input runfiles
    * @param outputManifest the generated symlink tree manifest (must have "MANIFEST" base name).
    *     Symlink tree root will be set to the artifact's parent directory.
+   * @param repoMappingManifest the repository mapping manifest
    * @param filesetRoot non-null if this is a fileset symlink tree,
    */
   public SymlinkTreeAction(
@@ -97,6 +102,7 @@ public SymlinkTreeAction(
       Artifact inputManifest,
       @Nullable Runfiles runfiles,
       Artifact outputManifest,
+      @Nullable Artifact repoMappingManifest,
       @Nullable String filesetRoot,
       ActionEnvironment env,
       boolean enableRunfiles,
@@ -104,7 +110,8 @@ public SymlinkTreeAction(
       boolean skipRunfilesManifests) {
     super(
         owner,
-        computeInputs(enableRunfiles, skipRunfilesManifests, runfiles, inputManifest),
+        computeInputs(
+            enableRunfiles, skipRunfilesManifests, runfiles, inputManifest, repoMappingManifest),
         ImmutableSet.of(outputManifest),
         env);
     Preconditions.checkArgument(outputManifest.getPath().getBaseName().equals("MANIFEST"));
@@ -118,13 +125,15 @@ public SymlinkTreeAction(
     this.inprocessSymlinkCreation = inprocessSymlinkCreation;
     this.skipRunfilesManifests = skipRunfilesManifests && enableRunfiles && (filesetRoot == null);
     this.inputManifest = this.skipRunfilesManifests ? null : inputManifest;
+    this.repoMappingManifest = repoMappingManifest;
   }
 
   private static NestedSet<Artifact> computeInputs(
       boolean enableRunfiles,
       boolean skipRunfilesManifests,
       Runfiles runfiles,
-      Artifact inputManifest) {
+      Artifact inputManifest,
+      @Nullable Artifact repoMappingManifest) {
     NestedSetBuilder<Artifact> inputs = NestedSetBuilder.<Artifact>stableOrder();
     if (!skipRunfilesManifests || !enableRunfiles || runfiles == null) {
       inputs.add(inputManifest);
@@ -134,6 +143,9 @@ private static NestedSet<Artifact> computeInputs(
     // existing, so directory or file links can be made as appropriate.
     if (enableRunfiles && runfiles != null && OS.getCurrent() == OS.WINDOWS) {
       inputs.addTransitive(runfiles.getAllArtifacts());
+      if (repoMappingManifest != null) {
+        inputs.add(repoMappingManifest);
+      }
     }
     return inputs.build();
   }
@@ -151,6 +163,11 @@ public Artifact getOutputManifest() {
     return outputManifest;
   }
 
+  @Nullable
+  public Artifact getRepoMappingManifest() {
+    return repoMappingManifest;
+  }
+
   public boolean isFilesetTree() {
     return filesetRoot != null;
   }
@@ -201,6 +218,10 @@ protected void computeKey(
     if (runfiles != null) {
       runfiles.fingerprint(fp);
     }
+    fp.addBoolean(repoMappingManifest != null);
+    if (repoMappingManifest != null) {
+      fp.addPath(repoMappingManifest.getExecPath());
+    }
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
index fc5dc61676466f..5d2e4371f1e177 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java
@@ -151,7 +151,8 @@ private static Map<PathFragment, Artifact> runfilesToMap(
         .getRunfiles()
         .getRunfilesInputs(
             action.getInputManifest() == null ? actionExecutionContext.getEventHandler() : null,
-            action.getOwner().getLocation());
+            action.getOwner().getLocation(),
+            action.getRepoMappingManifest());
   }
 
   private static void createOutput(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index 4734c3a1754af8..df8b8aaa0b1225 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -495,6 +495,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
                   // This matches the code below in collectDefaultRunfiles.
                   .addTransitiveArtifactsWrappedInStableOrder(common.getRuntimeClasspath())
                   .build(),
+              null,
               true));
       filesBuilder.add(runtimeClasspathArtifact);
 


Test patch:
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
index 726b433670c331..cf908ce46c1651 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -379,6 +379,7 @@ private TestParams createTestAction(int shards)
           SingleRunfilesSupplier.createCaching(
               runfilesSupport.getRunfilesDirectoryExecPath(),
               runfilesSupport.getRunfiles(),
+              runfilesSupport.getRepoMappingManifest(),
               runfilesSupport.isBuildRunfileLinks(),
               runfilesSupport.isRunfilesEnabled());
     } else {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java b/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java
index ee17341d27ce32..fc0a713e1495af 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java
@@ -56,6 +56,7 @@ public void testGetArtifactsWithSingleMapping() {
             PathFragment.create("notimportant"),
             mkRunfiles(artifacts),
             /*manifest=*/ null,
+            /*repoMappingManifest=*/ null,
             /*buildRunfileLinks=*/ false,
             /*runfileLinksEnabled=*/ false);
 
@@ -69,6 +70,7 @@ public void testGetManifestsWhenNone() {
             PathFragment.create("ignored"),
             Runfiles.EMPTY,
             /*manifest=*/ null,
+            /*repoMappingManifest=*/ null,
             /*buildRunfileLinks=*/ false,
             /*runfileLinksEnabled=*/ false);
     assertThat(underTest.getManifests()).isEmpty();
@@ -82,6 +84,7 @@ public void testGetManifestsWhenSupplied() {
             PathFragment.create("ignored"),
             Runfiles.EMPTY,
             manifest,
+            /*repoMappingManifest=*/ null,
             /*buildRunfileLinks=*/ false,
             /*runfileLinksEnabled=*/ false);
     assertThat(underTest.getManifests()).containsExactly(manifest);
@@ -94,6 +97,7 @@ public void withOverriddenRunfilesDir() {
             PathFragment.create("old"),
             Runfiles.EMPTY,
             ActionsTestUtil.createArtifact(rootDir, "manifest"),
+            /*repoMappingManifest=*/ null,
             /*buildRunfileLinks=*/ false,
             /*runfileLinksEnabled=*/ false);
     PathFragment newDir = PathFragment.create("new");
@@ -115,6 +119,7 @@ public void withOverriddenRunfilesDir_noChange_sameObject() {
             dir,
             Runfiles.EMPTY,
             ActionsTestUtil.createArtifact(rootDir, "manifest"),
+            /*repoMappingManifest=*/ null,
             /*buildRunfileLinks=*/ false,
             /*runfileLinksEnabled=*/ false);
     assertThat(original.withOverriddenRunfilesDir(dir)).isSameInstanceAs(original);
@@ -126,12 +131,16 @@ public void cachedMappings() {
     Runfiles runfiles = mkRunfiles(mkArtifacts("a", "b", "c"));
     SingleRunfilesSupplier underTest =
         SingleRunfilesSupplier.createCaching(
-            dir, runfiles, /*buildRunfileLinks=*/ false, /*runfileLinksEnabled=*/ false);
+            dir,
+            runfiles,
+            /*repoMappingManifest=*/ null,
+            /*buildRunfileLinks=*/ false,
+            /*runfileLinksEnabled=*/ false);
 
     Map<PathFragment, Map<PathFragment, Artifact>> mappings1 = underTest.getMappings();
     Map<PathFragment, Map<PathFragment, Artifact>> mappings2 = underTest.getMappings();
 
-    assertThat(mappings1).containsExactly(dir, runfiles.getRunfilesInputs(null, null));
+    assertThat(mappings1).containsExactly(dir, runfiles.getRunfilesInputs(null, null, null));
     assertThat(mappings1).isEqualTo(mappings2);
     assertThat(mappings1.get(dir)).isSameInstanceAs(mappings2.get(dir));
   }
@@ -143,14 +152,18 @@ public void cachedMappings_sharedAcrossDirOverrides() {
     Runfiles runfiles = mkRunfiles(mkArtifacts("a", "b", "c"));
     SingleRunfilesSupplier original =
         SingleRunfilesSupplier.createCaching(
-            oldDir, runfiles, /*buildRunfileLinks=*/ false, /*runfileLinksEnabled=*/ false);
-    SingleRunfilesSupplier overriden = original.withOverriddenRunfilesDir(newDir);
+            oldDir,
+            runfiles,
+            /*repoMappingManifest=*/ null,
+            /*buildRunfileLinks=*/ false,
+            /*runfileLinksEnabled=*/ false);
+    SingleRunfilesSupplier overridden = original.withOverriddenRunfilesDir(newDir);
 
     Map<PathFragment, Map<PathFragment, Artifact>> mappingsOld = original.getMappings();
-    Map<PathFragment, Map<PathFragment, Artifact>> mappingsNew = overriden.getMappings();
+    Map<PathFragment, Map<PathFragment, Artifact>> mappingsNew = overridden.getMappings();
 
-    assertThat(mappingsOld).containsExactly(oldDir, runfiles.getRunfilesInputs(null, null));
-    assertThat(mappingsNew).containsExactly(newDir, runfiles.getRunfilesInputs(null, null));
+    assertThat(mappingsOld).containsExactly(oldDir, runfiles.getRunfilesInputs(null, null, null));
+    assertThat(mappingsNew).containsExactly(newDir, runfiles.getRunfilesInputs(null, null, null));
     assertThat(mappingsOld.get(newDir)).isSameInstanceAs(mappingsNew.get(oldDir));
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
index 49e3aba3e4c9e9..31a92732c2fb4a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
@@ -409,6 +409,7 @@ public void testInputManifestsRemovedIfSupplied() throws Exception {
                     PathFragment.create("destination"),
                     Runfiles.EMPTY,
                     manifest,
+                    /* repoMappingManifest= */ null,
                     /* buildRunfileLinks= */ false,
                     /* runfileLinksEnabled= */ false))
             .addOutput(getBinArtifactWithNoOwner("output"))
@@ -628,6 +629,7 @@ private static RunfilesSupplier runfilesSupplier(Artifact manifest, PathFragment
         dir,
         Runfiles.EMPTY,
         manifest,
+        /* repoMappingManifest= */ null,
         /* buildRunfileLinks= */ false,
         /* runfileLinksEnabled= */ false);
   }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java
index b849cf8733acbf..6ce4bcc2771442 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkTreeActionTest.java
@@ -70,6 +70,7 @@ public void testComputeKey() throws Exception {
                         ? new Runfiles.Builder("TESTING", false).addArtifact(runfile).build()
                         : new Runfiles.Builder("TESTING", false).addArtifact(runfile2).build(),
                     outputManifest,
+                    /*repoMappingManifest=*/ null,
                     /*filesetRoot=*/ null,
                     createActionEnvironment(
                         attributesToFlip.contains(RunfilesActionAttributes.FIXED_ENVIRONMENT),
@@ -85,6 +86,7 @@ public void testComputeKey() throws Exception {
                     inputManifest,
                     /*runfiles=*/ null,
                     outputManifest,
+                    /*repoMappingManifest=*/ null,
                     /*filesetRoot=*/ "root",
                     createActionEnvironment(
                         attributesToFlip.contains(FilesetActionAttributes.FIXED_ENVIRONMENT),
@@ -103,6 +105,7 @@ public void testComputeKey() throws Exception {
                         ? new Runfiles.Builder("TESTING", false).addArtifact(runfile).build()
                         : new Runfiles.Builder("TESTING", false).addArtifact(runfile2).build(),
                     outputManifest,
+                    /*repoMappingManifest=*/ null,
                     /*filesetRoot=*/ null,
                     createActionEnvironment(
                         attributesToFlip.contains(SkipManifestAttributes.FIXED_ENVIRONMENT),
@@ -131,6 +134,7 @@ public void testNullRunfilesThrows() {
                 inputManifest,
                 /*runfiles=*/ null,
                 outputManifest,
+                /*repoMappingManifest=*/ null,
                 /*filesetRoot=*/ null,
                 createActionEnvironment(false, false),
                 /*enableRunfiles=*/ true,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
index 53b05d891bef9c..12fbcb55ec44f8 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
@@ -562,6 +562,7 @@ public static RunfilesSupplier createRunfilesSupplier(
         runfilesDir,
         runfiles,
         /*manifest=*/ null,
+        /*repoMappingManifest=*/ null,
         /*buildRunfileLinks=*/ false,
         /*runfileLinksEnabled=*/ false);
   }
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
index 839a4bb64dd615..ad7a7a5e8d2687 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategyTest.java
@@ -92,6 +92,7 @@ public void outputServiceInteraction() throws Exception {
             inputManifest,
             runfiles,
             outputManifest,
+            /*repoMappingManifest=*/ null,
             /*filesetRoot=*/ null,
             ActionEnvironment.EMPTY,
             /*enableRunfiles=*/ true,
@@ -138,6 +139,7 @@ public void inprocessSymlinkCreation() throws Exception {
             inputManifest,
             runfiles,
             outputManifest,
+            /*repoMappingManifest=*/ null,
             /*filesetRoot=*/ null,
             ActionEnvironment.EMPTY,
             /*enableRunfiles=*/ true,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
index ed7d6da4fb9d0a..ec40c1f7e85a51 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
@@ -218,6 +218,7 @@ public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) {
                       PathFragment.create("a"),
                       Runfiles.EMPTY,
                       artifactA,
+                      /* repoMappingManifest= */ null,
                       /* buildRunfileLinks= */ false,
                       /* runfileLinksEnabled= */ false));
             } else {
@@ -226,6 +227,7 @@ public Action generate(ImmutableSet<KeyAttributes> attributesToFlip) {
                       PathFragment.create("a"),
                       Runfiles.EMPTY,
                       artifactB,
+                      /* repoMappingManifest= */ null,
                       /* buildRunfileLinks= */ false,
                       /* runfileLinksEnabled= */ false));
             }
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
index 3ecf4c1950728e..a04a2dc31a3abb 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
@@ -1033,7 +1033,7 @@ public void testRunfilesSymlinkConflict() throws Exception {
             "  symlinks = {'sym1': ruleContext.files.srcs[1]})");
     Runfiles runfiles = (Runfiles) result;
     reporter.removeHandler(failFastHandler); // So it doesn't throw an exception.
-    runfiles.getRunfilesInputs(reporter, null);
+    var unused = runfiles.getRunfilesInputs(reporter, null, null);
     assertContainsEvent("ERROR <no location>: overwrote runfile");
   }
 
diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py
index 5312a26de2e708..301477560bb224 100644
--- a/src/test/py/bazel/bzlmod/bazel_module_test.py
+++ b/src/test/py/bazel/bzlmod/bazel_module_test.py
@@ -583,27 +583,29 @@ def testRunfilesRepoMappingManifest(self):
     self.main_registry.setModuleBasePath('projects')
     projects_dir = self.main_registry.projects
 
-    # Set up a "bare_rule" module that contains the "bare_binary" rule which
+    # Set up a "bare_rule" module that contains the "bare_test" rule which
     # passes runfiles along
     self.main_registry.createLocalPathModule('bare_rule', '1.0', 'bare_rule')
     projects_dir.joinpath('bare_rule').mkdir(exist_ok=True)
     scratchFile(projects_dir.joinpath('bare_rule', 'WORKSPACE'))
     scratchFile(projects_dir.joinpath('bare_rule', 'BUILD'))
+    # The working directory of a test is the subdirectory of the runfiles
+    # directory corresponding to the main repository.
     scratchFile(
         projects_dir.joinpath('bare_rule', 'defs.bzl'), [
-            'def _bare_binary_impl(ctx):',
+            'def _bare_test_impl(ctx):',
             '  exe = ctx.actions.declare_file(ctx.label.name)',
             '  ctx.actions.write(exe,',
-            '    "#/bin/bash\\nif [[ ! -f \\"$0.repo_mapping\\" ]]; then\\necho >&2 \\"ERROR: cannot find repo mapping manifest file\\"\\nexit 1\\nfi",',
+            '    "#/bin/bash\\nif [[ ! -f ../_repo_mapping || ! -s ../_repo_mapping ]]; then\\necho >&2 \\"ERROR: cannot find repo mapping manifest file\\"\\nexit 1\\nfi",',
             '    True)',
             '  runfiles = ctx.runfiles(files=ctx.files.data)',
             '  for data in ctx.attr.data:',
             '    runfiles = runfiles.merge(data[DefaultInfo].default_runfiles)',
             '  return DefaultInfo(files=depset(direct=[exe]), executable=exe, runfiles=runfiles)',
-            'bare_binary=rule(',
-            '  implementation=_bare_binary_impl,',
+            'bare_test=rule(',
+            '  implementation=_bare_test_impl,',
             '  attrs={"data":attr.label_list(allow_files=True)},',
-            '  executable=True,',
+            '  test=True,',
             ')',
         ])
 
@@ -617,8 +619,8 @@ def testRunfilesRepoMappingManifest(self):
     self.ScratchFile('WORKSPACE')
     self.ScratchFile('WORKSPACE.bzlmod', ['workspace(name="me_ws")'])
     self.ScratchFile('BUILD', [
-        'load("@bare_rule//:defs.bzl", "bare_binary")',
-        'bare_binary(name="me",data=["@foo"])',
+        'load("@bare_rule//:defs.bzl", "bare_test")',
+        'bare_test(name="me",data=["@foo"])',
     ])
     self.main_registry.createLocalPathModule('foo', '1.0', 'foo', {
         'quux': '1.0',
@@ -633,16 +635,16 @@ def testRunfilesRepoMappingManifest(self):
     self.main_registry.createLocalPathModule('quux', '2.0', 'quux2',
                                              {'bare_rule': '1.0'})
     for dir_name, build_file in [
-        ('foo', 'bare_binary(name="foo",data=["@quux"])'),
-        ('bar', 'bare_binary(name="bar",data=["@quux"])'),
-        ('quux1', 'bare_binary(name="quux")'),
-        ('quux2', 'bare_binary(name="quux")'),
+        ('foo', 'bare_test(name="foo",data=["@quux"])'),
+        ('bar', 'bare_test(name="bar",data=["@quux"])'),
+        ('quux1', 'bare_test(name="quux")'),
+        ('quux2', 'bare_test(name="quux")'),
     ]:
       projects_dir.joinpath(dir_name).mkdir(exist_ok=True)
       scratchFile(projects_dir.joinpath(dir_name, 'WORKSPACE'))
       scratchFile(
           projects_dir.joinpath(dir_name, 'BUILD'), [
-              'load("@bare_rule//:defs.bzl", "bare_binary")',
+              'load("@bare_rule//:defs.bzl", "bare_test")',
               'package(default_visibility=["//visibility:public"])',
               build_file,
           ])
@@ -650,25 +652,47 @@ def testRunfilesRepoMappingManifest(self):
     # We use a shell script to check that the binary itself can see the repo
     # mapping manifest. This obviously doesn't work on Windows, so we just build
     # the target. TODO(wyv): make this work on Windows by using Batch.
-    bazel_command = 'build' if self.IsWindows() else 'run'
+    # On Linux and macOS, the script is executed in the sandbox, so we verify
+    # that the repository mapping is present in it.
+    bazel_command = 'build' if self.IsWindows() else 'test'
 
     # Finally we get to build stuff!
-    self.RunBazel([bazel_command, '//:me'], allow_failure=False)
-    with open(self.Path('bazel-bin/me.repo_mapping'), 'r') as f:
-      self.assertEqual(
-          f.read().strip(), """,foo,foo~1.0
+    exit_code, stderr, stdout = self.RunBazel(
+        [bazel_command, '//:me', '--test_output=errors'], allow_failure=True)
+    self.AssertExitCode(0, exit_code, stderr, stdout)
+
+    paths = ['bazel-bin/me.repo_mapping']
+    if not self.IsWindows():
+      paths.append('bazel-bin/me.runfiles/_repo_mapping')
+    for path in paths:
+      with open(self.Path(path), 'r') as f:
+        self.assertEqual(
+            f.read().strip(), """,foo,foo~1.0
 ,me,_main
 ,me_ws,_main
 foo~1.0,foo,foo~1.0
 foo~1.0,quux,quux~2.0
 quux~2.0,quux,quux~2.0""")
-    self.RunBazel([bazel_command, '@bar//:bar'], allow_failure=False)
-    with open(self.Path('bazel-bin/external/bar~2.0/bar.repo_mapping'),
-              'r') as f:
-      self.assertEqual(
-          f.read().strip(), """bar~2.0,bar,bar~2.0
+    with open(self.Path('bazel-bin/me.runfiles_manifest')) as f:
+      self.assertIn('_repo_mapping ', f.read())
+
+    exit_code, stderr, stdout = self.RunBazel(
+        [bazel_command, '@bar//:bar', '--test_output=errors'],
+        allow_failure=True)
+    self.AssertExitCode(0, exit_code, stderr, stdout)
+
+    paths = ['bazel-bin/external/bar~2.0/bar.repo_mapping']
+    if not self.IsWindows():
+      paths.append('bazel-bin/external/bar~2.0/bar.runfiles/_repo_mapping')
+    for path in paths:
+      with open(self.Path(path), 'r') as f:
+        self.assertEqual(
+            f.read().strip(), """bar~2.0,bar,bar~2.0
 bar~2.0,quux,quux~2.0
 quux~2.0,quux,quux~2.0""")
+    with open(
+        self.Path('bazel-bin/external/bar~2.0/bar.runfiles_manifest')) as f:
+      self.assertIn('_repo_mapping ', f.read())
 
 if __name__ == '__main__':
   unittest.main()