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..25496b7416d84d 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,15 @@ 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 { + @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 +120,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 +132,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 +179,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..41c40706238044 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -277,6 +277,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", @@ -305,9 +306,11 @@ java_library( "StableSort.java", ], deps = [ + "//src/main/java/com/google/devtools/build/lib/cmdline", "//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/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/protobuf:spawn_java_proto", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java index 805c7123000b82..2370337ea46f1e 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 @@ -471,63 +524,69 @@ private int logDirectory( ExecLogEntry.Directory.newBuilder() .setPath(input.getExecPathString()) .addAllFiles( - expandDirectory(root, /* pathPrefix= */ null, inputMetadataProvider)))); + expandDirectory(root, /* pathPrefix= */ 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 +594,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 +613,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 +649,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 +669,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..3d3cd90499153d 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.collect.ImmutableList.toImmutableList; + import com.github.luben.zstd.ZstdInputStream; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; 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,123 @@ 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); + } } } } - 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())); + } + } } } } - 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 +346,226 @@ 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); + Preconditions.checkState(matcher.matches()); + return matcher; } - private static T getFromMap(Map map, int id) throws IOException { - T value = map.get(id); - if (value == null) { - throw new IOException(String.format("referenced entry %d is missing or has wrong type", id)); + private boolean hasWorkspaceRunfilesDirectory(String path) { + return extractRunfilesPath(path, siblingRepositoryLayout).group("repo") == null; + } + + private Stream getRunfilesPaths(String execPath, boolean legacyExternalRunfiles) { + MatchResult matchResult = extractRunfilesPath(execPath, siblingRepositoryLayout); + String repo = matchResult.group("repo"); + String repoRelativePath = matchResult.group("path"); + if (repo == null) { + return Stream.of(workspaceRunfilesDirectory + "/" + repoRelativePath); + } else { + Stream.Builder paths = Stream.builder(); + paths.add(repo + "/" + repoRelativePath); + if (legacyExternalRunfiles) { + paths.add("%s/external/%s/%s".formatted(workspaceRunfilesDirectory, repo, repoRelativePath)); + } + return paths.build(); } - return value; + } + + private Collection reconstructRunfilesSymlinkTarget(String newPath, int targetId) + throws IOException { + if (targetId == 0) { + return ImmutableList.of(File.newBuilder().setPath(newPath).build()); + } + return switch (getInput(targetId)) { + case Input.File(File file) -> ImmutableList.of(file.toBuilder().setPath(newPath).build()); + case Input.Symlink(File symlink) -> + ImmutableList.of(symlink.toBuilder().setPath(newPath).build()); + case Input.Directory(String path, Collection files) -> + files.stream() + .map( + file -> + file.toBuilder() + .setPath(newPath + file.getPath().substring(path.length())) + .build()) + .collect(toImmutableList()); + }; + } + + private void putInput(int id, Input input) throws IOException { + putEntry(id, input); + } + + private void putInputSet(int id, ExecLogEntry.InputSet inputSet) throws IOException { + putEntry(id, inputSet); + } + + private void putSymlinkEntrySet(int id, ExecLogEntry.SymlinkEntrySet symlinkEntries) + throws IOException { + putEntry(id, symlinkEntries); + } + + private void putEntry(int id, Object entry) throws IOException { + if (id == 0) { + // The entry won't be referenced, so we don't need to store it. + return; + } + // Bazel emits consecutive non-zero IDs. + if (id != inputMap.size()) { + throw new IOException( + "ids must be consecutive, got %d after %d".formatted(id, inputMap.size())); + } + inputMap.add( + switch (entry) { + // Unwrap trivial wrappers to reduce retained memory usage. + case Input.File file -> file.file; + case Input.Symlink symlink -> symlink.symlink; + default -> entry; + }); + } + + private Input getInput(int id) throws IOException { + Object value = inputMap.get(id); + return switch (value) { + case Input input -> input; + case Protos.File file -> + file.getSymlinkTargetPath().isEmpty() ? new Input.File(file) : new Input.Symlink(file); + case null -> throw new IOException("referenced input %d is missing".formatted(id)); + default -> throw new IOException("entry %d is not an input: %s".formatted(id, value)); + }; + } + + private ExecLogEntry.InputSet getInputSet(int id) throws IOException { + Object value = inputMap.get(id); + return switch (value) { + case ExecLogEntry.InputSet inputSet -> inputSet; + case null -> throw new IOException("referenced input set %d is missing".formatted(id)); + default -> throw new IOException("entry %d is not an input set: %s".formatted(id, value)); + }; + } + + private ExecLogEntry.SymlinkEntrySet getSymlinkEntrySet(int id) throws IOException { + Object value = inputMap.get(id); + return switch (value) { + case ExecLogEntry.SymlinkEntrySet symlinkEntries -> symlinkEntries; + case null -> + throw new IOException("referenced set of symlink entries %d is missing".formatted(id)); + default -> + throw new IOException( + "entry %d is not a set of symlink entries: %s".formatted(id, value)); + }; } @Override diff --git a/src/main/protobuf/spawn.proto b/src/main/protobuf/spawn.proto index f5c261dd7d5324..1fe1b03c3b47b6 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,30 +262,93 @@ 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 = 1; + reserved 2, 3; + // Entry IDs of other input sets contained in this set. + repeated uint32 transitive_set_ids = 4; + } + + // 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 { + reserved 2, 3; 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 = 1; // 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; @@ -294,10 +367,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; @@ -356,8 +429,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 +442,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..531e6ff56fcbf0 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,53 @@ 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 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 +71,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 +81,86 @@ 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.List; 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 +369,7 @@ public void testTreeInput( ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/tree/child") + .setPath("bazel-out/k8-fastbuild/bin/tree/child") .setDigest(getDigest("abc")) .setIsTool(inputsMode.isTool()) .build())) @@ -324,7 +403,7 @@ public void testUnresolvedSymlinkInput(@TestParameter InputsMode inputsMode) thr defaultSpawnExecBuilder() .addInputs( File.newBuilder() - .setPath("out/symlink") + .setPath("bazel-out/k8-fastbuild/bin/symlink") .setSymlinkTargetPath("/some/path") .setIsTool(inputsMode.isTool())) .build()); @@ -338,20 +417,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 +435,9 @@ public void testRunfilesFileInput() throws Exception { context, defaultSpawnExecBuilder() .addInputs( - File.newBuilder().setPath("out/foo.runfiles/data.txt").setDigest(getDigest("abc"))) + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/_main/data.txt") + .setDigest(getDigest("abc"))) .build()); } @@ -376,20 +452,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 +474,7 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/foo.runfiles/dir/data.txt") + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/_main/dir/data.txt") .setDigest(getDigest("abc")) .build())) .build()); @@ -412,21 +483,673 @@ 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(); + + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, + runfilesTree, + runfilesInput, + externalGenArtifact, + externalSourceArtifact), + createInputMap(runfilesTree), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/_main/sub/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/_main/sub/dir/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/_main/sub/dir/script.py") + .setDigest(getDigest("abc"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/some_repo/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/__init__.py")) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/some_repo/other/pkg/gen.py") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/foo.runfiles/some_repo/pkg/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-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, + 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( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source"))); + } + builder + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/other/pkg/gen.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/other/symlink") + .setDigest(getDigest("symlink_gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/pkg/source.txt") + .setDigest(getDigest("source"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/some/symlink") + .setDigest(getDigest("symlink_source"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/root/other/symlink") + .setDigest(getDigest("root_symlink_gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/root/symlink") + .setDigest(getDigest("root_symlink_source"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-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 ? "_main/" : "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()); + + List files = + new ArrayList<>( + ImmutableList.of( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/%s/root_symlink" + .formatted(rootSymlinkUnderMain ? "_main" : "some_repo")) + .setDigest(getDigest("root_symlink_target")) + .build(), + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/%s/symlink" + .formatted(symlinkUnderMain ? "_main" : "some_repo")) + .setDigest(getDigest("symlink_target")) + .build(), + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen")) + .build(), + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source")) + .build())); + if (legacyExternalRunfiles) { + files.add( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen")) + .build()); + files.add( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source")) + .build()); + if (!symlinkUnderMain) { + files.add( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/symlink") + .setDigest(getDigest("symlink_target")) + .build()); + } + } else if (!symlinkUnderMain && !rootSymlinkUnderMain) { + files.add( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/.runfile") + .build()); + } + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .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( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/pkg/file.txt") + .setDigest(getDigest("external_gen"))); + } + builder + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/pkg/file.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-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( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/external/some_repo/pkg/source.txt") + .setDigest(getDigest("external_source"))); + } + builder + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/other/pkg/gen.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/pkg/source.txt") + .setDigest(getDigest("source"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/some_repo/other/pkg/gen.txt") + .setDigest(getDigest("external_gen"))) + .addInputs( + File.newBuilder() + .setPath("bazel-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("_main/pkg/source.txt", symlinkSourceArtifact), + /* legacyExternalRunfiles= */ false, + sourceArtifact); Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build(); SpawnLogContext context = createSpawnLogContext(); - FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache(); - inputMetadataProvider.putRunfilesTree(runfilesMiddleman, runfilesTree); + context.logSpawn( + spawn, + createInputMetadataProvider( + runfilesMiddleman, runfilesTree, sourceArtifact, symlinkSourceArtifact), + createInputMap(runfilesTree), + fs, + defaultTimeout(), + defaultSpawnResult()); + + closeAndAssertLog( + context, + defaultSpawnExecBuilder() + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/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("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/pkg/file.txt") + .setDigest(getDigest("file")) + : File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/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, - inputMetadataProvider, + createInputMetadataProvider( + runfilesMiddleman, runfilesTree, sourceFile, genFile, otherSourceFile, otherGenFile), createInputMap(runfilesTree), fs, defaultTimeout(), @@ -435,7 +1158,130 @@ public void testRunfilesEmptyInput() throws Exception { closeAndAssertLog( context, defaultSpawnExecBuilder() - .addInputs(File.newBuilder().setPath("out/foo.runfiles/__init__.py")) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/pkg/file.txt") + .setDigest(getDigest("gen"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/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( + "_main/file", sourceFile, + "_main/source_dir", sourceDir, + "_main/gen_dir", genDir, + "_main/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("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/file") + .setDigest(getDigest("source"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/gen_dir/other_file") + .setDigest(getDigest("gen_dir_file"))) + .addInputs( + File.newBuilder() + .setPath( + "bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/source_dir/some_file") + .setDigest(getDigest("source_dir_file"))) + .addInputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/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("_main/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("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/pkg/symlink/file") + .setDigest(getDigest("abc"))) .build()); } @@ -482,7 +1328,7 @@ public void testFilesetInput(@TestParameter DirContents dirContents) throws Exce ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/dir/file.txt") + .setPath("bazel-out/k8-fastbuild/bin/dir/file.txt") .setDigest(getDigest("abc")) .build())) .build()); @@ -551,8 +1397,11 @@ public void testFileOutput( closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/file") - .addActualOutputs(File.newBuilder().setPath("out/file").setDigest(getDigest("abc"))) + .addListedOutputs("bazel-out/k8-fastbuild/bin/file") + .addActualOutputs( + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/file") + .setDigest(getDigest("abc"))) .build()); } @@ -579,9 +1428,11 @@ public void testFileOutputWithDirectoryContents(@TestParameter OutputsMode outpu closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/file") + .addListedOutputs("bazel-out/k8-fastbuild/bin/file") .addActualOutputs( - File.newBuilder().setPath("out/file/file").setDigest(getDigest("abc"))) + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/file/file") + .setDigest(getDigest("abc"))) .build()); } @@ -631,17 +1482,17 @@ public void testTreeOutput( closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/tree") + .addListedOutputs("bazel-out/k8-fastbuild/bin/tree") .addAllActualOutputs( dirContents.isEmpty() ? ImmutableList.of() : ImmutableList.of( File.newBuilder() - .setPath("out/tree/dir1/file1") + .setPath("bazel-out/k8-fastbuild/bin/tree/dir1/file1") .setDigest(getDigest("abc")) .build(), File.newBuilder() - .setPath("out/tree/dir2/file2") + .setPath("bazel-out/k8-fastbuild/bin/tree/dir2/file2") .setDigest(getDigest("def")) .build())) .build()); @@ -666,7 +1517,9 @@ public void testTreeOutputWithInvalidType(@TestParameter OutputsMode outputsMode defaultTimeout(), defaultSpawnResult()); - closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/tree").build()); + closeAndAssertLog( + context, + defaultSpawnExecBuilder().addListedOutputs("bazel-out/k8-fastbuild/bin/tree").build()); } @Test @@ -691,9 +1544,11 @@ public void testUnresolvedSymlinkOutput(@TestParameter OutputsMode outputsMode) closeAndAssertLog( context, defaultSpawnExecBuilder() - .addListedOutputs("out/symlink") + .addListedOutputs("bazel-out/k8-fastbuild/bin/symlink") .addActualOutputs( - File.newBuilder().setPath("out/symlink").setSymlinkTargetPath("/some/path")) + File.newBuilder() + .setPath("bazel-out/k8-fastbuild/bin/symlink") + .setSymlinkTargetPath("/some/path")) .build()); } @@ -716,7 +1571,9 @@ public void testUnresolvedSymlinkOutputWithInvalidType(@TestParameter OutputsMod defaultTimeout(), defaultSpawnResult()); - closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/symlink").build()); + closeAndAssertLog( + context, + defaultSpawnExecBuilder().addListedOutputs("bazel-out/k8-fastbuild/bin/symlink").build()); } @Test @@ -735,7 +1592,9 @@ public void testMissingOutput(@TestParameter OutputsMode outputsMode) throws Exc defaultTimeout(), defaultSpawnResult()); - closeAndAssertLog(context, defaultSpawnExecBuilder().addListedOutputs("out/missing").build()); + closeAndAssertLog( + context, + defaultSpawnExecBuilder().addListedOutputs("bazel-out/k8-fastbuild/bin/missing").build()); } @Test @@ -986,22 +1845,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 +1912,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 +1922,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 +1955,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..edcefc8ecc9a48 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogReconstructorTest.java @@ -0,0 +1,76 @@ +package com.google.devtools.build.lib.exec; + +import static com.google.common.truth.Truth.assertThat; + +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("bazel-out/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result(null, "pkg/file.txt")); + assertThat(matchDefault("bazel-out/k8-fastbuild/bin/pkg/external/file.txt")) + .isEqualTo(new Result(null, "pkg/external/file.txt")); + assertThat(matchDefault("bazel-out/k8-fastbuild/bin/external/some_repo/pkg/file.txt")) + .isEqualTo(new Result("some_repo", "pkg/file.txt")); + assertThat(matchDefault("bazel-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("bazel-out/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result(null, "pkg/file.txt")); + assertThat(matchSibling("bazel-out/k8-fastbuild/bin/pkg/external/file.txt")) + .isEqualTo(new Result(null, "pkg/external/file.txt")); + assertThat(matchSibling("bazel-out/k8-fastbuild/bin/external/pkg/file.txt")) + .isEqualTo(new Result(null, "external/pkg/file.txt")); + assertThat(matchSibling("bazel-out/some_repo/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result("some_repo", "pkg/file.txt")); + assertThat(matchSibling("bazel-out/some-repo+/k8-fastbuild/bin/pkg/file.txt")) + .isEqualTo(new Result("some-repo+", "pkg/file.txt")); + assertThat(matchSibling("bazel-out/k8-fastbuild/coverage-metadata/bin/other/pkg/gen.txt")) + .isEqualTo(new Result(null, "bin/other/pkg/gen.txt")); + assertThat( + matchSibling( + "bazel-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..35a7f76f901c13 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 Iterable 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; + } }; }