diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 4419d2f3effb81..25b3698457a432 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -157,6 +157,7 @@ java_library( ":thread_state_receiver", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", + "//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry", "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/bugreport", "//src/main/java/com/google/devtools/build/lib/buildeventstream", @@ -498,6 +499,7 @@ java_library( deps = [ ":artifacts", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", + "//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/net/starlark/java/eval", diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java index 7a6823af5ce3e0..1812194a91aaa1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.Map; import javax.annotation.Nullable; @@ -119,4 +120,13 @@ public boolean isBuildRunfileLinks(PathFragment runfilesDir) { public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) { throw new UnsupportedOperationException(); } + + @Override + public Map getRunfilesTreesForLogging() { + Map result = new LinkedHashMap<>(); + for (RunfilesSupplier supplier : suppliers) { + result.putAll(supplier.getRunfilesTreesForLogging()); + } + return result; + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java index 7ba71666df9740..26ba42c35eaafa 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java @@ -63,4 +63,9 @@ public boolean isBuildRunfileLinks(PathFragment runfilesDir) { public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) { throw new UnsupportedOperationException(); } + + @Override + public Map getRunfilesTreesForLogging() { + return ImmutableMap.of(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java index a57e1452365326..39caa060e3eda6 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/RunfilesSupplier.java @@ -16,6 +16,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.analysis.SymlinkEntry; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.vfs.PathFragment; @@ -66,4 +67,44 @@ public interface RunfilesSupplier extends StarlarkValue { * #getRunfilesDirs} returns a set of size 1. */ RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir); + + /** + * Returns information about the runfiles trees in this supplier for logging purposes. + * + *

The field names are chosen to minimize the diff with the actual RunfilesTree class in Bazel + * 8, which simplifies cherry-picks. + * + * @param getArtifactsAtCanonicalLocationsForLogging Returns artifacts the runfiles tree contain + * symlinks to at their canonical locations. + *

This does not include artifacts that only the symlinks and root symlinks point + * to. + * @param getEmptyFilenamesForLogging Returns the set of names of implicit empty files to + * materialize. + *

If this runfiles tree does not implicitly add empty files, implementations should have a + * dedicated fast path that returns an empty set without traversing the tree. + * @param getSymlinksForLogging Returns the set of custom symlink entries. + * @param getRootSymlinksForLogging Returns the set of root symlinks. + * @param getRepoMappingManifestForLogging Returns the repo mapping manifest if it exists. + * @param isLegacyExternalRunfiles Whether this runfiles tree materializes external runfiles also + * at their legacy locations. + * @param isMappingCached Whether this runfiles tree is likely to be used by multiple spawns. + */ + record RunfilesTree( + PathFragment getExecPath, + NestedSet getArtifactsAtCanonicalLocationsForLogging, + Iterable getEmptyFilenamesForLogging, + NestedSet getSymlinksForLogging, + NestedSet getRootSymlinksForLogging, + @Nullable Artifact getRepoMappingManifestForLogging, + boolean isLegacyExternalRunfiles, + boolean isMappingCached) {} + + /** + * A map from runfiles middleman artifacts to the {@link RunfilesTree} they represent. + * + *

This is used for the purposes of the execution log only and matches the architecture of + * Bazel 8, in which a runfiles tree can be obtained from its middleman via {@link + * InputMetadataProvider}. + */ + Map getRunfilesTreesForLogging(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java b/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java new file mode 100644 index 00000000000000..e69de29bb2d1d6 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 04854ed8317c0c..c724792fe93f43 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 @@ -250,10 +250,17 @@ public NestedSet getSymlinks() { @Override public Depset /**/ getEmptyFilenamesForStarlark() { - return Depset.of(String.class, getEmptyFilenames()); + return Depset.of( + String.class, + NestedSetBuilder.wrap( + Order.STABLE_ORDER, + Iterables.transform(getEmptyFilenames(), PathFragment::getPathString))); } - public NestedSet getEmptyFilenames() { + public Iterable getEmptyFilenames() { + if (emptyFilesSupplier == DUMMY_EMPTY_FILES_SUPPLIER) { + return ImmutableList.of(); + } Set manifestKeys = Streams.concat( symlinks.toList().stream().map(SymlinkEntry::getPath), @@ -264,13 +271,7 @@ public NestedSet getEmptyFilenames() { ? artifact.getOutputDirRelativePath(false) : artifact.getRunfilesPath())) .collect(ImmutableSet.toImmutableSet()); - Iterable emptyKeys = emptyFilesSupplier.getExtraPaths(manifestKeys); - return NestedSetBuilder.stableOrder() - .addAll( - Streams.stream(emptyKeys) - .map(PathFragment::toString) - .collect(ImmutableList.toImmutableList())) - .build(); + return emptyFilesSupplier.getExtraPaths(manifestKeys); } /** @@ -387,6 +388,10 @@ public Map getRunfilesInputs( return builder.build(); } + public boolean isLegacyExternalRunfiles() { + return legacyExternalRunfiles; + } + /** * Helper class to handle munging the paths of external artifacts. */ 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 56abd4d70c014a..6833a4a67ecf02 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 @@ -563,6 +563,22 @@ public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) { runfiles, repoMappingManifest, runfileSymlinksMode, - buildRunfileLinks); + buildRunfileLinks, + runfilesMiddleman); + } + + @Override + public Map getRunfilesTreesForLogging() { + return ImmutableMap.of( + getRunfilesMiddleman(), + new RunfilesTree( + getRunfilesDirectoryExecPath(), + runfiles.getArtifacts(), + runfiles.getEmptyFilenames(), + runfiles.getSymlinks(), + runfiles.getRootSymlinks(), + getRepoMappingManifest(), + runfiles.isLegacyExternalRunfiles(), + /* isMappingCached= */ false)); } } 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 26fca6a7c45a07..8b26a4202fb04f 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 @@ -40,6 +40,7 @@ public final class SingleRunfilesSupplier implements RunfilesSupplier { @Nullable private final Artifact repoMappingManifest; private final RunfileSymlinksMode runfileSymlinksMode; private final boolean buildRunfileLinks; + @Nullable private final Artifact runfilesMiddleman; /** * Same as {@link SingleRunfilesSupplier#SingleRunfilesSupplier(PathFragment, Runfiles, boolean, @@ -54,14 +55,16 @@ public static SingleRunfilesSupplier createCaching( Runfiles runfiles, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, - boolean buildRunfileLinks) { + boolean buildRunfileLinks, + @Nullable Artifact runfilesMiddleman) { return new SingleRunfilesSupplier( runfilesDir, runfiles, /* runfilesCachingEnabled= */ true, repoMappingManifest, runfileSymlinksMode, - buildRunfileLinks); + buildRunfileLinks, + runfilesMiddleman); } /** @@ -78,14 +81,16 @@ public SingleRunfilesSupplier( Runfiles runfiles, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, - boolean buildRunfileLinks) { + boolean buildRunfileLinks, + @Nullable Artifact runfilesMiddleman) { this( runfilesDir, runfiles, /* runfilesCachingEnabled= */ false, repoMappingManifest, runfileSymlinksMode, - buildRunfileLinks); + buildRunfileLinks, + runfilesMiddleman); } private SingleRunfilesSupplier( @@ -94,7 +99,8 @@ private SingleRunfilesSupplier( boolean runfilesCachingEnabled, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, - boolean buildRunfileLinks) { + boolean buildRunfileLinks, + @Nullable Artifact runfilesMiddleman) { this( runfilesDir, runfiles, @@ -105,7 +111,8 @@ private SingleRunfilesSupplier( /* eventHandler= */ null, /* location= */ null, repoMappingManifest), repoMappingManifest, runfileSymlinksMode, - buildRunfileLinks); + buildRunfileLinks, + runfilesMiddleman); } private SingleRunfilesSupplier( @@ -114,7 +121,8 @@ private SingleRunfilesSupplier( Supplier> runfilesInputs, @Nullable Artifact repoMappingManifest, RunfileSymlinksMode runfileSymlinksMode, - boolean buildRunfileLinks) { + boolean buildRunfileLinks, + @Nullable Artifact runfilesMiddleman) { checkArgument(!runfilesDir.isAbsolute()); this.runfilesDir = checkNotNull(runfilesDir); this.runfiles = checkNotNull(runfiles); @@ -122,6 +130,7 @@ private SingleRunfilesSupplier( this.repoMappingManifest = repoMappingManifest; this.runfileSymlinksMode = runfileSymlinksMode; this.buildRunfileLinks = buildRunfileLinks; + this.runfilesMiddleman = runfilesMiddleman; } @Override @@ -163,7 +172,28 @@ public SingleRunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfiles runfilesInputs, repoMappingManifest, runfileSymlinksMode, - buildRunfileLinks); + buildRunfileLinks, + runfilesMiddleman); + } + + @Override + public Map getRunfilesTreesForLogging() { + if (runfilesMiddleman == null) { + // This can only happen with the "new_runfiles_supplier" Python builtin function, which is + // unused in builtin rules as well as in the latest rules_python version. + return ImmutableMap.of(); + } + return ImmutableMap.of( + runfilesMiddleman, + new RunfilesTree( + runfilesDir, + runfiles.getArtifacts(), + runfiles.getEmptyFilenames(), + runfiles.getSymlinks(), + runfiles.getRootSymlinks(), + repoMappingManifest, + runfiles.isLegacyExternalRunfiles(), + runfilesInputs instanceof RunfilesCacher)); } /** Softly caches the result of {@link Runfiles#getRunfilesInputs}. */ 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 0ad0ffab1e6f95..13e77c0a6841a5 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 @@ -366,7 +366,8 @@ private TestParams createTestAction(int shards) runfilesSupport.getRunfiles(), runfilesSupport.getRepoMappingManifest(), runfilesSupport.getRunfileSymlinksMode(), - runfilesSupport.isBuildRunfileLinks()); + runfilesSupport.isBuildRunfileLinks(), + runfilesSupport.getRunfilesMiddleman()); } else { testRunfilesSupplier = runfilesSupport; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD index ae5d5a454476d0..0fe03e3831053c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -136,6 +136,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:executor_builder", "//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry", "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java index 5be63fb76b23d0..19710e2d1b93ef 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/SpawnLogModule.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.exec.ExpandedSpawnLogContext.Encoding; import com.google.devtools.build.lib.exec.ModuleActionContextRegistry; import com.google.devtools.build.lib.exec.SpawnLogContext; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; @@ -104,6 +105,10 @@ private void initOutputs(CommandEnvironment env) throws IOException { new CompactSpawnLogContext( outputPath, env.getExecRoot().asFragment(), + env.getWorkspaceName(), + env.getOptions() + .getOptions(BuildLanguageOptions.class) + .experimentalSiblingRepositoryLayout, env.getOptions().getOptions(RemoteOptions.class), env.getRuntime().getFileSystem().getDigestFunction(), env.getXattrProvider()); diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index cfd94913148ad0..1dc4fed16b7f64 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -269,6 +269,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier", + "//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry", "//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_utils", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", @@ -297,8 +298,8 @@ java_library( ], deps = [ "//src/main/java/com/google/devtools/build/lib/profiler", - "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/util:pair", + "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/protobuf:spawn_java_proto", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java index 1b2273d6b527c0..992373c5cb56f9 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java @@ -18,16 +18,22 @@ import com.github.luben.zstd.ZstdOutputStream; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.InputMetadataProvider; +import com.google.devtools.build.lib.actions.RunfilesSupplier; +import com.google.devtools.build.lib.actions.RunfilesSupplier.RunfilesTree; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.analysis.SymlinkEntry; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor; @@ -49,17 +55,17 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.XattrProvider; -import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.errorprone.annotations.CheckReturnValue; import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import java.io.BufferedOutputStream; import java.io.IOException; import java.time.Duration; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.SortedMap; import java.util.concurrent.ForkJoinPool; import javax.annotation.Nullable; @@ -136,12 +142,15 @@ private interface ExecLogEntrySupplier { } private final PathFragment execRoot; + private final String workspaceName; + private final boolean siblingRepositoryLayout; @Nullable private final RemoteOptions remoteOptions; private final DigestHashFunction digestHashFunction; private final XattrProvider xattrProvider; // Maps a key identifying an entry into its ID. - // Each key is either a NestedSet.Node or the String path of a file, directory or symlink. + // Each key is either a NestedSet.Node or the String path of a file, directory, symlink or + // runfiles tree. // Only entries that are likely to be referenced by future entries are stored. // Use a specialized map for minimal memory footprint. @GuardedBy("this") @@ -157,11 +166,15 @@ private interface ExecLogEntrySupplier { public CompactSpawnLogContext( Path outputPath, PathFragment execRoot, + String workspaceName, + boolean siblingRepositoryLayout, @Nullable RemoteOptions remoteOptions, DigestHashFunction digestHashFunction, XattrProvider xattrProvider) throws IOException, InterruptedException { this.execRoot = execRoot; + this.workspaceName = workspaceName; + this.siblingRepositoryLayout = siblingRepositoryLayout; this.remoteOptions = remoteOptions; this.digestHashFunction = digestHashFunction; this.xattrProvider = xattrProvider; @@ -178,13 +191,14 @@ private static MessageOutputStream getOutputStream(Path path) thro } private void logInvocation() throws IOException, InterruptedException { - logEntry( - null, + logEntryWithoutId( () -> ExecLogEntry.newBuilder() .setInvocation( ExecLogEntry.Invocation.newBuilder() - .setHashFunctionName(digestHashFunction.toString()))); + .setHashFunctionName(digestHashFunction.toString()) + .setWorkspaceRunfilesDirectory(workspaceName) + .setSiblingRepositoryLayout(siblingRepositoryLayout))); } @Override @@ -223,21 +237,16 @@ public void logSpawn( for (ActionInput output : spawn.getOutputFiles()) { Path path = fileSystem.getPath(execRoot.getRelative(output.getExecPath())); if (!output.isDirectory() && !output.isSymlink() && path.isFile()) { - builder.addOutputs( - ExecLogEntry.Output.newBuilder() - .setFileId(logFile(output, path, inputMetadataProvider))); + builder.addOutputsBuilder().setOutputId(logFile(output, path, inputMetadataProvider)); } else if (!output.isSymlink() && path.isDirectory()) { // TODO(tjgq): Tighten once --incompatible_disallow_unsound_directory_outputs is gone. - builder.addOutputs( - ExecLogEntry.Output.newBuilder() - .setDirectoryId(logDirectory(output, path, inputMetadataProvider))); + builder + .addOutputsBuilder() + .setOutputId(logDirectory(output, path, inputMetadataProvider)); } else if (output.isSymlink() && path.isSymbolicLink()) { - builder.addOutputs( - ExecLogEntry.Output.newBuilder() - .setUnresolvedSymlinkId(logUnresolvedSymlink(output, path))); + builder.addOutputsBuilder().setOutputId(logUnresolvedSymlink(output, path)); } else { - builder.addOutputs( - ExecLogEntry.Output.newBuilder().setInvalidOutputPath(output.getExecPathString())); + builder.addOutputsBuilder().setInvalidOutputPath(output.getExecPathString()); } } @@ -259,7 +268,7 @@ public void logSpawn( builder.setMetrics(getSpawnMetricsProto(result)); try (SilentCloseable c1 = Profiler.instance().profile("logEntry")) { - logEntry(null, () -> ExecLogEntry.newBuilder().setSpawn(builder)); + logEntryWithoutId(() -> ExecLogEntry.newBuilder().setSpawn(builder)); } } } @@ -285,7 +294,7 @@ public void logSymlinkAction(AbstractAction action) throws IOException, Interrup builder.setMnemonic(action.getMnemonic()); try (SilentCloseable c1 = Profiler.instance().profile("logEntry")) { - logEntry(null, () -> ExecLogEntry.newBuilder().setSymlinkAction(builder)); + logEntryWithoutId(() -> ExecLogEntry.newBuilder().setSymlinkAction(builder)); } } } @@ -293,8 +302,8 @@ public void logSymlinkAction(AbstractAction action) throws IOException, Interrup /** * Logs the inputs. * - * @return the entry ID of the {@link ExecLogEntry.Set} describing the inputs, or 0 if there are - * no inputs. + * @return the entry ID of the {@link ExecLogEntry.InputSet} describing the inputs, or 0 if there + * are no inputs. */ private int logInputs( Spawn spawn, InputMetadataProvider inputMetadataProvider, FileSystem fileSystem) @@ -305,68 +314,71 @@ private int logInputs( // spawn is almost never a transitive member of other nested sets, and not recording its entry // ID turns out to be a very significant memory optimization. - ImmutableList.Builder additionalDirectoryIds = ImmutableList.builder(); - - for (Map.Entry> entry : - spawn.getRunfilesSupplier().getMappings().entrySet()) { - // The runfiles symlink tree might not have been materialized on disk, so use the mapping. - additionalDirectoryIds.add( - logRunfilesDirectory( - entry.getKey(), entry.getValue(), inputMetadataProvider, fileSystem)); - } + ImmutableList.Builder additionalInputIds = ImmutableList.builder(); for (Artifact fileset : spawn.getFilesetMappings().keySet()) { // The fileset symlink tree is always materialized on disk. - additionalDirectoryIds.add( + additionalInputIds.add( logDirectory( fileset, fileSystem.getPath(execRoot.getRelative(fileset.getExecPath())), inputMetadataProvider)); } - return logNestedSet( + return logInputSet( spawn.getInputFiles(), - additionalDirectoryIds.build(), + spawn.getRunfilesSupplier().getRunfilesTreesForLogging(), + spawn.getRunfilesSupplier().getRunfilesTreesForLogging().keySet(), + additionalInputIds.build(), inputMetadataProvider, fileSystem, - /* shared= */ false); + /* shared= */ false, + "TestRunner".equals(spawn.getMnemonic())); } /** * Logs the tool inputs. * - * @return the entry ID of the {@link ExecLogEntry.Set} describing the tool inputs, or 0 if there - * are no tool inputs. + * @return the entry ID of the {@link ExecLogEntry.InputSet} describing the tool inputs, or 0 if + * there are no tool inputs. */ private int logTools( Spawn spawn, InputMetadataProvider inputMetadataProvider, FileSystem fileSystem) throws IOException, InterruptedException { - return logNestedSet( + return logInputSet( spawn.getToolFiles(), + spawn.getRunfilesSupplier().getRunfilesTreesForLogging(), + ImmutableSet.of(), ImmutableList.of(), inputMetadataProvider, fileSystem, - /* shared= */ true); + /* shared= */ true, + "TestRunner".equals(spawn.getMnemonic())); } /** * Logs a nested set. * * @param set the nested set + * @param runfilesTrees runfiles trees of the spawn to be added to the set if it contains the + * corresponding middleman * @param additionalDirectoryIds the entry IDs of additional {@link ExecLogEntry.Directory} * entries to include as direct members * @param shared whether this nested set is likely to be a transitive member of other sets * @return the entry ID of the {@link ExecLogEntry.InputSet} describing the nested set, or 0 if * the nested set is empty. */ - private int logNestedSet( + private int logInputSet( NestedSet set, + Map runfilesTrees, + Set extraMiddleman, Collection additionalDirectoryIds, InputMetadataProvider inputMetadataProvider, FileSystem fileSystem, - boolean shared) + boolean shared, + boolean isTestRunnerSpawn) throws IOException, InterruptedException { - if (set.isEmpty() && additionalDirectoryIds.isEmpty()) { + if (set.isEmpty() && additionalDirectoryIds.isEmpty() && extraMiddleman.isEmpty()) { return 0; } @@ -374,42 +386,98 @@ private int logNestedSet( shared ? set.toNode() : null, () -> { ExecLogEntry.InputSet.Builder builder = - ExecLogEntry.InputSet.newBuilder().addAllDirectoryIds(additionalDirectoryIds); + ExecLogEntry.InputSet.newBuilder().addAllInputIds(additionalDirectoryIds); for (NestedSet transitive : set.getNonLeaves()) { checkState(!transitive.isEmpty()); builder.addTransitiveSetIds( - logNestedSet( + logInputSet( transitive, - /* additionalDirectoryIds= */ ImmutableList.of(), + ImmutableMap.of(), + ImmutableSet.of(), + ImmutableList.of(), inputMetadataProvider, fileSystem, - /* shared= */ true)); + /* shared= */ true, + /* isTestRunnerSpawn= */ false)); } - for (ActionInput input : set.getLeaves()) { - // Runfiles are logged separately. - if (input instanceof Artifact && ((Artifact) input).isMiddlemanArtifact()) { + for (ActionInput input : Iterables.concat(set.getLeaves(), extraMiddleman)) { + if (input instanceof Artifact artifact && artifact.isMiddlemanArtifact()) { + RunfilesTree runfilesTree = runfilesTrees.get(artifact); + builder.addInputIds( + logRunfilesTree( + runfilesTree, inputMetadataProvider, fileSystem, isTestRunnerSpawn)); continue; } + // Filesets are logged separately. - if (input instanceof Artifact && ((Artifact) input).isFileset()) { + if (input instanceof Artifact artifact && artifact.isFileset()) { continue; } - Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath())); - if (isInputDirectory(input, path, inputMetadataProvider)) { - builder.addDirectoryIds(logDirectory(input, path, inputMetadataProvider)); - } else if (input.isSymlink()) { - builder.addUnresolvedSymlinkIds(logUnresolvedSymlink(input, path)); - } else { - builder.addFileIds(logFile(input, path, inputMetadataProvider)); - } + + builder.addInputIds(logInput(input, inputMetadataProvider, fileSystem)); } return ExecLogEntry.newBuilder().setInputSet(builder); }); } + /** + * Logs a nested set of {@link SymlinkEntry}. + * + * @return the entry ID of the {@link ExecLogEntry.SymlinkEntrySet} describing the nested set, or + * 0 if the nested set is empty. + */ + private int logSymlinkEntries( + NestedSet symlinks, + InputMetadataProvider inputMetadataProvider, + FileSystem fileSystem) + throws IOException, InterruptedException { + if (symlinks.isEmpty()) { + return 0; + } + + return logEntry( + symlinks.toNode(), + () -> { + ExecLogEntry.SymlinkEntrySet.Builder builder = ExecLogEntry.SymlinkEntrySet.newBuilder(); + + for (NestedSet transitive : symlinks.getNonLeaves()) { + checkState(!transitive.isEmpty()); + builder.addTransitiveSetIds( + logSymlinkEntries(transitive, inputMetadataProvider, fileSystem)); + } + + for (SymlinkEntry input : symlinks.getLeaves()) { + builder.putDirectEntries( + input.getPathString(), + logInput(input.getArtifact(), inputMetadataProvider, fileSystem)); + } + + return ExecLogEntry.newBuilder().setSymlinkEntrySet(builder); + }); + } + + /** + * Logs a single input that is either a file, a directory or a symlink. + * + * @return the entry ID of the {@link ExecLogEntry} describing the input. + */ + private int logInput( + ActionInput input, InputMetadataProvider inputMetadataProvider, FileSystem fileSystem) + throws IOException, InterruptedException { + Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath())); + // TODO(tjgq): Tighten once --incompatible_disallow_unsound_directory_outputs is gone. + if (isInputDirectory(input, path, inputMetadataProvider)) { + return logDirectory(input, path, inputMetadataProvider); + } else if (input.isSymlink()) { + return logUnresolvedSymlink(input, path); + } else { + return logFile(input, path, inputMetadataProvider); + } + } + /** * Logs a file. * @@ -449,7 +517,7 @@ private int logFile(ActionInput input, Path path, InputMetadataProvider inputMet * Logs a directory. * *

This may be either a source directory, a fileset or an output directory. For runfiles, - * {@link #logRunfilesDirectory} must be used instead. + * {@link #logRunfilesTree} must be used instead. * * @param input the input representing the directory. * @param root the path to the directory, which must have already been verified to be of the @@ -466,64 +534,76 @@ private int logDirectory( .setDirectory( ExecLogEntry.Directory.newBuilder() .setPath(input.getExecPathString()) - .addAllFiles( - expandDirectory(root, /* pathPrefix= */ null, inputMetadataProvider)))); + .addAllFiles(expandDirectory(root, inputMetadataProvider)))); } /** - * Logs a runfiles directory. + * Logs a runfiles directory by storing the information in its {@link RunfilesSupplier}. * - *

We can't use {@link #logDirectory} because the runfiles symlink tree might not have been - * materialized on disk. We must follow the mappings to the actual location of the artifacts. + *

Since runfiles trees can be very large and, for tests, are only used by a single spawn, we + * store them in the log as a special entry that references the nested set of artifacts instead of + * as a flat directory. * - * @param root the path to the runfiles directory - * @param mapping a map from runfiles-relative path to the underlying artifact, or null for an - * empty file - * @return the entry ID of the {@link ExecLogEntry.Directory} describing the directory. + * @return the entry ID of the {@link ExecLogEntry.RunfilesTree} describing the directory. */ - private int logRunfilesDirectory( - PathFragment root, - Map mapping, + private int logRunfilesTree( + RunfilesTree runfilesTree, InputMetadataProvider inputMetadataProvider, - FileSystem fileSystem) + FileSystem fileSystem, + boolean isTestRunnerSpawn) throws IOException, InterruptedException { return logEntry( - root.getPathString(), + // Runfiles of non-test spawns are tool inputs and thus potentially reused + // between spawns. Runfiles of test spawns are reused if the test is attempted + // multiple times in the same build; in this case, the runfiles tree caches + // its mapping. + !isTestRunnerSpawn || runfilesTree.isMappingCached() + ? runfilesTree.getExecPath().getPathString() + : null, () -> { - ExecLogEntry.Directory.Builder builder = - ExecLogEntry.Directory.newBuilder().setPath(root.getPathString()); - - for (Map.Entry entry : mapping.entrySet()) { - String runfilesPath = entry.getKey().getPathString(); - Artifact input = entry.getValue(); - - if (input == null) { - // Empty file. - builder.addFiles(ExecLogEntry.File.newBuilder().setPath(runfilesPath)); - continue; - } - - Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath())); - - if (isInputDirectory(input, path, inputMetadataProvider)) { - builder.addAllFiles(expandDirectory(path, runfilesPath, inputMetadataProvider)); - continue; - } - - Digest digest = - computeDigest( - input, - path, - inputMetadataProvider, - xattrProvider, - digestHashFunction, - /* includeHashFunctionName= */ false); - - builder.addFiles( - ExecLogEntry.File.newBuilder().setPath(runfilesPath).setDigest(digest)); + ExecLogEntry.RunfilesTree.Builder builder = + ExecLogEntry.RunfilesTree.newBuilder() + .setPath(runfilesTree.getExecPath().getPathString()) + .setLegacyExternalRunfiles(runfilesTree.isLegacyExternalRunfiles()); + + builder.setInputSetId( + logInputSet( + runfilesTree.getArtifactsAtCanonicalLocationsForLogging(), + ImmutableMap.of(), + ImmutableSet.of(), + ImmutableList.of(), + inputMetadataProvider, + fileSystem, + // The runfiles tree itself is shared, but the nested set is unique to the tree as + // it contains the executable. + /* shared= */ false, + // This value only matters for nested sets that may contain runfiles trees, but + // these are never nested. + /* isTestRunnerSpawn= */ false)); + builder.setSymlinksId( + logSymlinkEntries( + runfilesTree.getSymlinksForLogging(), inputMetadataProvider, fileSystem)); + builder.setRootSymlinksId( + logSymlinkEntries( + runfilesTree.getRootSymlinksForLogging(), inputMetadataProvider, fileSystem)); + builder.addAllEmptyFiles( + Iterables.transform( + runfilesTree.getEmptyFilenamesForLogging(), PathFragment::getPathString)); + Artifact repoMappingManifest = runfilesTree.getRepoMappingManifestForLogging(); + if (repoMappingManifest != null) { + builder.setRepoMappingManifest( + ExecLogEntry.File.newBuilder() + .setDigest( + computeDigest( + repoMappingManifest, + repoMappingManifest.getPath(), + inputMetadataProvider, + xattrProvider, + digestHashFunction, + /* includeHashFunctionName= */ false))); } - return ExecLogEntry.newBuilder().setDirectory(builder); + return ExecLogEntry.newBuilder().setRunfilesTree(builder); }); } @@ -531,19 +611,15 @@ private int logRunfilesDirectory( * Expands a directory. * * @param root the path to the directory - * @param pathPrefix a prefix to prepend to each child path * @return the list of files transitively contained in the directory */ private List expandDirectory( - Path root, @Nullable String pathPrefix, InputMetadataProvider inputMetadataProvider) + Path root, InputMetadataProvider inputMetadataProvider) throws IOException, InterruptedException { ArrayList files = new ArrayList<>(); visitDirectory( root, (child) -> { - String childPath = pathPrefix != null ? pathPrefix + "/" : ""; - childPath += child.relativeTo(root).getPathString(); - Digest digest = computeDigest( /* input= */ null, @@ -554,14 +630,17 @@ private List expandDirectory( /* includeHashFunctionName= */ false); ExecLogEntry.File file = - ExecLogEntry.File.newBuilder().setPath(childPath).setDigest(digest).build(); + ExecLogEntry.File.newBuilder() + .setPath(child.relativeTo(root).getPathString()) + .setDigest(digest) + .build(); synchronized (files) { files.add(file); } }); - Collections.sort(files, EXEC_LOG_ENTRY_FILE_COMPARATOR); + files.sort(EXEC_LOG_ENTRY_FILE_COMPARATOR); return files; } @@ -587,6 +666,16 @@ private int logUnresolvedSymlink(ActionInput input, Path path) .setTargetPath(path.readSymbolicLink().getPathString()))); } + /** + * Ensures an entry is written to the log without an ID. + * + * @param supplier called to compute the entry; may cause other entries to be logged + */ + private synchronized void logEntryWithoutId(ExecLogEntrySupplier supplier) + throws IOException, InterruptedException { + outputStream.write(supplier.get().build()); + } + /** * Ensures an entry is written to the log and returns its assigned ID. * @@ -597,14 +686,15 @@ private int logUnresolvedSymlink(ActionInput input, Path path) * @param supplier called to compute the entry; may cause other entries to be logged * @return the entry ID */ - @CanIgnoreReturnValue + @CheckReturnValue private synchronized int logEntry(@Nullable Object key, ExecLogEntrySupplier supplier) throws IOException, InterruptedException { try (SilentCloseable c = Profiler.instance().profile("logEntry/synchronized")) { if (key == null) { // No need to check for a previously added entry. + ExecLogEntry.Builder entry = supplier.get(); int id = nextEntryId++; - outputStream.write(supplier.get().setId(id).build()); + outputStream.write(entry.setId(id).build()); return id; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java index 37b40d68e98d87..2b6e775614561d 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java @@ -13,14 +13,19 @@ // limitations under the License. package com.google.devtools.build.lib.exec; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue.UnresolvedSymlinkArtifactValue; import com.google.devtools.build.lib.actions.InputMetadataProvider; +import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.Spawns; @@ -156,6 +161,19 @@ public void logSpawn( builder.addAllEnvironmentVariables(getEnvironmentVariables(spawn)); ImmutableSet toolFiles = spawn.getToolFiles().toSet(); + ImmutableList toolRunfilesDirectories = + toolFiles.stream() + .filter( + actionInput -> + actionInput instanceof Artifact artifact && artifact.isMiddlemanArtifact()) + .map( + runfilesMiddleman -> + spawn + .getRunfilesSupplier() + .getRunfilesTreesForLogging() + .get(runfilesMiddleman)) + .map(RunfilesSupplier.RunfilesTree::getExecPath) + .collect(toImmutableList()); try (SilentCloseable c1 = Profiler.instance().profile("logSpawn/inputs")) { for (Map.Entry e : inputMap.entrySet()) { @@ -164,14 +182,18 @@ public void logSpawn( if (input instanceof VirtualActionInput.EmptyActionInput) { // Do not include a digest, as it's a waste of space. - builder.addInputsBuilder().setPath(displayPath.getPathString()); + builder + .addInputsBuilder() + .setPath(displayPath.getPathString()) + .setIsTool(toolRunfilesDirectories.stream().anyMatch(displayPath::startsWith)); continue; } boolean isTool = toolFiles.contains(input) || (input instanceof TreeFileArtifact - && toolFiles.contains(((TreeFileArtifact) input).getParent())); + && toolFiles.contains(((TreeFileArtifact) input).getParent())) + || toolRunfilesDirectories.stream().anyMatch(displayPath::startsWith); Path contentPath = fileSystem.getPath(execRoot.getRelative(input.getExecPathString())); diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index 24e4a67357d037..17eb9c3948bb4c 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java @@ -122,7 +122,7 @@ private static void addMapping( /** Adds runfiles inputs from runfilesSupplier to inputMappings. */ @VisibleForTesting - void addRunfilesToInputs( + public void addRunfilesToInputs( Map inputMap, RunfilesSupplier runfilesSupplier, InputMetadataProvider inputMetadataProvider, diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java index 0b495b775348ea..56f1b5f188675a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java @@ -13,38 +13,110 @@ // limitations under the License. package com.google.devtools.build.lib.exec; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableList.toImmutableList; + import com.github.luben.zstd.ZstdInputStream; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.exec.Protos.ExecLogEntry; import com.google.devtools.build.lib.exec.Protos.File; import com.google.devtools.build.lib.exec.Protos.SpawnExec; -import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.MessageInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.SortedMap; import java.util.SortedSet; import java.util.TreeMap; import java.util.TreeSet; +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.regex.MatchResult; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; import javax.annotation.Nullable; /** Reconstructs an execution log in expanded format from the compact format representation. */ public final class SpawnLogReconstructor implements MessageInputStream { + // The path of the repo mapping manifest file under the runfiles tree. + private static final String REPO_MAPPING_MANIFEST = "_repo_mapping"; + + // Examples: + // * bazel-out/k8-fastbuild/bin/pkg/file.txt (repo: null, path: "pkg/file.txt") + // * bazel-out/k8-fastbuild/bin/external/some_repo/pkg/file.txt (repo: "some_repo", path: + // "pkg/file.txt") + private static final Pattern DEFAULT_GENERATED_FILE_RUNFILES_PATH_PATTERN = + Pattern.compile("(?:bazel|blaze)-out/[^/]+/[^/]+/(?:external/(?[^/]+)/)?(?.+)"); + + // Examples: + // * pkg/file.txt (repo: null, path: "pkg/file.txt") + // * external/some_repo/pkg/file.txt (repo: "some_repo", path: "pkg/file.txt") + private static final Pattern DEFAULT_SOURCE_FILE_RUNFILES_PATH_PATTERN = + Pattern.compile("(?:external/(?[^/]+)/)?(?.+)"); + + // Examples: + // * bazel-out/k8-fastbuild/bin/pkg/file.txt (repo: null, path: "pkg/file.txt") + // * bazel-out/some_repo/k8-fastbuild/bin/pkg/file.txt (repo: "some_repo", path: "pkg/file.txt") + // * bazel-out/k8-fastbuild/k8-fastbuild/bin/pkg/file.txt (repo: "k8-fastbuild", path: + // "pkg/file.txt") + // + // Repo names are distinguished from mnemonics via a positive lookahead on the following segment, + // which in the case of a repo name is a mnemonic and thus contains a hyphen, whereas a mnemonic + // is followed by an output directory name, which does not contain a hyphen unless it is + // "coverage-metadata" (which in turn is not likely to be a mnemonic). + private static final Pattern SIBLING_LAYOUT_GENERATED_FILE_RUNFILES_PATH_PATTERN = + Pattern.compile( + "(?:bazel|blaze)-out/(?:(?[^/]+(?=/[^/]+-[^/]+/)(?!/coverage-metadata/))/)?[^/]+/[^/]+/(?.+)"); + + // Examples: + // * pkg/file.txt (repo: null, path: "pkg/file.txt") + // * ../some_repo/pkg/file.txt (repo: "some_repo", path: "pkg/file.txt") + private static final Pattern SIBLING_LAYOUT_SOURCE_FILE_RUNFILES_PATH_PATTERN = + Pattern.compile("(?:\\.\\./(?[^/]+)/)?(?.+)"); + private final ZstdInputStream in; - private final HashMap fileMap = new HashMap<>(); - private final HashMap>> dirMap = new HashMap<>(); - private final HashMap symlinkMap = new HashMap<>(); - private final HashMap setMap = new HashMap<>(); + /** Represents a reconstructed input file, symlink, or directory. */ + private sealed interface Input { + String path(); + + record File(Protos.File file) implements Input { + @Override + public String path() { + return file.getPath(); + } + } + + record Symlink(Protos.File symlink) implements Input { + @Override + public String path() { + return symlink.getPath(); + } + } + + record Directory(String path, Collection files) implements Input {} + } + + // Stores both Inputs and InputSets. Bazel uses consecutive IDs starting from 1, so we can use + // an ArrayList to store them together efficiently. + private final ArrayList inputMap = new ArrayList<>(); private String hashFunctionName = ""; + private String workspaceRunfilesDirectory = ""; + private boolean siblingRepositoryLayout = false; public SpawnLogReconstructor(InputStream in) throws IOException { this.in = new ZstdInputStream(in); + // Add a null entry for the 0th index as IDs are 1-based. + inputMap.add(null); } @Override @@ -53,29 +125,28 @@ public SpawnExec read() throws IOException { ExecLogEntry entry; while ((entry = ExecLogEntry.parseDelimitedFrom(in)) != null) { switch (entry.getTypeCase()) { - case INVOCATION: + case INVOCATION -> { hashFunctionName = entry.getInvocation().getHashFunctionName(); - break; - case FILE: - fileMap.put(entry.getId(), reconstructFile(entry.getFile())); - break; - case DIRECTORY: - dirMap.put(entry.getId(), reconstructDir(entry.getDirectory())); - break; - case UNRESOLVED_SYMLINK: - symlinkMap.put(entry.getId(), reconstructSymlink(entry.getUnresolvedSymlink())); - break; - case INPUT_SET: - setMap.put(entry.getId(), entry.getInputSet()); - break; - case SPAWN: + workspaceRunfilesDirectory = entry.getInvocation().getWorkspaceRunfilesDirectory(); + siblingRepositoryLayout = entry.getInvocation().getSiblingRepositoryLayout(); + } + case FILE -> putInput(entry.getId(), reconstructFile(entry.getFile())); + case DIRECTORY -> putInput(entry.getId(), reconstructDir(entry.getDirectory())); + case UNRESOLVED_SYMLINK -> + putInput(entry.getId(), reconstructSymlink(entry.getUnresolvedSymlink())); + case RUNFILES_TREE -> + putInput(entry.getId(), reconstructRunfilesDir(entry.getRunfilesTree())); + case INPUT_SET -> putInputSet(entry.getId(), entry.getInputSet()); + case SYMLINK_ENTRY_SET -> putSymlinkEntrySet(entry.getId(), entry.getSymlinkEntrySet()); + case SPAWN -> { return reconstructSpawnExec(entry.getSpawn()); - case SYMLINK_ACTION: + } + case SYMLINK_ACTION -> { // Symlink actions are not represented in the expanded format. - break; - default: - throw new IOException( - String.format("unknown entry type %d", entry.getTypeCase().getNumber())); + } + default -> + throw new IOException( + String.format("unknown entry type %d", entry.getTypeCase().getNumber())); } } return null; @@ -102,12 +173,14 @@ private SpawnExec reconstructSpawnExec(ExecLogEntry.Spawn entry) throws IOExcept builder.setPlatform(entry.getPlatform()); } - SortedMap inputs = reconstructInputs(entry.getInputSetId()); - SortedMap toolInputs = reconstructInputs(entry.getToolSetId()); + SortedMap inputs = new TreeMap<>(); + visitInputSet(entry.getInputSetId(), file -> inputs.put(file.getPath(), file), input -> {}); + HashSet toolInputs = new HashSet<>(); + visitInputSet(entry.getToolSetId(), file -> toolInputs.add(file.getPath()), input -> {}); for (Map.Entry e : inputs.entrySet()) { File file = e.getValue(); - if (toolInputs.containsKey(e.getKey())) { + if (toolInputs.contains(e.getKey())) { file = file.toBuilder().setIsTool(true).build(); } builder.addInputs(file); @@ -117,29 +190,20 @@ private SpawnExec reconstructSpawnExec(ExecLogEntry.Spawn entry) throws IOExcept for (ExecLogEntry.Output output : entry.getOutputsList()) { switch (output.getTypeCase()) { - case FILE_ID: - File file = getFromMap(fileMap, output.getFileId()); - listedOutputs.add(file.getPath()); - builder.addActualOutputs(file); - break; - case DIRECTORY_ID: - Pair> dir = getFromMap(dirMap, output.getDirectoryId()); - listedOutputs.add(dir.getFirst()); - for (File dirFile : dir.getSecond()) { - builder.addActualOutputs(dirFile); + case OUTPUT_ID -> { + Input input = getInput(output.getOutputId()); + listedOutputs.add(input.path()); + switch (input) { + case Input.File(File file) -> builder.addActualOutputs(file); + case Input.Symlink(File symlink) -> builder.addActualOutputs(symlink); + case Input.Directory(String ignored, Collection files) -> + builder.addAllActualOutputs(files); } - break; - case UNRESOLVED_SYMLINK_ID: - File symlink = getFromMap(symlinkMap, output.getUnresolvedSymlinkId()); - listedOutputs.add(symlink.getPath()); - builder.addActualOutputs(symlink); - break; - case INVALID_OUTPUT_PATH: - listedOutputs.add(output.getInvalidOutputPath()); - break; - default: - throw new IOException( - String.format("unknown output type %d", output.getTypeCase().getNumber())); + } + case INVALID_OUTPUT_PATH -> listedOutputs.add(output.getInvalidOutputPath()); + default -> + throw new IOException( + "unknown output type %d".formatted(output.getTypeCase().getNumber())); } } @@ -152,56 +216,129 @@ private SpawnExec reconstructSpawnExec(ExecLogEntry.Spawn entry) throws IOExcept return builder.build(); } - private SortedMap reconstructInputs(int setId) throws IOException { - TreeMap inputs = new TreeMap<>(); - ArrayDeque setsToVisit = new ArrayDeque<>(); - HashSet visited = new HashSet<>(); - if (setId != 0) { - setsToVisit.addLast(setId); - visited.add(setId); + private void visitInputSet(int inputSetId, Consumer visitFile, Consumer visitInput) + throws IOException { + if (inputSetId == 0) { + return; } + ArrayDeque setsToVisit = new ArrayDeque<>(); + HashMap previousVisitCount = new HashMap<>(); + setsToVisit.push(inputSetId); while (!setsToVisit.isEmpty()) { - ExecLogEntry.InputSet set = getFromMap(setMap, setsToVisit.removeFirst()); - for (int fileId : set.getFileIdsList()) { - if (visited.add(fileId)) { - File file = getFromMap(fileMap, fileId); - inputs.put(file.getPath(), file); + int currentSetId = setsToVisit.pop(); + // In case order matters (it does for runfiles, but not for inputs), we visit the set in + // post-order (corresponds to Order#COMPILE_ORDER). Transitive sets are visited before direct + // children; both are visited in left-to-right order. + switch (previousVisitCount.merge(currentSetId, 0, (oldValue, newValue) -> 1)) { + case 0 -> { + // First visit, queue transitive sets for visit before revisiting the current set. + setsToVisit.push(currentSetId); + for (int transitiveSetId : + getInputSet(currentSetId).getTransitiveSetIdsList().reversed()) { + if (!previousVisitCount.containsKey(transitiveSetId)) { + setsToVisit.push(transitiveSetId); + } + } } - } - for (int dirId : set.getDirectoryIdsList()) { - if (visited.add(dirId)) { - Pair> dir = getFromMap(dirMap, dirId); - for (File dirFile : dir.getSecond()) { - inputs.put(dirFile.getPath(), dirFile); + case 1 -> { + // Second visit, visit the direct inputs only. + for (int inputId : getInputSet(currentSetId).getInputIdsList()) { + if (previousVisitCount.put(inputId, 1) != null) { + continue; + } + Input input = getInput(inputId); + visitInput.accept(input); + switch (input) { + case Input.File(File file) -> visitFile.accept(file); + case Input.Symlink(File symlink) -> visitFile.accept(symlink); + case Input.Directory(String ignored, Collection files) -> + files.forEach(visitFile); + } } } + default -> + throw new IllegalStateException( + "expected visit count to be 0 or 1, was " + previousVisitCount.get(currentSetId)); } - for (int symlinkId : set.getUnresolvedSymlinkIdsList()) { - if (visited.add(symlinkId)) { - File symlink = getFromMap(symlinkMap, symlinkId); - inputs.put(symlink.getPath(), symlink); + } + } + + private void visitSymlinkEntries( + ExecLogEntry.RunfilesTree runfilesTree, + boolean rootSymlinks, + BiConsumer> entryConsumer) + throws IOException { + int symlinkEntrySetId = + rootSymlinks ? runfilesTree.getRootSymlinksId() : runfilesTree.getSymlinksId(); + if (symlinkEntrySetId == 0) { + return; + } + ArrayDeque setsToVisit = new ArrayDeque<>(); + HashMap previousVisitCount = new HashMap<>(); + setsToVisit.push(symlinkEntrySetId); + while (!setsToVisit.isEmpty()) { + int currentSetId = setsToVisit.pop(); + // As order matters, we visit the set in post-order (corresponds to Order#COMPILE_ORDER). + // Transitive sets are visited before direct children; both are visited in left-to-right + // order. + switch (previousVisitCount.merge(currentSetId, 0, (oldValue, newValue) -> 1)) { + case 0 -> { + // First visit, queue transitive sets for visit before revisiting the current set. + setsToVisit.push(currentSetId); + for (int transitiveSetId : + getSymlinkEntrySet(currentSetId).getTransitiveSetIdsList().reversed()) { + if (!previousVisitCount.containsKey(transitiveSetId)) { + setsToVisit.push(transitiveSetId); + } + } } - } - for (int transitiveSetId : set.getTransitiveSetIdsList()) { - if (visited.add(transitiveSetId)) { - setsToVisit.addLast(transitiveSetId); + case 1 -> { + // Second visit, visit the direct entries only. + for (var pathAndInputId : + getSymlinkEntrySet(currentSetId).getDirectEntriesMap().entrySet()) { + String runfilesTreeRelativePath; + if (rootSymlinks) { + runfilesTreeRelativePath = pathAndInputId.getKey(); + } else if (pathAndInputId.getKey().startsWith("../")) { + runfilesTreeRelativePath = pathAndInputId.getKey().substring(3); + } else { + runfilesTreeRelativePath = workspaceRunfilesDirectory + "/" + pathAndInputId.getKey(); + } + String path = runfilesTree.getPath() + "/" + runfilesTreeRelativePath; + entryConsumer.accept( + runfilesTreeRelativePath, + reconstructRunfilesSymlinkTarget(path, pathAndInputId.getValue())); + if (runfilesTree.getLegacyExternalRunfiles() + && !rootSymlinks + && !runfilesTreeRelativePath.startsWith(workspaceRunfilesDirectory + "/")) { + String runfilesTreeLegacyRelativePath = + workspaceRunfilesDirectory + "/external/" + runfilesTreeRelativePath; + entryConsumer.accept( + runfilesTreeLegacyRelativePath, + reconstructRunfilesSymlinkTarget( + runfilesTree.getPath() + "/" + runfilesTreeLegacyRelativePath, + pathAndInputId.getValue())); + } + } } + default -> + throw new IllegalStateException( + "expected visit count to be 0 or 1, was " + previousVisitCount.get(currentSetId)); } } - return inputs; } - private Pair> reconstructDir(ExecLogEntry.Directory dir) { + private Input.Directory reconstructDir(ExecLogEntry.Directory dir) { ImmutableList.Builder builder = ImmutableList.builderWithExpectedSize(dir.getFilesCount()); - for (ExecLogEntry.File dirFile : dir.getFilesList()) { + for (var dirFile : dir.getFilesList()) { builder.add(reconstructFile(dir, dirFile)); } - return Pair.of(dir.getPath(), builder.build()); + return new Input.Directory(dir.getPath(), builder.build()); } - private File reconstructFile(ExecLogEntry.File entry) { - return reconstructFile(null, entry); + private Input.File reconstructFile(ExecLogEntry.File entry) { + return new Input.File(reconstructFile(null, entry)); } private File reconstructFile( @@ -215,19 +352,227 @@ private File reconstructFile( return builder.build(); } - private static File reconstructSymlink(ExecLogEntry.UnresolvedSymlink entry) { - return File.newBuilder() - .setPath(entry.getPath()) - .setSymlinkTargetPath(entry.getTargetPath()) - .build(); + private static Input.Symlink reconstructSymlink(ExecLogEntry.UnresolvedSymlink entry) { + return new Input.Symlink( + File.newBuilder() + .setPath(entry.getPath()) + .setSymlinkTargetPath(entry.getTargetPath()) + .build()); + } + + private Input.Directory reconstructRunfilesDir(ExecLogEntry.RunfilesTree runfilesTree) + throws IOException { + // In case of path collisions, runfiles should be collected in the following order, with + // later sources overriding earlier ones (see + // com.google.devtools.build.lib.analysis.Runfiles#getRunfilesInputs): + // + // 1. symlinks + // 2. artifacts at canonical locations + // 3. empty files + // 4. root symlinks + // 5. the _repo_mapping file with the repo mapping manifest + // 6. the /.runfile file (if the workspace runfiles directory + // wouldn't exist otherwise) + // + // Within each group represented by a nested set, the entries are traversed in postorder (i.e. + // the transitive sets are visited before the direct children). This is important to resolve + // conflicts in the same order as the real Runfiles implementation. + LinkedHashMap runfiles = new LinkedHashMap<>(); + final boolean[] hasWorkspaceRunfilesDirectory = {runfilesTree.getLegacyExternalRunfiles()}; + + visitSymlinkEntries( + runfilesTree, + /* rootSymlinks= */ false, + (rootRelativePath, files) -> { + hasWorkspaceRunfilesDirectory[0] |= + rootRelativePath.startsWith(workspaceRunfilesDirectory + "/"); + for (var file : files) { + runfiles.put(file.getPath(), file); + } + }); + + LinkedHashSet flattenedArtifacts = new LinkedHashSet<>(); + visitInputSet( + runfilesTree.getInputSetId(), + flattenedArtifacts::add, + // This is bug-for-bug compatible with the implementation in Runfiles by considering + // an empty non-external directory as a runfiles entry under the workspace runfiles + // directory even though it won't be materialized as one. + input -> hasWorkspaceRunfilesDirectory[0] |= hasWorkspaceRunfilesDirectory(input.path())); + flattenedArtifacts.stream() + .flatMap( + file -> + getRunfilesPaths(file.getPath(), runfilesTree.getLegacyExternalRunfiles()) + .map( + relativePath -> + file.toBuilder() + .setPath(runfilesTree.getPath() + "/" + relativePath) + .build())) + .forEach(file -> runfiles.put(file.getPath(), file)); + + for (String emptyFile : runfilesTree.getEmptyFilesList()) { + // Empty files are only created as siblings or parents of existing files, so they can't + // by themselves create a workspace runfiles directory if it wouldn't exist otherwise. + String newPath; + if (emptyFile.startsWith("../")) { + newPath = runfilesTree.getPath() + "/" + emptyFile.substring(3); + } else { + newPath = runfilesTree.getPath() + "/" + workspaceRunfilesDirectory + "/" + emptyFile; + } + runfiles.put(newPath, File.newBuilder().setPath(newPath).build()); + } + + visitSymlinkEntries( + runfilesTree, + /* rootSymlinks= */ true, + (rootRelativePath, files) -> { + hasWorkspaceRunfilesDirectory[0] |= + rootRelativePath.startsWith(workspaceRunfilesDirectory + "/"); + for (var file : files) { + runfiles.put(file.getPath(), file); + } + }); + + if (runfilesTree.hasRepoMappingManifest()) { + runfiles.put( + REPO_MAPPING_MANIFEST, + File.newBuilder() + .setPath(runfilesTree.getPath() + "/" + REPO_MAPPING_MANIFEST) + .setDigest(runfilesTree.getRepoMappingManifest().getDigest()) + .build()); + } + + if (!runfilesTree.getLegacyExternalRunfiles() && !hasWorkspaceRunfilesDirectory[0]) { + String dotRunfilePath = + "%s/%s/.runfile".formatted(runfilesTree.getPath(), workspaceRunfilesDirectory); + runfiles.put(dotRunfilePath, File.newBuilder().setPath(dotRunfilePath).build()); + } + // Copy to avoid retaining the entire runfiles map. + return new Input.Directory(runfilesTree.getPath(), ImmutableList.copyOf(runfiles.values())); + } + + @VisibleForTesting + static MatchResult extractRunfilesPath(String execPath, boolean siblingRepositoryLayout) { + Matcher matcher = + (siblingRepositoryLayout + ? SIBLING_LAYOUT_GENERATED_FILE_RUNFILES_PATH_PATTERN + : DEFAULT_GENERATED_FILE_RUNFILES_PATH_PATTERN) + .matcher(execPath); + if (matcher.matches()) { + return matcher; + } + matcher = + (siblingRepositoryLayout + ? SIBLING_LAYOUT_SOURCE_FILE_RUNFILES_PATH_PATTERN + : DEFAULT_SOURCE_FILE_RUNFILES_PATH_PATTERN) + .matcher(execPath); + checkState(matcher.matches()); + return matcher; } - private static T getFromMap(Map map, int id) throws IOException { - T value = map.get(id); - if (value == null) { - throw new IOException(String.format("referenced entry %d is missing or has wrong type", id)); + private boolean hasWorkspaceRunfilesDirectory(String path) { + return extractRunfilesPath(path, siblingRepositoryLayout).group("repo") == null; + } + + private Stream getRunfilesPaths(String execPath, boolean legacyExternalRunfiles) { + MatchResult matchResult = extractRunfilesPath(execPath, siblingRepositoryLayout); + String repo = matchResult.group("repo"); + String repoRelativePath = matchResult.group("path"); + if (repo == null) { + return Stream.of(workspaceRunfilesDirectory + "/" + repoRelativePath); + } else { + Stream.Builder paths = Stream.builder(); + paths.add(repo + "/" + repoRelativePath); + if (legacyExternalRunfiles) { + paths.add( + "%s/external/%s/%s".formatted(workspaceRunfilesDirectory, repo, repoRelativePath)); + } + return paths.build(); } - return value; + } + + private Collection reconstructRunfilesSymlinkTarget(String newPath, int targetId) + throws IOException { + if (targetId == 0) { + return ImmutableList.of(File.newBuilder().setPath(newPath).build()); + } + return switch (getInput(targetId)) { + case Input.File(File file) -> ImmutableList.of(file.toBuilder().setPath(newPath).build()); + case Input.Symlink(File symlink) -> + ImmutableList.of(symlink.toBuilder().setPath(newPath).build()); + case Input.Directory(String path, Collection files) -> + files.stream() + .map( + file -> + file.toBuilder() + .setPath(newPath + file.getPath().substring(path.length())) + .build()) + .collect(toImmutableList()); + }; + } + + private void putInput(int id, Input input) throws IOException { + putEntry(id, input); + } + + private void putInputSet(int id, ExecLogEntry.InputSet inputSet) throws IOException { + putEntry(id, inputSet); + } + + private void putSymlinkEntrySet(int id, ExecLogEntry.SymlinkEntrySet symlinkEntries) + throws IOException { + putEntry(id, symlinkEntries); + } + + private void putEntry(int id, Object entry) throws IOException { + if (id == 0) { + // The entry won't be referenced, so we don't need to store it. + return; + } + // Bazel emits consecutive non-zero IDs. + if (id != inputMap.size()) { + throw new IOException( + "ids must be consecutive, got %d after %d".formatted(id, inputMap.size())); + } + inputMap.add( + switch (entry) { + // Unwrap trivial wrappers to reduce retained memory usage. + case Input.File file -> file.file; + case Input.Symlink symlink -> symlink.symlink; + default -> entry; + }); + } + + private Input getInput(int id) throws IOException { + Object value = inputMap.get(id); + return switch (value) { + case Input input -> input; + case Protos.File file -> + file.getSymlinkTargetPath().isEmpty() ? new Input.File(file) : new Input.Symlink(file); + case null -> throw new IOException("referenced input %d is missing".formatted(id)); + default -> throw new IOException("entry %d is not an input: %s".formatted(id, value)); + }; + } + + private ExecLogEntry.InputSet getInputSet(int id) throws IOException { + Object value = inputMap.get(id); + return switch (value) { + case ExecLogEntry.InputSet inputSet -> inputSet; + case null -> throw new IOException("referenced input set %d is missing".formatted(id)); + default -> throw new IOException("entry %d is not an input set: %s".formatted(id, value)); + }; + } + + private ExecLogEntry.SymlinkEntrySet getSymlinkEntrySet(int id) throws IOException { + Object value = inputMap.get(id); + return switch (value) { + case ExecLogEntry.SymlinkEntrySet symlinkEntries -> symlinkEntries; + case null -> + throw new IOException("referenced set of symlink entries %d is missing".formatted(id)); + default -> + throw new IOException( + "entry %d is not a set of symlink entries: %s".formatted(id, value)); + }; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java index dfc9ba379d3b9d..49e95dd1080b37 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java @@ -152,7 +152,8 @@ public Object addEnv(StarlarkRuleContext ruleContext, String runfilesStr, Runfil runfiles, /* repoMappingManifest= */ null, ruleContext.getConfiguration().getRunfileSymlinksMode(), - ruleContext.getConfiguration().buildRunfileLinks()); + ruleContext.getConfiguration().buildRunfileLinks(), + /* runfilesMiddleman= */ null); } // TODO(rlevasseur): Remove once Starlark exposes this directly, see diff --git a/src/main/protobuf/spawn.proto b/src/main/protobuf/spawn.proto index f5c261dd7d5324..ebfaaef94b63eb 100644 --- a/src/main/protobuf/spawn.proto +++ b/src/main/protobuf/spawn.proto @@ -222,6 +222,16 @@ message ExecLogEntry { message Invocation { // The hash function used to compute digests. string hash_function_name = 1; + + // The name of the subdirectory of the runfiles tree corresponding to the + // main repository (also known as the "workspace name"). + // + // With --enable_bzlmod, this is always "_main", but can vary when using + // WORKSPACE. + string workspace_runfiles_directory = 2; + + // Whether --experimental_sibling_repository_layout is enabled. + bool sibling_repository_layout = 3; } // An input or output file. @@ -235,7 +245,7 @@ message ExecLogEntry { } // An input or output directory. - // May be a source directory, a runfiles or fileset tree, or a tree artifact. + // May be a source directory, a fileset tree, or a tree artifact. message Directory { // The directory path. string path = 1; @@ -252,34 +262,101 @@ message ExecLogEntry { } // A set of spawn inputs. - // The contents of the set are the directly referenced files, directories and - // symlinks in addition to the contents of all transitively referenced sets. + // The contents of the set are the directly contained entries in addition to + // the contents of all transitively referenced sets. When order matters, + // transitive sets come before direct entries and within a set, entries are + // considered in left-to-right order ("postorder"). // Sets are not canonical: two sets with different structure may yield the // same contents. message InputSet { - // Entry IDs of files belonging to this set. - repeated int32 file_ids = 1; - // Entry IDs of directories belonging to this set. - repeated int32 directory_ids = 2; - // Entry IDs of unresolved symlinks belonging to this set. - repeated int32 unresolved_symlink_ids = 3; - // Entry IDs of other sets contained in this set. - repeated int32 transitive_set_ids = 4; + // Entry IDs of files, directories, unresolved symlinks or runfiles trees + // belonging to this set. + repeated uint32 input_ids = 5; + // Entry IDs of other input sets contained in this set. + repeated uint32 transitive_set_ids = 4; + + reserved 1, 2, 3; + } + + // A collection of runfiles symlinked at custom locations. + // The contents of the set are the directly contained entries in addition to + // the contents of all transitively referenced sets. When order matters, + // transitive sets come before direct entries and within a set, entries are + // considered in left-to-right order ("postorder"). + // Sets are not canonical: two sets with different structure may yield the + // same contents. + message SymlinkEntrySet { + // A map from relative paths of runfiles symlinks to the entry IDs of the + // symlink target, which may be a file, directory, or unresolved symlink. + map direct_entries = 1; + // Entry IDs of other symlink entry sets transitively contained in this set. + repeated uint32 transitive_set_ids = 2; + } + + // A structured representation of the .runfiles directory of an executable. + // + // Instead of storing the directory directly, the tree is represented + // similarly to its in-memory representation in Bazel and needs to be + // reassembled from the following parts (in case of path collisions, later + // entries overwrite earlier ones): + // + // 1. symlinks (symlinks_id) + // 2. artifacts at canonical locations (input_set_id) + // 3. empty files (empty_files) + // 4. root symlinks (root_symlinks_id) + // 5. the _repo_mapping file with the repo mapping manifest + // (repo_mapping_manifest) + // 6. the /.runfile file (if the workspace + // runfiles directory + // wouldn't exist otherwise) + // + // See SpawnLogReconstructor#reconstructRunfilesDir for details. + message RunfilesTree { + // The runfiles tree path. + string path = 1; + // The entry ID of the set of artifacts in the runfiles tree that are + // symlinked at their canonical locations relative to the tree path. + // See SpawnLogReconstructor#getRunfilesPaths for how to recover the + // tree-relative paths of the artifacts from their exec paths. + // + // In case of path collisions, later artifacts overwrite earlier ones and + // artifacts override custom symlinks. + // + // The referenced set must not transitively contain any runfile trees. + uint32 input_set_id = 2; + // The entry ID of the set of symlink entries with paths relative to the + // subdirectory of the runfiles tree root corresponding to the main + // repository. + uint32 symlinks_id = 3; + // The entry ID of the set of symlink entries with paths relative to the + // root of the runfiles tree. + uint32 root_symlinks_id = 4; + // The paths of empty files relative to the subdirectory of the runfiles + // tree root corresponding to the main repository. + repeated string empty_files = 5; + // The "_repo_mapping" file at the root of the runfiles tree, if it exists. + // Only the digest is stored as the relative path is fixed. + File repo_mapping_manifest = 6; + // Whether the runfiles tree contains external runfiles at their legacy + // locations (e.g. _main/external/bazel_tools/tools/bash/runfiles.bash) + // in addition to the default locations (e.g. + // bazel_tools/tools/bash/runfiles.bash). + bool legacy_external_runfiles = 7; } // A spawn output. message Output { oneof type { - // An output file, i.e., ctx.actions.declare_file. - int32 file_id = 1; - // An output directory, i.e., ctx.actions.declare_directory. - int32 directory_id = 2; - // An output unresolved symlink, i.e., ctx.actions.declare_symlink. - int32 unresolved_symlink_id = 3; + // The ID of a file (ctx.actions.declare_file), directory + // (ctx.actions.declare_directory) or unresolved symlink + // (ctx.actions.declare_symlink) that is an output of the spawn. + uint32 output_id = 5; // A declared output that is either missing or has the wrong type // (e.g., a file where a directory was expected). string invalid_output_path = 4; } + + reserved 1, 2, 3; } // An executed spawn. @@ -294,10 +371,10 @@ message ExecLogEntry { Platform platform = 3; // Entry ID of the set of inputs. Unset means empty. - int32 input_set_id = 4; + uint32 input_set_id = 4; // Entry ID of the set of tool inputs. Unset means empty. - int32 tool_set_id = 5; + uint32 tool_set_id = 5; // The set of outputs. repeated Output outputs = 6; @@ -343,7 +420,8 @@ message ExecLogEntry { // A symlink action, which is not backed by a spawn. message SymlinkAction { - // The path of the input file of the action (i.e., the target of the symlink). + // The path of the input file of the action (i.e., the target of the + // symlink). string input_path = 1; // The path of the output file of the action (i.e., the symlink itself). @@ -356,8 +434,9 @@ message ExecLogEntry { string mnemonic = 4; } - // The entry ID. Must be nonzero. - int32 id = 1; + // If nonzero, then this entry may be referenced by later entries by this ID. + // Nonzero IDs are unique within an execution log, but may not be contiguous. + uint32 id = 1; // The entry payload. oneof type { @@ -368,5 +447,7 @@ message ExecLogEntry { InputSet input_set = 6; Spawn spawn = 7; SymlinkAction symlink_action = 8; + SymlinkEntrySet symlink_entry_set = 9; + RunfilesTree runfiles_tree = 10; } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java index 430f542096b50e..1fa3cadc13c3e7 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java @@ -649,7 +649,8 @@ public ImmutableList getExtraPaths( public void fingerprint(Fingerprint fingerprint) {} }) .build(); - assertThat(runfiles.getEmptyFilenames().toList()) - .containsExactly("my-artifact-empty", "my-symlink-empty"); + assertThat(runfiles.getEmptyFilenames()) + .containsExactly( + PathFragment.create("my-artifact-empty"), PathFragment.create("my-symlink-empty")); } } 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 edbc789a828d66..a1e08100594907 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 @@ -58,7 +58,8 @@ public void testGetArtifactsWithSingleMapping() { mkRunfiles(artifacts), /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); assertThat(underTest.getArtifacts().toList()).containsExactlyElementsIn(artifacts); } @@ -71,7 +72,8 @@ public void withOverriddenRunfilesDir() { Runfiles.EMPTY, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); PathFragment newDir = PathFragment.create("new"); RunfilesSupplier overridden = original.withOverriddenRunfilesDir(newDir); @@ -91,7 +93,8 @@ public void withOverriddenRunfilesDir_noChange_sameObject() { Runfiles.EMPTY, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); assertThat(original.withOverriddenRunfilesDir(dir)).isSameInstanceAs(original); } @@ -105,7 +108,8 @@ public void cachedMappings() { runfiles, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); Map> mappings1 = underTest.getMappings(); Map> mappings2 = underTest.getMappings(); @@ -126,7 +130,8 @@ public void cachedMappings_sharedAcrossDirOverrides() { runfiles, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); SingleRunfilesSupplier overridden = original.withOverriddenRunfilesDir(newDir); Map> mappingsOld = original.getMappings(); 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 e90c2183fb71b0..c5fd625547bc0c 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 @@ -377,7 +377,8 @@ public void testInputManifestsRemovedIfSupplied() throws Exception { Runfiles.EMPTY, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false)) + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null)) .addOutput(getBinArtifactWithNoOwner("output")) .setExecutable(scratch.file("/bin/xxx").asFragment()) .setProgressMessage("Test") @@ -581,7 +582,8 @@ private static RunfilesSupplier runfilesSupplier(PathFragment dir) { Runfiles.EMPTY, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); } private ActionOwner nullOwnerWithTargetConfig() { 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 3d96e98208a175..a7d4158ebdc71e 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 @@ -558,7 +558,8 @@ public static RunfilesSupplier createRunfilesSupplier( runfiles, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null); } public static BuildOptions execOptions(BuildOptions targetOptions, EventHandler handler) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD index 56a29f76da0043..3dd0bc93a93e74 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD @@ -70,6 +70,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment", "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_transition", + "//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry", "//src/main/java/com/google/devtools/build/lib/analysis:target_and_configuration", "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report_action_factory", "//src/main/java/com/google/devtools/build/lib/analysis:test/instrumented_files_info", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java b/src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BUILD index 3c56f6513f7b87..4e78f9ff8d9094 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BUILD @@ -27,6 +27,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/rules/python", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java index 7921a6e9b53e62..93b92847cd32d7 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.regex.Pattern; import org.junit.Test; import org.junit.runner.RunWith; @@ -344,8 +345,7 @@ public void explicitInitPy_CanBeGloballyEnabled() throws Exception { " srcs = ['foo.py'],", ")")); useConfiguration("--incompatible_default_to_explicit_init_py=true"); - assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames().toList()) - .isEmpty(); + assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames()).isEmpty(); } @Test @@ -359,8 +359,8 @@ public void explicitInitPy_CanBeSelectivelyDisabled() throws Exception { " legacy_create_init = True,", ")")); useConfiguration("--incompatible_default_to_explicit_init_py=true"); - assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames().toList()) - .containsExactly("pkg/__init__.py"); + assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames()) + .containsExactly(PathFragment.create("pkg/__init__.py")); } @Test @@ -373,8 +373,8 @@ public void explicitInitPy_CanBeGloballyDisabled() throws Exception { " srcs = ['foo.py'],", ")")); useConfiguration("--incompatible_default_to_explicit_init_py=false"); - assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames().toList()) - .containsExactly("pkg/__init__.py"); + assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames()) + .containsExactly(PathFragment.create("pkg/__init__.py")); } @Test @@ -388,8 +388,7 @@ public void explicitInitPy_CanBeSelectivelyEnabled() throws Exception { " legacy_create_init = False,", ")")); useConfiguration("--incompatible_default_to_explicit_init_py=false"); - assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames().toList()) - .isEmpty(); + assertThat(getDefaultRunfiles(getConfiguredTarget("//pkg:foo")).getEmptyFilenames()).isEmpty(); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD index 8dddd43c18044a..3ac715bcdd5359 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -35,7 +35,14 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", + "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options", + "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", + "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_factory", + "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_registry", + "//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", + "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", + "//src/main/java/com/google/devtools/build/lib/bazel/rules/python", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/cmdline", @@ -83,6 +90,7 @@ java_library( "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/exec/util", "//src/test/java/com/google/devtools/build/lib/testutil", + "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", "//third_party:guava", "//third_party:jsr305", "//third_party:junit4", diff --git a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java index 07d55d3400d79d..dd89d33da6cc43 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.exec; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; +import static com.google.devtools.build.lib.testutil.TestConstants.PRODUCT_NAME; import com.github.luben.zstd.ZstdInputStream; import com.google.common.collect.ImmutableList; @@ -22,18 +24,24 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BuildConfigurationEvent; +import com.google.devtools.build.lib.actions.InputMetadataProvider; +import com.google.devtools.build.lib.actions.RunfilesSupplier; +import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.exec.Protos.File; import com.google.devtools.build.lib.exec.Protos.SpawnExec; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.remote.options.RemoteOptions; +import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.common.options.Options; import com.google.testing.junit.testparameterinjector.TestParameter; @@ -143,12 +151,13 @@ public void testSymlinkAction() throws IOException, InterruptedException { assertThat(entries) .containsExactly( Protos.ExecLogEntry.newBuilder() - .setId(1) .setInvocation( - Protos.ExecLogEntry.Invocation.newBuilder().setHashFunctionName("SHA-256")) + Protos.ExecLogEntry.Invocation.newBuilder() + .setHashFunctionName("SHA-256") + .setWorkspaceRunfilesDirectory(TestConstants.WORKSPACE_NAME) + .setSiblingRepositoryLayout(siblingRepositoryLayout)) .build(), Protos.ExecLogEntry.newBuilder() - .setId(2) .setSymlinkAction( Protos.ExecLogEntry.SymlinkAction.newBuilder() .setInputPath("source") @@ -158,6 +167,91 @@ public void testSymlinkAction() throws IOException, InterruptedException { .build()); } + @Test + public void testRunfilesTreeReusedForTool() throws Exception { + Artifact tool = ActionsTestUtil.createArtifact(rootDir, "data.txt"); + writeFile(tool, "abc"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "middleman"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + /* legacyExternalRunfiles= */ false, + NestedSetBuilder.wrap(Order.COMPILE_ORDER, ImmutableList.of(tool))); + + Artifact firstInput = ActionsTestUtil.createArtifact(rootDir, "first_input"); + writeFile(firstInput, "def"); + InputMetadataProvider firstInputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, firstInput, tool); + Artifact secondInput = ActionsTestUtil.createArtifact(rootDir, "second_input"); + writeFile(secondInput, "ghi"); + InputMetadataProvider secondInputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, secondInput, tool); + + Spawn firstSpawn = + defaultSpawnBuilder() + .withRunfilesSupplier(runfilesSupplier) + .withInputs(firstInput) + .withTool(runfilesMiddleman) + .build(); + Spawn secondSpawn = + defaultSpawnBuilder() + .withRunfilesSupplier(runfilesSupplier) + .withInputs(secondInput) + .withTool(runfilesMiddleman) + .build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + firstSpawn, + firstInputMetadataProvider, + createInputMap(runfilesSupplier, firstInputMetadataProvider, firstInput), + fs, + defaultTimeout(), + defaultSpawnResult()); + context.logSpawn( + secondSpawn, + secondInputMetadataProvider, + createInputMap(runfilesSupplier, secondInputMetadataProvider, secondInput), + fs, + defaultTimeout(), + defaultSpawnResult()); + + var entries = closeAndReadCompactLog(context); + assertThat(entries.stream().filter(Protos.ExecLogEntry::hasRunfilesTree)).hasSize(1); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/_main/data.txt") + .setDigest(getDigest("abc")) + .setIsTool(true)) + .addInputs( + File.newBuilder() + .setPath("first_input") + .setDigest(getDigest("def")) + .setIsTool(false)) + .build(), + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/_main/data.txt") + .setDigest(getDigest("abc")) + .setIsTool(true)) + .addInputs( + File.newBuilder() + .setPath("second_input") + .setDigest(getDigest("ghi")) + .setIsTool(false)) + .build()); + } + @Override protected SpawnLogContext createSpawnLogContext(ImmutableMap platformProperties) throws IOException, InterruptedException { @@ -167,6 +261,8 @@ protected SpawnLogContext createSpawnLogContext(ImmutableMap pla return new CompactSpawnLogContext( logPath, execRoot.asFragment(), + TestConstants.WORKSPACE_NAME, + siblingRepositoryLayout, remoteOptions, DigestHashFunction.SHA256, SyscallCache.NO_CACHE); @@ -188,4 +284,19 @@ protected void closeAndAssertLog(SpawnLogContext context, SpawnExec... expected) assertThat(actual).containsExactlyElementsIn(expected).inOrder(); } + + private ImmutableList closeAndReadCompactLog(SpawnLogContext context) + throws IOException { + context.close(); + + ImmutableList.Builder entries = ImmutableList.builder(); + try (InputStream in = logPath.getInputStream(); + ZstdInputStream zstdIn = new ZstdInputStream(in)) { + Protos.ExecLogEntry entry; + while ((entry = Protos.ExecLogEntry.parseDelimitedFrom(zstdIn)) != null) { + entries.add(entry); + } + } + return entries.build(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java index d9dc9cde3be16d..7af01adb5d6669 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java @@ -14,44 +14,66 @@ package com.google.devtools.build.lib.exec; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.exec.SpawnLogContext.millisToProto; +import static com.google.devtools.build.lib.testutil.TestConstants.PRODUCT_NAME; +import static com.google.devtools.build.lib.testutil.TestConstants.WORKSPACE_NAME; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static java.util.Comparator.comparing; import com.google.common.base.Utf8; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.actions.ActionEnvironment; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactRoot; -import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.InputMetadataProvider; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnMetrics; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; -import com.google.devtools.build.lib.actions.StaticInputMetadataProvider; -import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.analysis.Runfiles; +import com.google.devtools.build.lib.analysis.ServerDirectories; +import com.google.devtools.build.lib.analysis.SingleRunfilesSupplier; +import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.config.CoreOptions; +import com.google.devtools.build.lib.analysis.config.FragmentFactory; +import com.google.devtools.build.lib.analysis.config.FragmentRegistry; +import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.bazel.rules.python.BazelPythonSemantics; +import com.google.devtools.build.lib.cmdline.LabelConstants; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.exec.Protos.Digest; import com.google.devtools.build.lib.exec.Protos.EnvironmentVariable; import com.google.devtools.build.lib.exec.Protos.File; import com.google.devtools.build.lib.exec.Protos.Platform; import com.google.devtools.build.lib.exec.Protos.SpawnExec; +import com.google.devtools.build.lib.exec.util.FakeActionInputFileCache; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.server.FailureDetails.Crash; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; +import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.vfs.DelegateFileSystem; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.Dirent; @@ -61,25 +83,85 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import com.google.devtools.common.options.OptionsParsingException; import com.google.protobuf.util.Timestamps; import com.google.testing.junit.testparameterinjector.TestParameter; +import com.google.testing.junit.testparameterinjector.TestParameterInjector; import java.io.IOException; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; -import java.util.HashMap; import java.util.Map; import java.util.SortedMap; +import java.util.TreeMap; +import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; /** Base class for {@link SpawnLogContext} tests. */ +@RunWith(TestParameterInjector.class) public abstract class SpawnLogContextTestBase { protected final DigestHashFunction digestHashFunction = DigestHashFunction.SHA256; protected final FileSystem fs = new InMemoryFileSystem(digestHashFunction); - protected final Path execRoot = fs.getPath("/execroot"); - protected final ArtifactRoot rootDir = ArtifactRoot.asSourceRoot(Root.fromPath(execRoot)); - protected final ArtifactRoot outputDir = - ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "out"); + protected final Path outputBase = fs.getPath("/home/user/bazel/output_base"); + protected final Path externalRoot = + outputBase.getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION); + protected final RepositoryName externalRepo = RepositoryName.createUnvalidated("some_repo"); + + protected ArtifactRoot outputDir; + protected Path execRoot; + protected ArtifactRoot rootDir; + protected ArtifactRoot middlemanDir; + protected ArtifactRoot externalSourceRoot; + protected ArtifactRoot externalOutputDir; + protected BuildConfigurationValue configuration; + + @TestParameter public boolean siblingRepositoryLayout; + + @Before + public void setup() throws InvalidConfigurationException, OptionsParsingException { + BuildOptions defaultBuildOptions = BuildOptions.of(ImmutableList.of(CoreOptions.class)); + configuration = + BuildConfigurationValue.createForTesting( + defaultBuildOptions, + "k8-fastbuild", + TestConstants.WORKSPACE_NAME, + siblingRepositoryLayout, + new BlazeDirectories( + new ServerDirectories(outputBase, outputBase, outputBase), + /* workspace= */ null, + /* defaultSystemJavabase= */ null, + TestConstants.PRODUCT_NAME), + new BuildConfigurationValue.GlobalStateProvider() { + @Override + public ActionEnvironment getActionEnvironment(BuildOptions buildOptions) { + return ActionEnvironment.EMPTY; + } + + @Override + public FragmentRegistry getFragmentRegistry() { + return FragmentRegistry.create( + ImmutableList.of(), ImmutableList.of(), ImmutableList.of()); + } + + @Override + public ImmutableSet getReservedActionMnemonics() { + return ImmutableSet.of(); + } + }, + new FragmentFactory()); + outputDir = configuration.getBinDirectory(RepositoryName.MAIN); + middlemanDir = configuration.getMiddlemanDirectory(RepositoryName.MAIN); + execRoot = configuration.getDirectories().getExecRoot(TestConstants.WORKSPACE_NAME); + rootDir = ArtifactRoot.asSourceRoot(Root.fromPath(execRoot)); + + externalSourceRoot = + ArtifactRoot.asExternalSourceRoot( + Root.fromPath(externalRoot.getChild(externalRepo.getName()))); + externalOutputDir = configuration.getBinDirectory(externalRepo); + } // A fake action filesystem that provides a fast digest, but refuses to compute it from the // file contents (which won't be available when building without the bytes). @@ -288,7 +370,7 @@ public void testTreeInput( ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/tree/child") + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/tree/child") .setDigest(getDigest("abc")) .setIsTool(inputsMode.isTool()) .build())) @@ -322,30 +404,36 @@ public void testUnresolvedSymlinkInput(@TestParameter InputsMode inputsMode) thr defaultSpawnExecBuilder() .addInputs( File.newBuilder() - .setPath("out/symlink") + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/symlink") .setSymlinkTargetPath("/some/path") .setIsTool(inputsMode.isTool())) .build()); } @Test - public void testRunfilesFileInput() throws Exception { + public void testRunfilesFileInput(@TestParameter InputsMode inputsMode) throws Exception { + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "data.txt"); writeFile(runfilesInput, "abc"); PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); RunfilesSupplier runfilesSupplier = - createRunfilesSupplier(runfilesRoot, ImmutableMap.of("data.txt", runfilesInput)); + createRunfilesSupplier(runfilesMiddleman, runfilesRoot, runfilesInput); - Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + SpawnBuilder spawnBuilder = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier); + if (inputsMode.isTool()) { + spawnBuilder.withTool(runfilesMiddleman); + } SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, runfilesInput); context.logSpawn( - spawn, - createInputMetadataProvider(runfilesInput), - createInputMap(runfilesSupplier), + spawnBuilder.build(), + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), fs, defaultTimeout(), defaultSpawnResult()); @@ -354,12 +442,22 @@ public void testRunfilesFileInput() throws Exception { context, defaultSpawnExecBuilder() .addInputs( - File.newBuilder().setPath("out/foo.runfiles/data.txt").setDigest(getDigest("abc"))) + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/data.txt") + .setDigest(getDigest("abc")) + .setIsTool(inputsMode.isTool())) .build()); } @Test - public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) throws Exception { + public void testRunfilesDirectoryInput( + @TestParameter DirContents dirContents, @TestParameter InputsMode inputsMode) + throws Exception { + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "dir"); runfilesInput.getPath().createDirectoryAndParents(); @@ -369,16 +467,21 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); RunfilesSupplier runfilesSupplier = - createRunfilesSupplier(runfilesRoot, ImmutableMap.of("dir", runfilesInput)); + createRunfilesSupplier(runfilesMiddleman, runfilesRoot, runfilesInput); - Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + SpawnBuilder spawnBuilder = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier); + if (inputsMode.isTool()) { + spawnBuilder.withTool(runfilesMiddleman); + } SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, runfilesInput); context.logSpawn( - spawn, - createInputMetadataProvider(runfilesInput), - createInputMap(runfilesSupplier), + spawnBuilder.build(), + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), fs, defaultTimeout(), defaultSpawnResult()); @@ -391,35 +494,992 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/foo.runfiles/dir/data.txt") + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/dir/data.txt") .setDigest(getDigest("abc")) + .setIsTool(inputsMode.isTool()) .build())) .build()); } @Test - public void testRunfilesEmptyInput() throws Exception { + public void testRunfilesEmptyInput(@TestParameter InputsMode inputsMode) throws Exception { + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "sub/dir/script.py"); + writeFile(runfilesInput, "abc"); + PackageIdentifier someRepoPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("pkg")); + Artifact externalSourceArtifact = + ActionsTestUtil.createArtifact( + externalSourceRoot, + someRepoPkg.getExecPath(siblingRepositoryLayout).getChild("lib.py").getPathString()); + writeFile(externalSourceArtifact, "external_source"); + PackageIdentifier someRepoOtherPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("other/pkg")); + Artifact externalGenArtifact = + ActionsTestUtil.createArtifact( + externalOutputDir, + someRepoOtherPkg + .getPackagePath(siblingRepositoryLayout) + .getChild("gen.py") + .getPathString()); + writeFile(externalGenArtifact, "external_gen"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); - HashMap mapping = new HashMap<>(); - mapping.put("__init__.py", null); - RunfilesSupplier runfilesSupplier = createRunfilesSupplier(runfilesRoot, mapping); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + runfilesInput, + externalGenArtifact, + externalSourceArtifact); + + SpawnBuilder spawnBuilder = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier); + if (inputsMode.isTool()) { + spawnBuilder.withTool(runfilesMiddleman); + } + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider( + runfilesSupplier, + runfilesInput, + externalGenArtifact, + externalSourceArtifact); + + context.logSpawn( + spawnBuilder.build(), + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/sub/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/sub/dir/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/sub/dir/script.py") + .setDigest(getDigest("abc")) + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/gen.py") + .setDigest(getDigest("external_gen")) + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/__init__.py") + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/lib.py") + .setDigest(getDigest("external_source")) + .setIsTool(inputsMode.isTool())) + .build()); + } + + @Test + public void testRunfilesMixedRoots(@TestParameter boolean legacyExternalRunfiles) + throws Exception { + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + Artifact sourceArtifact = ActionsTestUtil.createArtifact(rootDir, "pkg/source.txt"); + writeFile(sourceArtifact, "source"); + Artifact genArtifact = ActionsTestUtil.createArtifact(outputDir, "other/pkg/gen.txt"); + writeFile(genArtifact, "gen"); + PackageIdentifier someRepoPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("pkg")); + Artifact externalSourceArtifact = + ActionsTestUtil.createArtifact( + externalSourceRoot, + someRepoPkg + .getExecPath(siblingRepositoryLayout) + .getChild("source.txt") + .getPathString()); + writeFile(externalSourceArtifact, "external_source"); + PackageIdentifier someRepoOtherPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("other/pkg")); + Artifact externalGenArtifact = + ActionsTestUtil.createArtifact( + externalOutputDir, + someRepoOtherPkg + .getPackagePath(siblingRepositoryLayout) + .getChild("gen.txt") + .getPathString()); + writeFile(externalGenArtifact, "external_gen"); + + Artifact symlinkSourceTarget = ActionsTestUtil.createArtifact(rootDir, "pkg/target.txt"); + writeFile(symlinkSourceTarget, "symlink_source"); + Artifact symlinkGenTarget = ActionsTestUtil.createArtifact(outputDir, "pkg/target.txt"); + writeFile(symlinkGenTarget, "symlink_gen"); + + Artifact rootSymlinkSourceTarget = + ActionsTestUtil.createArtifact(rootDir, "pkg/root_target.txt"); + writeFile(rootSymlinkSourceTarget, "root_symlink_source"); + Artifact rootSymlinkGenTarget = + ActionsTestUtil.createArtifact(outputDir, "pkg/root_target.txt"); + writeFile(rootSymlinkGenTarget, "root_symlink_gen"); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of( + "some/symlink", symlinkSourceTarget, + "other/symlink", symlinkGenTarget), + ImmutableMap.of( + "root/symlink", rootSymlinkSourceTarget, + "root/other/symlink", rootSymlinkGenTarget), + legacyExternalRunfiles, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact); Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider( + runfilesSupplier, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact, + symlinkSourceTarget, + symlinkGenTarget, + rootSymlinkSourceTarget, + rootSymlinkGenTarget); context.logSpawn( spawn, - createInputMetadataProvider(), - createInputMap(runfilesSupplier), + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), fs, defaultTimeout(), defaultSpawnResult()); + var builder = defaultSpawnExecBuilder(); + if (legacyExternalRunfiles) { + builder + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source"))); + } + builder + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/other/pkg/gen.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/other/symlink") + .setDigest(getDigest("symlink_gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/source.txt") + .setDigest(getDigest("source"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/some/symlink") + .setDigest(getDigest("symlink_source"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + "-out/k8-fastbuild/bin/tools/foo.runfiles/root/other/symlink") + .setDigest(getDigest("root_symlink_gen"))) + .addInputs( + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/tools/foo.runfiles/root/symlink") + .setDigest(getDigest("root_symlink_source"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source"))); + closeAndAssertLog(context, builder.build()); + } + + @Test + public void testRunfilesExternalOnly( + @TestParameter boolean legacyExternalRunfiles, + @TestParameter boolean symlinkUnderMain, + @TestParameter boolean rootSymlinkUnderMain) + throws Exception { + PackageIdentifier someRepoPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("pkg")); + Artifact externalSourceArtifact = + ActionsTestUtil.createArtifact( + externalSourceRoot, + someRepoPkg + .getExecPath(siblingRepositoryLayout) + .getChild("source.txt") + .getPathString()); + writeFile(externalSourceArtifact, "external_source"); + PackageIdentifier someRepoOtherPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("other/pkg")); + Artifact externalGenArtifact = + ActionsTestUtil.createArtifact( + externalOutputDir, + someRepoOtherPkg + .getPackagePath(siblingRepositoryLayout) + .getChild("gen.txt") + .getPathString()); + writeFile(externalGenArtifact, "external_gen"); + + Artifact symlinkTarget = ActionsTestUtil.createArtifact(outputDir, "pkg/root_target.txt"); + writeFile(symlinkTarget, "symlink_target"); + Artifact rootSymlinkTarget = ActionsTestUtil.createArtifact(rootDir, "pkg/root_target.txt"); + writeFile(rootSymlinkTarget, "root_symlink_target"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of((symlinkUnderMain ? "" : "../some_repo/") + "symlink", symlinkTarget), + ImmutableMap.of( + (rootSymlinkUnderMain ? WORKSPACE_NAME + "/" : "some_repo/") + "root_symlink", + rootSymlinkTarget), + legacyExternalRunfiles, + externalSourceArtifact, + externalGenArtifact); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider( + runfilesSupplier, + externalSourceArtifact, + externalGenArtifact, + symlinkTarget, + rootSymlinkTarget); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + ArrayList files = new ArrayList<>(); + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/%s/root_symlink" + .formatted(rootSymlinkUnderMain ? WORKSPACE_NAME : "some_repo")) + .setDigest(getDigest("root_symlink_target")) + .build()); + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/%s/symlink" + .formatted(symlinkUnderMain ? WORKSPACE_NAME : "some_repo")) + .setDigest(getDigest("symlink_target")) + .build()); + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen")) + .build()); + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source")) + .build()); + if (legacyExternalRunfiles) { + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen")) + .build()); + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source")) + .build()); + if (!symlinkUnderMain) { + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + "" + + WORKSPACE_NAME + + "/external/some_repo/symlink") + .setDigest(getDigest("symlink_target")) + .build()); + } + } else if (!symlinkUnderMain && !rootSymlinkUnderMain) { + files.add( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/.runfile") + .build()); + } closeAndAssertLog( context, defaultSpawnExecBuilder() - .addInputs(File.newBuilder().setPath("out/foo.runfiles/__init__.py")) + .addAllInputs(files.stream().sorted(comparing(File::getPath)).toList()) + .build()); + } + + @Test + public void testRunfilesFilesCollide(@TestParameter boolean legacyExternalRunfiles) + throws Exception { + Artifact sourceArtifact = ActionsTestUtil.createArtifact(rootDir, "pkg/file.txt"); + writeFile(sourceArtifact, "source"); + Artifact genArtifact = ActionsTestUtil.createArtifact(outputDir, "pkg/file.txt"); + writeFile(genArtifact, "gen"); + PackageIdentifier someRepoPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("pkg")); + Artifact externalSourceArtifact = + ActionsTestUtil.createArtifact( + externalSourceRoot, + someRepoPkg.getExecPath(siblingRepositoryLayout).getChild("file.txt").getPathString()); + writeFile(externalSourceArtifact, "external_source"); + Artifact externalGenArtifact = + ActionsTestUtil.createArtifact( + externalOutputDir, + someRepoPkg + .getPackagePath(siblingRepositoryLayout) + .getChild("file.txt") + .getPathString()); + writeFile(externalGenArtifact, "external_gen"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + legacyExternalRunfiles, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider( + runfilesSupplier, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + var builder = defaultSpawnExecBuilder(); + if (legacyExternalRunfiles) { + builder.addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/pkg/file.txt") + .setDigest(getDigest("external_gen"))); + } + builder + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/file.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/pkg/file.txt") + .setDigest(getDigest("external_gen"))); + closeAndAssertLog(context, builder.build()); + } + + @Test + public void testRunfilesFilesAndSymlinksCollide(@TestParameter boolean legacyExternalRunfiles) + throws Exception { + Artifact sourceArtifact = ActionsTestUtil.createArtifact(rootDir, "pkg/source.txt"); + writeFile(sourceArtifact, "source"); + Artifact genArtifact = ActionsTestUtil.createArtifact(outputDir, "other/pkg/gen.txt"); + writeFile(genArtifact, "gen"); + PackageIdentifier someRepoPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("pkg")); + Artifact externalSourceArtifact = + ActionsTestUtil.createArtifact( + externalSourceRoot, + someRepoPkg + .getExecPath(siblingRepositoryLayout) + .getChild("source.txt") + .getPathString()); + writeFile(externalSourceArtifact, "external_source"); + PackageIdentifier someRepoOtherPkg = + PackageIdentifier.create(externalRepo, PathFragment.create("other/pkg")); + Artifact externalGenArtifact = + ActionsTestUtil.createArtifact( + externalOutputDir, + someRepoOtherPkg + .getPackagePath(siblingRepositoryLayout) + .getChild("gen.txt") + .getPathString()); + writeFile(externalGenArtifact, "external_gen"); + + Artifact symlinkSourceArtifact = ActionsTestUtil.createArtifact(rootDir, "pkg/not_source.txt"); + writeFile(symlinkSourceArtifact, "symlink_source"); + Artifact symlinkGenArtifact = + ActionsTestUtil.createArtifact(outputDir, "other/pkg/not_gen.txt"); + writeFile(symlinkGenArtifact, "symlink_gen"); + Artifact symlinkExternalSourceArtifact = + ActionsTestUtil.createArtifact(externalSourceRoot, "external/some_repo/pkg/not_source.txt"); + writeFile(symlinkExternalSourceArtifact, "symlink_external_source"); + Artifact symlinkExternalGenArtifact = + ActionsTestUtil.createArtifact(outputDir, "external/some_repo/other/pkg/not_gen.txt"); + writeFile(symlinkExternalGenArtifact, "symlink_external_gen"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of( + // Symlinks are always relative to the workspace runfiles directory. + "pkg/source.txt", symlinkSourceArtifact, + "other/pkg/gen.txt", symlinkGenArtifact, + "../some_repo/pkg/source.txt", symlinkExternalSourceArtifact, + "../some_repo/other/pkg/gen.txt", symlinkExternalGenArtifact), + ImmutableMap.of(), + legacyExternalRunfiles, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider( + runfilesSupplier, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact, + symlinkSourceArtifact, + symlinkGenArtifact, + symlinkExternalSourceArtifact, + symlinkExternalGenArtifact); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + var builder = defaultSpawnExecBuilder(); + if (legacyExternalRunfiles) { + builder + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/external/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source"))); + } + builder + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/other/pkg/gen.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/source.txt") + .setDigest(getDigest("source"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source"))); + closeAndAssertLog(context, builder.build()); + } + + @Test + public void testRunfilesFileAndRootSymlinkCollide() throws Exception { + Artifact sourceArtifact = ActionsTestUtil.createArtifact(rootDir, "pkg/source.txt"); + writeFile(sourceArtifact, "source"); + + Artifact symlinkSourceArtifact = ActionsTestUtil.createArtifact(rootDir, "pkg/not_source.txt"); + writeFile(symlinkSourceArtifact, "symlink_source"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(WORKSPACE_NAME + "/pkg/source.txt", symlinkSourceArtifact), + /* legacyExternalRunfiles= */ false, + sourceArtifact); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, sourceArtifact, symlinkSourceArtifact); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/source.txt") + .setDigest(getDigest("symlink_source"))) + .build()); + } + + @Test + public void testRunfilesCrossTypeCollision(@TestParameter boolean symlinkFirst) throws Exception { + Artifact file = ActionsTestUtil.createArtifact(rootDir, "pkg/file.txt"); + writeFile(file, "file"); + Artifact symlink = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputDir, "pkg/file.txt"); + symlink.getPath().getParentDirectory().createDirectoryAndParents(); + symlink.getPath().createSymbolicLink(PathFragment.create("/some/path/other_file.txt")); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + var artifacts = + symlinkFirst ? ImmutableList.of(symlink, file) : ImmutableList.of(file, symlink); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + /* legacyExternalRunfiles= */ false, + NestedSetBuilder.wrap(Order.STABLE_ORDER, artifacts)); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, file, symlink); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + symlinkFirst + ? File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/file.txt") + .setDigest(getDigest("file")) + : File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/file.txt") + .setSymlinkTargetPath("/some/path/other_file.txt")) + .build()); + } + + @Test + public void testRunfilesPostOrderCollision(@TestParameter boolean nestBoth) throws Exception { + Artifact sourceFile = ActionsTestUtil.createArtifact(rootDir, "pkg/file.txt"); + writeFile(sourceFile, "source"); + Artifact genFile = ActionsTestUtil.createArtifact(outputDir, "pkg/file.txt"); + writeFile(genFile, "gen"); + Artifact otherSourceFile = ActionsTestUtil.createArtifact(rootDir, "pkg/other_file.txt"); + writeFile(otherSourceFile, "other_source"); + Artifact otherGenFile = ActionsTestUtil.createArtifact(outputDir, "pkg/other_file.txt"); + writeFile(otherGenFile, "other_gen"); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + var artifactsBuilder = + NestedSetBuilder.compileOrder() + .addTransitive( + NestedSetBuilder.wrap( + Order.COMPILE_ORDER, ImmutableList.of(sourceFile, otherGenFile))); + var remainingArtifacts = ImmutableList.of(genFile, otherSourceFile); + if (nestBoth) { + artifactsBuilder.addTransitive( + NestedSetBuilder.wrap(Order.COMPILE_ORDER, remainingArtifacts)); + } else { + artifactsBuilder.addAll(remainingArtifacts); + } + var artifacts = artifactsBuilder.build(); + assertThat(artifacts.toList()) + .containsExactly(sourceFile, otherGenFile, genFile, otherSourceFile) + .inOrder(); + if (nestBoth) { + assertThat(artifacts.getNonLeaves()).hasSize(2); + } + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + /* legacyExternalRunfiles= */ false, + artifacts); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider( + runfilesSupplier, + sourceFile, + genFile, + otherSourceFile, + otherGenFile); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/file.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/other_file.txt") + .setDigest(getDigest("other_source"))) + .build()); + } + + @Test + public void testRunfilesSymlinkTargets( + @TestParameter boolean rootSymlinks, @TestParameter InputsMode inputsMode) throws Exception { + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + Artifact sourceFile = ActionsTestUtil.createArtifact(rootDir, "pkg/file.txt"); + writeFile(sourceFile, "source"); + Artifact sourceDir = ActionsTestUtil.createArtifact(rootDir, "pkg/source_dir"); + sourceDir.getPath().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1( + sourceDir.getPath().getRelative("some_file"), "source_dir_file"); + Artifact genDir = + ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputDir, "pkg/gen_dir"); + genDir.getPath().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1( + genDir.getPath().getRelative("other_file"), "gen_dir_file"); + Artifact symlink = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputDir, "pkg/symlink"); + symlink.getPath().getParentDirectory().createDirectoryAndParents(); + symlink.getPath().createSymbolicLink(PathFragment.create("/some/path")); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + rootSymlinks + ? ImmutableMap.of() + : ImmutableMap.of( + "file", sourceFile, + "source_dir", sourceDir, + "gen_dir", genDir, + "symlink", symlink), + rootSymlinks + ? ImmutableMap.of( + WORKSPACE_NAME + "/file", sourceFile, + WORKSPACE_NAME + "/source_dir", sourceDir, + WORKSPACE_NAME + "/gen_dir", genDir, + WORKSPACE_NAME + "/symlink", symlink) + : ImmutableMap.of(), + /* legacyExternalRunfiles= */ false); + + SpawnBuilder spawnBuilder = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier); + if (inputsMode.isTool()) { + spawnBuilder.withTool(runfilesMiddleman); + } + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, sourceFile, sourceDir, genDir, symlink); + + context.logSpawn( + spawnBuilder.build(), + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/file") + .setDigest(getDigest("source")) + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/gen_dir/other_file") + .setDigest(getDigest("gen_dir_file")) + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/source_dir/some_file") + .setDigest(getDigest("source_dir_file")) + .setIsTool(inputsMode.isTool())) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/symlink") + .setSymlinkTargetPath("/some/path") + .setIsTool(inputsMode.isTool())) + .build()); + } + + @Test + public void testRunfileSymlinkFileWithDirectoryContents( + @TestParameter boolean rootSymlink, @TestParameter OutputsMode outputsMode) throws Exception { + Artifact genFile = ActionsTestUtil.createArtifact(outputDir, "pkg/file.txt"); + genFile.getPath().createDirectoryAndParents(); + writeFile(genFile.getPath().getChild("file"), "abc"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesSupplier runfilesSupplier = + createRunfilesSupplier( + runfilesMiddleman, + runfilesRoot, + rootSymlink ? ImmutableMap.of() : ImmutableMap.of("pkg/symlink", genFile), + rootSymlink + ? ImmutableMap.of(WORKSPACE_NAME + "/pkg/symlink", genFile) + : ImmutableMap.of(), + /* legacyExternalRunfiles= */ false); + + Spawn spawn = defaultSpawnBuilder().withRunfilesSupplier(runfilesSupplier).build(); + + SpawnLogContext context = createSpawnLogContext(); + InputMetadataProvider inputMetadataProvider = + createInputMetadataProvider(runfilesSupplier, genFile); + + context.logSpawn( + spawn, + inputMetadataProvider, + createInputMap(runfilesSupplier, inputMetadataProvider), + outputsMode.getActionFileSystem(fs), + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/pkg/symlink/file") + .setDigest(getDigest("abc"))) .build()); } @@ -466,7 +1526,7 @@ public void testFilesetInput(@TestParameter DirContents dirContents) throws Exce ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/dir/file.txt") + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/dir/file.txt") .setDigest(getDigest("abc")) .build())) .build()); @@ -526,7 +1586,7 @@ public void testFileOutput( context.logSpawn( spawn, - createInputMetadataProvider(), + createInputMetadataProvider(fileOutput), createInputMap(), outputsMode.getActionFileSystem(fs), defaultTimeout(), @@ -535,8 +1595,11 @@ public void testFileOutput( closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/file") - .addActualOutputs(File.newBuilder().setPath("out/file").setDigest(getDigest("abc"))) + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/file") + .addActualOutputs( + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/file") + .setDigest(getDigest("abc"))) .build()); } @@ -563,9 +1626,11 @@ public void testFileOutputWithDirectoryContents(@TestParameter OutputsMode outpu closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/file") + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/file") .addActualOutputs( - File.newBuilder().setPath("out/file/file").setDigest(getDigest("abc"))) + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/file/file") + .setDigest(getDigest("abc"))) .build()); } @@ -615,17 +1680,17 @@ public void testTreeOutput( closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/tree") + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/tree") .addAllActualOutputs( dirContents.isEmpty() ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/tree/dir1/file1") + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/tree/dir1/file1") .setDigest(getDigest("abc")) .build(), File.newBuilder() - .setPath("out/tree/dir2/file2") + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/tree/dir2/file2") .setDigest(getDigest("def")) .build())) .build()); @@ -650,7 +1715,11 @@ public void testTreeOutputWithInvalidType(@TestParameter OutputsMode outputsMode defaultTimeout(), defaultSpawnResult()); - closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/tree").build()); + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/tree") + .build()); } @Test @@ -675,9 +1744,11 @@ public void testUnresolvedSymlinkOutput(@TestParameter OutputsMode outputsMode) closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/symlink") + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/symlink") .addActualOutputs( - File.newBuilder().setPath("out/symlink").setSymlinkTargetPath("/some/path")) + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/symlink") + .setSymlinkTargetPath("/some/path")) .build()); } @@ -700,7 +1771,11 @@ public void testUnresolvedSymlinkOutputWithInvalidType(@TestParameter OutputsMod defaultTimeout(), defaultSpawnResult()); - closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/symlink").build()); + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/symlink") + .build()); } @Test @@ -719,7 +1794,11 @@ public void testMissingOutput(@TestParameter OutputsMode outputsMode) throws Exc defaultTimeout(), defaultSpawnResult()); - closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/missing").build()); + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addListedOutputs(PRODUCT_NAME + "-out/k8-fastbuild/bin/missing") + .build()); } @Test @@ -971,20 +2050,69 @@ protected static SpawnExec.Builder defaultSpawnExecBuilder() { } protected static RunfilesSupplier createRunfilesSupplier( - PathFragment root, Map mapping) { - HashMap mappingByPath = new HashMap<>(); - for (Map.Entry entry : mapping.entrySet()) { - mappingByPath.put(PathFragment.create(entry.getKey()), entry.getValue()); + Artifact runfilesMiddleman, PathFragment root, Artifact... artifacts) { + return createRunfilesSupplier( + runfilesMiddleman, + root, + ImmutableMap.of(), + ImmutableMap.of(), + /* legacyExternalRunfiles= */ false, + NestedSetBuilder.wrap(Order.COMPILE_ORDER, Arrays.asList(artifacts))); + } + + protected static RunfilesSupplier createRunfilesSupplier( + Artifact runfilesMiddleman, + PathFragment root, + Map symlinks, + Map rootSymlinks, + boolean legacyExternalRunfiles, + NestedSet artifacts) { + Runfiles.Builder runfiles = + new Runfiles.Builder(TestConstants.WORKSPACE_NAME, legacyExternalRunfiles); + runfiles.addTransitiveArtifacts(artifacts); + for (Map.Entry entry : symlinks.entrySet()) { + runfiles.addSymlink(PathFragment.create(entry.getKey()), entry.getValue()); + } + for (Map.Entry entry : rootSymlinks.entrySet()) { + runfiles.addRootSymlink(PathFragment.create(entry.getKey()), entry.getValue()); } - RunfilesSupplier runfilesSupplier = mock(RunfilesSupplier.class); - when(runfilesSupplier.getMappings()).thenReturn(ImmutableMap.of(root, mappingByPath)); - return runfilesSupplier; + runfiles.setEmptyFilesSupplier(BazelPythonSemantics.GET_INIT_PY_FILES); + return new SingleRunfilesSupplier( + root, + runfiles.build(), + /* repoMappingManifest= */ null, + BuildConfigurationValue.RunfileSymlinksMode.EXTERNAL, + /* buildRunfileLinks= */ false, + runfilesMiddleman); + } + + protected static RunfilesSupplier createRunfilesSupplier( + Artifact runfilesMiddleman, + PathFragment root, + Map symlinks, + Map rootSymlinks, + boolean legacyExternalRunfiles, + Artifact... artifacts) { + return createRunfilesSupplier( + runfilesMiddleman, + root, + symlinks, + rootSymlinks, + legacyExternalRunfiles, + NestedSetBuilder.wrap(Order.COMPILE_ORDER, Arrays.asList(artifacts))); } protected static InputMetadataProvider createInputMetadataProvider(Artifact... artifacts) throws Exception { - ImmutableMap.Builder builder = ImmutableMap.builder(); - for (Artifact artifact : artifacts) { + return createInputMetadataProvider(EmptyRunfilesSupplier.INSTANCE, artifacts); + } + + protected static InputMetadataProvider createInputMetadataProvider( + RunfilesSupplier runfilesSupplier, Artifact... artifacts) throws Exception { + Iterable allArtifacts = + Iterables.concat(Arrays.asList(artifacts), runfilesSupplier.getArtifacts().toList()); + FakeActionInputFileCache builder = new FakeActionInputFileCache(); + for (Artifact artifact : allArtifacts) { if (artifact.isTreeArtifact()) { // Emulate ActionInputMap: add both tree and children. TreeArtifactValue treeMetadata = createTreeArtifactValue(artifact); @@ -999,28 +2127,34 @@ protected static InputMetadataProvider createInputMetadataProvider(Artifact... a builder.put(artifact, FileArtifactValue.createForTesting(artifact)); } } - return new StaticInputMetadataProvider(builder.buildOrThrow()); + return builder; } protected static SortedMap createInputMap(ActionInput... actionInputs) throws Exception { - return createInputMap(EmptyRunfilesSupplier.INSTANCE, actionInputs); + return createInputMap(EmptyRunfilesSupplier.INSTANCE, null, actionInputs); } protected static SortedMap createInputMap( - RunfilesSupplier runfilesSupplier, ActionInput... actionInputs) throws Exception { - ImmutableSortedMap.Builder builder = - ImmutableSortedMap.naturalOrder(); - for (Map.Entry> entry : - runfilesSupplier.getMappings().entrySet()) { - // Emulate SpawnInputExpander: expand runfiles, replacing nulls with empty inputs. - PathFragment root = entry.getKey(); - for (Map.Entry e : entry.getValue().entrySet()) { - PathFragment execPath = root.getRelative(e.getKey()); - Artifact artifact = e.getValue(); - builder.put(execPath, artifact != null ? artifact : VirtualActionInput.EMPTY_MARKER); - } - } + RunfilesSupplier runfilesSupplier, + InputMetadataProvider inputMetadataProvider, + ActionInput... actionInputs) + throws Exception { + TreeMap builder = new TreeMap<>(); + new SpawnInputExpander(/* execRoot= */ null, /* strict= */ false) + .addRunfilesToInputs( + builder, + runfilesSupplier, + inputMetadataProvider, + (treeArtifact, output) -> { + try { + output.addAll(createTreeArtifactValue(treeArtifact).getChildren()); + } catch (Exception e) { + throw new RuntimeException(e); + } + }, + PathMapper.NOOP, + PathFragment.EMPTY_FRAGMENT); for (ActionInput actionInput : actionInputs) { if (actionInput instanceof Artifact artifact && artifact.isTreeArtifact()) { // Emulate SpawnInputExpander: expand to children, preserve if empty. @@ -1036,7 +2170,7 @@ protected static SortedMap createInputMap( builder.put(actionInput.getExecPath(), actionInput); } } - return builder.buildOrThrow(); + return ImmutableSortedMap.copyOf(builder); } protected static TreeArtifactValue createTreeArtifactValue(Artifact tree) throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogReconstructorTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogReconstructorTest.java new file mode 100644 index 00000000000000..edf72ceb45a864 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogReconstructorTest.java @@ -0,0 +1,94 @@ +// Copyright 2024 The Bazel Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.exec; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.testutil.TestConstants.PRODUCT_NAME; + +import java.util.regex.MatchResult; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class SpawnLogReconstructorTest { + + @Test + public void extractRunfilesPathDefault() { + assertThat(matchDefault("file.txt")).isEqualTo(new Result(null, "file.txt")); + assertThat(matchDefault("pkg/file.txt")).isEqualTo(new Result(null, "pkg/file.txt")); + assertThat(matchDefault("pkg/external/file.txt")) + .isEqualTo(new Result(null, "pkg/external/file.txt")); + assertThat(matchDefault("external/some_repo/pkg/file.txt")) + .isEqualTo(new Result("some_repo", "pkg/file.txt")); + assertThat(matchDefault("external/some-repo+/pkg/file.txt")) + .isEqualTo(new Result("some-repo+", "pkg/file.txt")); + assertThat(matchDefault(PRODUCT_NAME + "-out/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result(null, "pkg/file.txt")); + assertThat(matchDefault(PRODUCT_NAME + "-out/k8-fastbuild/bin/pkg/external/file.txt")) + .isEqualTo(new Result(null, "pkg/external/file.txt")); + assertThat(matchDefault(PRODUCT_NAME + "-out/k8-fastbuild/bin/external/some_repo/pkg/file.txt")) + .isEqualTo(new Result("some_repo", "pkg/file.txt")); + assertThat( + matchDefault(PRODUCT_NAME + "-out/k8-fastbuild/bin/external/some-repo+/pkg/file.txt")) + .isEqualTo(new Result("some-repo+", "pkg/file.txt")); + } + + @Test + public void extractRunfilesPathSibling() { + assertThat(matchSibling("file.txt")).isEqualTo(new Result(null, "file.txt")); + assertThat(matchSibling("pkg/file.txt")).isEqualTo(new Result(null, "pkg/file.txt")); + assertThat(matchSibling("pkg/external/file.txt")) + .isEqualTo(new Result(null, "pkg/external/file.txt")); + assertThat(matchSibling("external/pkg/file.txt")) + .isEqualTo(new Result(null, "external/pkg/file.txt")); + assertThat(matchSibling("../some_repo/pkg/file.txt")) + .isEqualTo(new Result("some_repo", "pkg/file.txt")); + assertThat(matchSibling("../some-repo+/pkg/file.txt")) + .isEqualTo(new Result("some-repo+", "pkg/file.txt")); + assertThat(matchSibling(PRODUCT_NAME + "-out/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result(null, "pkg/file.txt")); + assertThat(matchSibling(PRODUCT_NAME + "-out/k8-fastbuild/bin/pkg/external/file.txt")) + .isEqualTo(new Result(null, "pkg/external/file.txt")); + assertThat(matchSibling(PRODUCT_NAME + "-out/k8-fastbuild/bin/external/pkg/file.txt")) + .isEqualTo(new Result(null, "external/pkg/file.txt")); + assertThat(matchSibling(PRODUCT_NAME + "-out/some_repo/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result("some_repo", "pkg/file.txt")); + assertThat(matchSibling(PRODUCT_NAME + "-out/some-repo+/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result("some-repo+", "pkg/file.txt")); + assertThat( + matchSibling( + PRODUCT_NAME + "-out/k8-fastbuild/coverage-metadata/bin/other/pkg/gen.txt")) + .isEqualTo(new Result(null, "bin/other/pkg/gen.txt")); + assertThat( + matchSibling( + PRODUCT_NAME + + "-out/some_repo/k8-fastbuild/coverage-metadata/bin/other/pkg/gen.txt")) + .isEqualTo(new Result("some_repo", "bin/other/pkg/gen.txt")); + } + + private record Result(String repo, String path) {} + + private static Result matchDefault(String path) { + MatchResult result = + SpawnLogReconstructor.extractRunfilesPath(path, /* siblingRepositoryLayout= */ false); + return new Result(result.group("repo"), result.group("path")); + } + + private static Result matchSibling(String path) { + MatchResult result = + SpawnLogReconstructor.extractRunfilesPath(path, /* siblingRepositoryLayout= */ true); + return new Result(result.group("repo"), result.group("path")); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index 364afac7d21aa2..5290b62cd87a27 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -81,6 +81,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", + "//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry", "//src/main/java/com/google/devtools/build/lib/authandtls", "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper", "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper:credential_module", diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 1f9d3208718b5b..05c8916bb6a49c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -2552,6 +2552,11 @@ public boolean isBuildRunfileLinks(PathFragment runfilesDir) { public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) { throw new UnsupportedOperationException(); } + + @Override + public Map getRunfilesTreesForLogging() { + throw new UnsupportedOperationException(); + } }; } 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 7d7a4dfe0604d8..b428397a8a8abf 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 @@ -216,7 +216,8 @@ public Action generate(ImmutableSet attributesToFlip) { Runfiles.EMPTY, /* repoMappingManifest= */ null, RunfileSymlinksMode.SKIP, - /* buildRunfileLinks= */ false)); + /* buildRunfileLinks= */ false, + /* runfilesMiddleman= */ null)); if (attributesToFlip.contains(KeyAttributes.INPUT)) { builder.addInput(artifactA);