From 5929cb72aa01768e6352898b1a056ef678c81d90 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 10 Nov 2022 17:31:35 +0100 Subject: [PATCH] Stage repository mapping manifest as a root symlink (#16733) By adding the repository mapping manifest to runfiles as a root symlink, it is staged as `foo.runfiles/_repo_mapping` with all execution strategies. This includes sandboxed and remote execution, which previously did not stage the manifest at all. As a side effect, runfiles libraries can now find the repository mapping manifest via `rlocation("_repo_mapping")`. Fixes #16643 Work towards #16124 Closes #16652. PiperOrigin-RevId: 487532254 Change-Id: I9774b8930337c5967fce92a861cc0db71dea2f0f --- .../devtools/build/lib/analysis/Runfiles.java | 8 ++- .../build/lib/analysis/RunfilesSupport.java | 13 ++-- .../lib/analysis/SingleRunfilesSupplier.java | 31 ++++++-- .../lib/analysis/SourceManifestAction.java | 18 +++-- .../analysis/actions/SymlinkTreeAction.java | 25 ++++++- .../lib/analysis/test/TestActionBuilder.java | 1 + .../build/lib/exec/SymlinkTreeStrategy.java | 3 +- .../build/lib/rules/java/JavaBinary.java | 1 + .../analysis/SingleRunfilesSupplierTest.java | 27 +++++-- .../lib/analysis/actions/SpawnActionTest.java | 2 + .../actions/SymlinkTreeActionTest.java | 4 ++ .../lib/analysis/util/AnalysisTestUtil.java | 1 + .../lib/exec/SymlinkTreeStrategyTest.java | 2 + .../lib/rules/cpp/LtoBackendActionTest.java | 2 + ...arlarkRuleImplementationFunctionsTest.java | 2 +- src/test/py/bazel/bzlmod/bazel_module_test.py | 70 +++++++++++++------ 16 files changed, 161 insertions(+), 49 deletions(-) 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 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 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 getRunfilesInputs( - EventHandler eventHandler, Location location) { + EventHandler eventHandler, Location location, @Nullable Artifact repoMappingManifest) { ConflictChecker checker = new ConflictChecker(conflictPolicy, eventHandler, location); Map manifest = getSymlinksAsMap(checker); // Add artifacts (committed to inclusion on construction of runfiles). @@ -417,6 +420,9 @@ public Map 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> 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}. * *

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> 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> { + private final Runfiles runfiles; + @Nullable private final Artifact repoMappingManifest; private volatile SoftReference> 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 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> 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 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 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 computeInputs( boolean enableRunfiles, boolean skipRunfilesManifests, Runfiles runfiles, - Artifact inputManifest) { + Artifact inputManifest, + @Nullable Artifact repoMappingManifest) { NestedSetBuilder inputs = NestedSetBuilder.stableOrder(); if (!skipRunfilesManifests || !enableRunfiles || runfiles == null) { inputs.add(inputManifest); @@ -134,6 +143,9 @@ private static NestedSet 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/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/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 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); 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> mappings1 = underTest.getMappings(); Map> 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> mappingsOld = original.getMappings(); - Map> mappingsNew = overriden.getMappings(); + Map> 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 attributesToFlip) { PathFragment.create("a"), Runfiles.EMPTY, artifactA, + /* repoMappingManifest= */ null, /* buildRunfileLinks= */ false, /* runfileLinksEnabled= */ false)); } else { @@ -226,6 +227,7 @@ public Action generate(ImmutableSet 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 : 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()