From 4fbc3f4da4354728bdc28f048a251c065060f39b Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 20 Sep 2024 03:08:02 -0700 Subject: [PATCH] Optimize representation of runfiles in compact execution log Runfiles trees are now represented with a custom `RunfilesTree` message in the compact execution log. This allows using `InputSet`s to representing all artifacts staged at canonical locations, with only symlinks and root symlinks stored flattened and with explicit runfiles paths. Since runfile paths can collide, this change makes it necessary to preserve the order of elements in an `InputSet`. The previous representation as repeated ID fields for each type (file, symlink, directory) made this impossible, so the representation has been modified to reference all direct entry IDs in a single repeated field. Since this also reduces the potential for type mismatches between the ID field type and the referenced message type, all other typed IDs are replaced with untyped ID fields. By slightly tweaking the way IDs are generated for nested entries and not emitting IDs for entries that are never referenced (e.g. `Spawn`s), IDs are now consecutive, which simplifies the (possibly concurrent) bookkeeping for consumers by allowing them to use an array to store the entries. Progress on #18643. RELNOTES: The compact execution log now stores runfiles in a more compact representation that should reduce the memory overhead and log output size, in particular for test spawns. This change required breaking changes to the (experimental) log format. Closes #23321. PiperOrigin-RevId: 676773599 Change-Id: I010653681ffa44557142bf25009e9178b5d68515 --- .../lib/actions/ActionExecutionContext.java | 32 + .../google/devtools/build/lib/actions/BUILD | 3 + .../build/lib/actions/RunfilesTree.java | 30 + .../devtools/build/lib/analysis/Runfiles.java | 23 +- .../build/lib/analysis/RunfilesSupport.java | 50 +- .../com/google/devtools/build/lib/bazel/BUILD | 1 + .../build/lib/bazel/SpawnLogModule.java | 5 + .../com/google/devtools/build/lib/exec/BUILD | 3 +- .../lib/exec/CompactSpawnLogContext.java | 268 ++-- .../build/lib/exec/SpawnInputExpander.java | 2 +- .../build/lib/exec/SpawnLogReconstructor.java | 497 ++++++- src/main/protobuf/spawn.proto | 125 +- .../build/lib/analysis/RunfilesTest.java | 5 +- .../devtools/build/lib/analysis/util/BUILD | 1 + .../lib/analysis/util/FakeRunfilesTree.java | 32 + .../build/lib/bazel/rules/python/BUILD | 1 + .../BazelPyBinaryConfiguredTargetTest.java | 15 +- .../com/google/devtools/build/lib/exec/BUILD | 8 + .../lib/exec/CompactSpawnLogContextTest.java | 10 +- .../lib/exec/SpawnLogContextTestBase.java | 1189 ++++++++++++++++- .../lib/exec/SpawnLogReconstructorTest.java | 94 ++ .../google/devtools/build/lib/remote/BUILD | 1 + .../remote/RemoteExecutionServiceTest.java | 33 + 23 files changed, 2136 insertions(+), 292 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/exec/SpawnLogReconstructorTest.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index c6d3749d4ebb5b..0d4913549fa6ad 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.cache.OutputMetadataStore; +import com.google.devtools.build.lib.analysis.SymlinkEntry; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.clock.Clock; @@ -88,6 +89,37 @@ public boolean isBuildRunfileLinks() { public String getWorkspaceName() { return wrapped.getWorkspaceName(); } + + @Override + public NestedSet getArtifactsAtCanonicalLocationsForLogging() { + return wrapped.getArtifactsAtCanonicalLocationsForLogging(); + } + + @Override + public Iterable getEmptyFilenamesForLogging() { + return wrapped.getEmptyFilenamesForLogging(); + } + + @Override + public NestedSet getSymlinksForLogging() { + return wrapped.getSymlinksForLogging(); + } + + @Override + public NestedSet getRootSymlinksForLogging() { + return wrapped.getRootSymlinksForLogging(); + } + + @Nullable + @Override + public Artifact getRepoMappingManifestForLogging() { + return wrapped.getRepoMappingManifestForLogging(); + } + + @Override + public boolean isLegacyExternalRunfiles() { + return wrapped.isLegacyExternalRunfiles(); + } } /** 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 8832e01fbbcbb9..9d563607b6db3e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -163,6 +163,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", @@ -553,8 +554,10 @@ 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", + "//third_party:jsr305", ], ) 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 index 1a1ea760ac1926..5228472def7b41 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java +++ b/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java @@ -14,10 +14,12 @@ package com.google.devtools.build.lib.actions; +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; import java.util.Map; +import javax.annotation.Nullable; /** Lazy wrapper for a single runfiles tree. */ // TODO(bazel-team): Ideally we could refer to Runfiles objects directly here, but current package @@ -45,4 +47,32 @@ public interface RunfilesTree { /** Returns the name of the workspace that the build is occurring in. */ String getWorkspaceName(); + + /** + * 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. + */ + NestedSet getArtifactsAtCanonicalLocationsForLogging(); + + /** + * 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. + */ + Iterable getEmptyFilenamesForLogging(); + + /** Returns the set of custom symlink entries. */ + NestedSet getSymlinksForLogging(); + + /** Returns the set of root symlinks. */ + NestedSet getRootSymlinksForLogging(); + + /** Returns the repo mapping manifest if it exists. */ + @Nullable + Artifact getRepoMappingManifestForLogging(); + + /** Whether this runfiles tree materializes external runfiles also at their legacy locations. */ + boolean isLegacyExternalRunfiles(); } 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 ea4f95d5490815..007467064e45c8 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 @@ -245,10 +245,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), @@ -259,13 +266,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); } /** @@ -383,6 +384,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 4f2c055c534f51..ee159b6bb25605 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 @@ -88,14 +88,16 @@ public final class RunfilesSupport { private static final String OUTPUT_MANIFEST_BASENAME = "MANIFEST"; private static final String REPO_MAPPING_MANIFEST_EXT = ".repo_mapping"; - private static class RunfilesTreeImpl implements RunfilesTree { + /** The implementation of {@link RunfilesTree}. */ + @VisibleForTesting + public static class RunfilesTreeImpl implements RunfilesTree { private static final WeakReference> NOT_YET_COMPUTED = new WeakReference<>(null); private final PathFragment execPath; private final Runfiles runfiles; - private final Artifact repoMappingManifest; + @Nullable private final Artifact repoMappingManifest; /** * The cached runfiles mapping. Possible values: @@ -119,7 +121,7 @@ private static class RunfilesTreeImpl implements RunfilesTree { private RunfilesTreeImpl( PathFragment execPath, Runfiles runfiles, - Artifact repoMappingManifest, + @Nullable Artifact repoMappingManifest, boolean buildRunfileLinks, boolean cacheMapping, RunfileSymlinksMode runfileSymlinksMode) { @@ -131,6 +133,17 @@ private RunfilesTreeImpl( this.cachedMapping = cacheMapping ? NOT_YET_COMPUTED : null; } + @VisibleForTesting + public RunfilesTreeImpl(PathFragment execPath, Runfiles runfiles) { + this( + execPath, + runfiles, + /* repoMappingManifest= */ null, + /* buildRunfileLinks= */ false, + /* cacheMapping= */ false, + RunfileSymlinksMode.EXTERNAL); + } + @Override public PathFragment getExecPath() { return execPath; @@ -167,6 +180,37 @@ public NestedSet getArtifacts() { return runfiles.getAllArtifacts(); } + @Override + public NestedSet getArtifactsAtCanonicalLocationsForLogging() { + return runfiles.getArtifacts(); + } + + @Override + public Iterable getEmptyFilenamesForLogging() { + return runfiles.getEmptyFilenames(); + } + + @Override + public NestedSet getSymlinksForLogging() { + return runfiles.getSymlinks(); + } + + @Override + public NestedSet getRootSymlinksForLogging() { + return runfiles.getRootSymlinks(); + } + + @Nullable + @Override + public Artifact getRepoMappingManifestForLogging() { + return repoMappingManifest; + } + + @Override + public boolean isLegacyExternalRunfiles() { + return runfiles.isLegacyExternalRunfiles(); + } + @Override public RunfileSymlinksMode getSymlinksMode() { return runfileSymlinksMode; 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 18eb277693aaae..aa8921c93cc46b 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 92bb9b347ef1ee..9d31c1d26e8577 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -260,7 +260,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:guava", - "//third_party:jsr305", ], ) @@ -277,6 +276,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_tree", + "//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", @@ -306,7 +306,6 @@ java_library( ], deps = [ "//src/main/java/com/google/devtools/build/lib/profiler", - "//src/main/java/com/google/devtools/build/lib/util:pair", "//src/main/java/com/google/devtools/build/lib/util/io:io-proto", "//src/main/protobuf:spawn_java_proto", "//third_party:guava", 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 805c7123000b82..35a201922b4e39 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 @@ -17,7 +17,9 @@ import static com.google.common.base.Preconditions.checkState; import com.github.luben.zstd.ZstdOutputStream; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +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; @@ -29,6 +31,7 @@ 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; @@ -50,17 +53,15 @@ 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.SortedMap; import java.util.concurrent.ForkJoinPool; import javax.annotation.Nullable; @@ -137,12 +138,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") @@ -158,11 +162,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; @@ -179,13 +187,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 @@ -224,21 +233,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()); } } @@ -260,7 +264,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)); } } } @@ -286,7 +290,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)); } } } @@ -294,8 +298,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) @@ -317,7 +321,7 @@ private int logInputs( inputMetadataProvider)); } - return logNestedSet( + return logInputSet( spawn.getInputFiles(), additionalDirectoryIds.build(), inputMetadataProvider, @@ -328,13 +332,13 @@ private int logInputs( /** * 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(), ImmutableList.of(), inputMetadataProvider, @@ -352,7 +356,7 @@ private int logTools( * @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, Collection additionalDirectoryIds, InputMetadataProvider inputMetadataProvider, @@ -367,12 +371,12 @@ 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(), inputMetadataProvider, @@ -381,39 +385,88 @@ private int logNestedSet( } for (ActionInput input : set.getLeaves()) { - if (input instanceof Artifact && ((Artifact) input).isMiddlemanArtifact()) { + if (input instanceof Artifact artifact && artifact.isMiddlemanArtifact()) { RunfilesTree runfilesTree = inputMetadataProvider.getRunfilesMetadata(input).getRunfilesTree(); - builder.addDirectoryIds( - // The runfiles symlink tree might not have been materialized on disk, so use the - // mapping. - logRunfilesDirectory( - runfilesTree.getExecPath(), - runfilesTree.getMapping(), + builder.addInputIds( + logRunfilesTree( + runfilesTree, inputMetadataProvider, - fileSystem)); + fileSystem, + // If the nested set containing the runfiles tree isn't shared (i.e., it + // contains inputs, not tools), the runfiles are also likely not shared. This + // avoids storing the runfiles tree of a test. + shared)); 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().setSymlinkEntries(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. * @@ -453,7 +506,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 @@ -470,64 +523,69 @@ 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 RunfilesTree}. * - *

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. + * @param shared whether this runfiles tree is likely to be contained in more than one Spawn's + * inputs + * @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 shared) throws IOException, InterruptedException { return logEntry( - root.getPathString(), + shared ? runfilesTree.getExecPath().getPathString() : null, () -> { - ExecLogEntry.Directory.Builder builder = - ExecLogEntry.Directory.newBuilder().setPath(root.getPathString()); + Preconditions.checkState(workspaceName.equals(runfilesTree.getWorkspaceName())); - for (Map.Entry entry : mapping.entrySet()) { - String runfilesPath = entry.getKey().getPathString(); - Artifact input = entry.getValue(); + ExecLogEntry.RunfilesTree.Builder builder = + ExecLogEntry.RunfilesTree.newBuilder() + .setPath(runfilesTree.getExecPath().getPathString()) + .setLegacyExternalRunfiles(runfilesTree.isLegacyExternalRunfiles()); - 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)); + builder.setInputSetId( + logInputSet( + runfilesTree.getArtifactsAtCanonicalLocationsForLogging(), + /* additionalDirectoryIds= */ 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)); + 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); }); } @@ -535,19 +593,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, @@ -558,14 +612,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; } @@ -591,6 +648,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. * @@ -601,14 +668,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/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index 728eee2e61f476..100f11ebfa7c0a 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 @@ -85,7 +85,7 @@ private static void addMapping( } @VisibleForTesting - void addSingleRunfilesTreeToInputs( + public void addSingleRunfilesTreeToInputs( RunfilesTree runfilesTree, Map inputMap, ArtifactExpander artifactExpander, 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 a5530792134b4f..ede4f06f1c64f4 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,12 +125,19 @@ public SpawnExec read() throws IOException { ExecLogEntry entry; while ((entry = ExecLogEntry.parseDelimitedFrom(in)) != null) { switch (entry.getTypeCase()) { - case INVOCATION -> hashFunctionName = entry.getInvocation().getHashFunctionName(); - case FILE -> fileMap.put(entry.getId(), reconstructFile(entry.getFile())); - case DIRECTORY -> dirMap.put(entry.getId(), reconstructDir(entry.getDirectory())); + case INVOCATION -> { + hashFunctionName = entry.getInvocation().getHashFunctionName(); + 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 -> - symlinkMap.put(entry.getId(), reconstructSymlink(entry.getUnresolvedSymlink())); - case INPUT_SET -> setMap.put(entry.getId(), entry.getInputSet()); + 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_ENTRIES -> putSymlinkEntrySet(entry.getId(), entry.getSymlinkEntries()); case SPAWN -> { return reconstructSpawnExec(entry.getSpawn()); } @@ -94,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); @@ -109,27 +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); - } - 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); } } - case UNRESOLVED_SYMLINK_ID -> { - File symlink = getFromMap(symlinkMap, output.getUnresolvedSymlinkId()); - listedOutputs.add(symlink.getPath()); - builder.addActualOutputs(symlink); - } case INVALID_OUTPUT_PATH -> listedOutputs.add(output.getInvalidOutputPath()); default -> throw new IOException( - String.format("unknown output type %d", output.getTypeCase().getNumber())); + "unknown output type %d".formatted(output.getTypeCase().getNumber())); } } @@ -142,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( @@ -205,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 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(); + } + } + + 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 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 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())); } - return value; + 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/protobuf/spawn.proto b/src/main/protobuf/spawn.proto index f5c261dd7d5324..7b0931610ae35e 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_entries = 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 97bf9f66e3929f..c5912335d298c7 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 @@ -625,7 +625,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/util/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD index 62fc72b8043ca2..4c8df2c63e268e 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 index 581a0d067312a5..e69e266fd54726 100644 --- 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 @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.RunfilesTree; import com.google.devtools.build.lib.analysis.Runfiles; +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.skyframe.serialization.autocodec.AutoCodec; @@ -90,4 +91,35 @@ public boolean isBuildRunfileLinks() { public String getWorkspaceName() { return runfiles.getPrefix(); } + + @Override + public NestedSet getArtifactsAtCanonicalLocationsForLogging() { + return runfiles.getArtifacts(); + } + + @Override + public Iterable getEmptyFilenamesForLogging() { + return runfiles.getEmptyFilenames(); + } + + @Override + public NestedSet getSymlinksForLogging() { + return runfiles.getSymlinks(); + } + + @Override + public NestedSet getRootSymlinksForLogging() { + return runfiles.getRootSymlinks(); + } + + @Nullable + @Override + public Artifact getRepoMappingManifestForLogging() { + return repoMappingManifest; + } + + @Override + public boolean isLegacyExternalRunfiles() { + return runfiles.isLegacyExternalRunfiles(); + } } 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 6dfecc435464bb..dd09e6da17ffc6 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; @@ -362,8 +363,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 @@ -377,8 +377,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 @@ -391,8 +391,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 @@ -406,8 +406,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 4cf72d1180bea5..6921f415dab1b8 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -37,7 +37,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", @@ -88,6 +95,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..961465861c9755 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 @@ -32,6 +32,7 @@ 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.SyscallCache; @@ -143,12 +144,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") @@ -167,6 +169,8 @@ protected SpawnLogContext createSpawnLogContext(ImmutableMap pla return new CompactSpawnLogContext( logPath, execRoot.asFragment(), + TestConstants.WORKSPACE_NAME, + siblingRepositoryLayout, remoteOptions, DigestHashFunction.SHA256, SyscallCache.NO_CACHE); 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 e67a0556ef1b7d..8e2fa663407d94 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,34 +14,55 @@ 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.ArtifactExpander; 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.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.RunfilesTree; 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.RunfilesSupport; +import com.google.devtools.build.lib.analysis.ServerDirectories; +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.BazelPyBuiltins; +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; @@ -52,6 +73,7 @@ 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,27 +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 ArtifactRoot middlemanDir = - ArtifactRoot.asDerivedRoot(execRoot, RootType.Middleman, "middlemen"); + 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). @@ -290,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())) @@ -324,7 +404,7 @@ 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()); @@ -338,20 +418,15 @@ public void testRunfilesFileInput() throws Exception { writeFile(runfilesInput, "abc"); PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); - RunfilesTree runfilesTree = - createRunfilesTree(runfilesRoot, ImmutableMap.of("data.txt", runfilesInput)); + RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput); Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); SpawnLogContext context = createSpawnLogContext(); - FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache(); - inputMetadataProvider.putRunfilesTree(runfilesMiddleman, runfilesTree); - inputMetadataProvider.put(runfilesInput, FileArtifactValue.createForTesting(runfilesInput)); - context.logSpawn( spawn, - inputMetadataProvider, + createInputMetadataProvider(runfilesMiddleman, runfilesTree, runfilesInput), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -361,7 +436,13 @@ 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"))) .build()); } @@ -376,20 +457,15 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t } PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); - RunfilesTree runfilesTree = - createRunfilesTree(runfilesRoot, ImmutableMap.of("dir", runfilesInput)); + RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput); Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); SpawnLogContext context = createSpawnLogContext(); - FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache(); - inputMetadataProvider.putRunfilesTree(runfilesMiddleman, runfilesTree); - inputMetadataProvider.put(runfilesInput, FileArtifactValue.createForTesting(runfilesInput)); - context.logSpawn( spawn, - inputMetadataProvider, + createInputMetadataProvider(runfilesMiddleman, runfilesTree, runfilesInput), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -403,7 +479,11 @@ 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")) .build())) .build()); @@ -412,30 +492,936 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t @Test public void testRunfilesEmptyInput() 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); - RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, mapping); + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, runfilesInput, externalGenArtifact, externalSourceArtifact); Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); SpawnLogContext context = createSpawnLogContext(); - FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache(); - inputMetadataProvider.putRunfilesTree(runfilesMiddleman, runfilesTree); + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, + runfilesTree, + runfilesInput, + externalGenArtifact, + externalSourceArtifact), + createInputMap(runfilesTree), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath(PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/sub/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/sub/dir/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/" + + WORKSPACE_NAME + + "/sub/dir/script.py") + .setDigest(getDigest("abc"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/gen.py") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + "-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/lib.py") + .setDigest(getDigest("external_source"))) + .build()); + } + + @Test + public void testRunfilesMixedRoots(@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 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"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesTree runfilesTree = + createRunfilesTree( + 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().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); context.logSpawn( spawn, - inputMetadataProvider, + createInputMetadataProvider( + runfilesMiddleman, + runfilesTree, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact, + symlinkSourceTarget, + symlinkGenTarget, + rootSymlinkSourceTarget, + rootSymlinkGenTarget), createInputMap(runfilesTree), 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"); + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, + ImmutableMap.of((symlinkUnderMain ? "" : "../some_repo/") + "symlink", symlinkTarget), + ImmutableMap.of( + (rootSymlinkUnderMain ? WORKSPACE_NAME + "/" : "some_repo/") + "root_symlink", + rootSymlinkTarget), + legacyExternalRunfiles, + externalSourceArtifact, + externalGenArtifact); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, + runfilesTree, + externalSourceArtifact, + externalGenArtifact, + symlinkTarget, + rootSymlinkTarget), + createInputMap(runfilesTree), + 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"); + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + legacyExternalRunfiles, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, + runfilesTree, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact), + createInputMap(runfilesTree), + 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"); + RunfilesTree runfilesTree = + createRunfilesTree( + 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().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, + runfilesTree, + sourceArtifact, + genArtifact, + externalSourceArtifact, + externalGenArtifact, + symlinkSourceArtifact, + symlinkGenArtifact, + symlinkExternalSourceArtifact, + symlinkExternalGenArtifact), + createInputMap(runfilesTree), + 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"); + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(WORKSPACE_NAME + "/pkg/source.txt", symlinkSourceArtifact), + /* legacyExternalRunfiles= */ false, + sourceArtifact); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, runfilesTree, sourceArtifact, symlinkSourceArtifact), + createInputMap(runfilesTree), + 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); + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + /* legacyExternalRunfiles= */ false, + NestedSetBuilder.wrap(Order.STABLE_ORDER, artifacts)); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider(runfilesMiddleman, runfilesTree, file, symlink), + createInputMap(runfilesTree), + 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"); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + + 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); + } + + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, + ImmutableMap.of(), + ImmutableMap.of(), + /* legacyExternalRunfiles= */ false, + artifacts); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, runfilesTree, sourceFile, genFile, otherSourceFile, otherGenFile), + createInputMap(runfilesTree), + 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) throws Exception { + 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")); + + Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); + + PathFragment runfilesRoot = outputDir.getExecPath().getRelative("tools/foo.runfiles"); + RunfilesTree runfilesTree = + createRunfilesTree( + 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); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, runfilesTree, sourceFile, sourceDir, genDir, symlink), + createInputMap(runfilesTree), + 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"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/gen_dir/other_file") + .setDigest(getDigest("gen_dir_file"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/source_dir/some_file") + .setDigest(getDigest("source_dir_file"))) + .addInputs( + File.newBuilder() + .setPath( + PRODUCT_NAME + + "-out/k8-fastbuild/bin/tools/foo.runfiles/" + + WORKSPACE_NAME + + "/symlink") + .setSymlinkTargetPath("/some/path")) + .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"); + RunfilesTree runfilesTree = + createRunfilesTree( + runfilesRoot, + rootSymlink ? ImmutableMap.of() : ImmutableMap.of("pkg/symlink", genFile), + rootSymlink + ? ImmutableMap.of(WORKSPACE_NAME + "/pkg/symlink", genFile) + : ImmutableMap.of(), + /* legacyExternalRunfiles= */ false); + + Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); + + SpawnLogContext context = createSpawnLogContext(); + + context.logSpawn( + spawn, + createInputMetadataProvider(runfilesMiddleman, runfilesTree, genFile), + createInputMap(runfilesTree), + 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()); } @@ -482,7 +1468,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()); @@ -551,8 +1537,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()); } @@ -579,9 +1568,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()); } @@ -631,17 +1622,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()); @@ -666,7 +1657,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 @@ -691,9 +1686,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()); } @@ -716,7 +1713,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 @@ -735,7 +1736,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 @@ -986,22 +1991,59 @@ protected static SpawnExec.Builder defaultSpawnExecBuilder() { .setMetrics(Protos.SpawnMetrics.getDefaultInstance()); } + protected static RunfilesTree createRunfilesTree(PathFragment root, Artifact... artifacts) { + return createRunfilesTree( + root, ImmutableMap.of(), ImmutableMap.of(), /* legacyExternalRunfiles= */ false, artifacts); + } + protected static RunfilesTree createRunfilesTree( - PathFragment root, Map mapping) { - HashMap mappingByPath = new HashMap<>(); - for (Map.Entry entry : mapping.entrySet()) { - mappingByPath.put(PathFragment.create(entry.getKey()), entry.getValue()); + 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()); } - RunfilesTree runfilesTree = mock(RunfilesTree.class); - when(runfilesTree.getExecPath()).thenReturn(root); - when(runfilesTree.getMapping()).thenReturn(mappingByPath); - return runfilesTree; + runfiles.setEmptyFilesSupplier(BazelPyBuiltins.GET_INIT_PY_FILES); + return new RunfilesSupport.RunfilesTreeImpl(root, runfiles.build()); + } + + protected static RunfilesTree createRunfilesTree( + PathFragment root, + Map symlinks, + Map rootSymlinks, + boolean legacyExternalRunfiles, + Artifact... artifacts) { + return createRunfilesTree( + 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(null, null, artifacts); + } + + protected static InputMetadataProvider createInputMetadataProvider( + Artifact runfilesMiddleman, RunfilesTree runfilesTree, Artifact... artifacts) + throws Exception { + Iterable allArtifacts = Arrays.asList(artifacts); + FakeActionInputFileCache builder = new FakeActionInputFileCache(); + if (runfilesMiddleman != null) { + allArtifacts = Iterables.concat(allArtifacts, runfilesTree.getArtifacts().toList()); + builder.putRunfilesTree(runfilesMiddleman, runfilesTree); + } + for (Artifact artifact : allArtifacts) { if (artifact.isTreeArtifact()) { // Emulate ActionInputMap: add both tree and children. TreeArtifactValue treeMetadata = createTreeArtifactValue(artifact); @@ -1016,7 +2058,7 @@ 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) @@ -1026,17 +2068,22 @@ protected static SortedMap createInputMap(ActionInput protected static SortedMap createInputMap( RunfilesTree runfilesTree, ActionInput... actionInputs) throws Exception { - ImmutableSortedMap.Builder builder = - ImmutableSortedMap.naturalOrder(); + TreeMap builder = new TreeMap<>(); if (runfilesTree != null) { - // Emulate SpawnInputExpander: expand runfiles, replacing nulls with empty inputs. - PathFragment root = runfilesTree.getExecPath(); - for (Map.Entry entry : runfilesTree.getMapping().entrySet()) { - PathFragment execPath = root.getRelative(entry.getKey()); - Artifact artifact = entry.getValue(); - builder.put(execPath, artifact != null ? artifact : VirtualActionInput.EMPTY_MARKER); - } + new SpawnInputExpander(/* execRoot= */ null) + .addSingleRunfilesTreeToInputs( + runfilesTree, + builder, + treeArtifact -> { + try { + return createTreeArtifactValue(treeArtifact).getChildren(); + } catch (Exception e) { + throw new ArtifactExpander.MissingExpansionException(e.getMessage()); + } + }, + PathMapper.NOOP, + PathFragment.EMPTY_FRAGMENT); } for (ActionInput actionInput : actionInputs) { @@ -1054,7 +2101,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 eedfc95f9bbf45..4820d871cc94f0 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -84,6 +84,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 786e4754a223fb..af881a20b6b487 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 @@ -81,6 +81,7 @@ import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.StaticInputMetadataProvider; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.analysis.SymlinkEntry; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode; import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -134,6 +135,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -2684,6 +2686,37 @@ public boolean isBuildRunfileLinks() { public String getWorkspaceName() { return "__main__"; } + + @Override + public NestedSet getArtifactsAtCanonicalLocationsForLogging() { + return NestedSetBuilder.wrap(Order.STABLE_ORDER, artifacts); + } + + @Override + public ImmutableList getEmptyFilenamesForLogging() { + return ImmutableList.of(); + } + + @Override + public NestedSet getSymlinksForLogging() { + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } + + @Override + public NestedSet getRootSymlinksForLogging() { + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } + + @Nullable + @Override + public Artifact getRepoMappingManifestForLogging() { + return null; + } + + @Override + public boolean isLegacyExternalRunfiles() { + return false; + } }; }