diff options
35 files changed, 297 insertions, 156 deletions
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 cd5085d947..c67645d41b 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 @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.actions; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; @@ -27,6 +28,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.common.options.OptionsClassProvider; @@ -47,10 +49,14 @@ public class ActionExecutionContext implements Closeable { private final MetadataHandler metadataHandler; private final FileOutErr fileOutErr; private final ImmutableMap<String, String> clientEnv; + private final ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> + inputFilesetMappings; private final ArtifactExpander artifactExpander; @Nullable private final Environment env; + @Nullable private ImmutableList<FilesetOutputSymlink> outputSymlinks; + private ActionExecutionContext( Executor executor, ActionInputFileCache actionInputFileCache, @@ -59,6 +65,7 @@ public class ActionExecutionContext implements Closeable { MetadataHandler metadataHandler, FileOutErr fileOutErr, Map<String, String> clientEnv, + ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings, @Nullable ArtifactExpander artifactExpander, @Nullable SkyFunction.Environment env) { this.actionInputFileCache = actionInputFileCache; @@ -67,6 +74,7 @@ public class ActionExecutionContext implements Closeable { this.metadataHandler = metadataHandler; this.fileOutErr = fileOutErr; this.clientEnv = ImmutableMap.copyOf(clientEnv); + this.inputFilesetMappings = inputFilesetMappings; this.executor = executor; this.artifactExpander = artifactExpander; this.env = env; @@ -80,6 +88,7 @@ public class ActionExecutionContext implements Closeable { MetadataHandler metadataHandler, FileOutErr fileOutErr, Map<String, String> clientEnv, + ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings, ArtifactExpander artifactExpander) { this( executor, @@ -89,6 +98,7 @@ public class ActionExecutionContext implements Closeable { metadataHandler, fileOutErr, clientEnv, + inputFilesetMappings, artifactExpander, null); } @@ -110,6 +120,7 @@ public class ActionExecutionContext implements Closeable { metadataHandler, fileOutErr, clientEnv, + ImmutableMap.of(), null, env); } @@ -176,6 +187,24 @@ public class ActionExecutionContext implements Closeable { return executor.getEventHandler(); } + public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getInputFilesetMappings() { + return inputFilesetMappings; + } + + @Nullable + public ImmutableList<FilesetOutputSymlink> getOutputSymlinks() { + return outputSymlinks; + } + + public void setOutputSymlinks(ImmutableList<FilesetOutputSymlink> outputSymlinks) { + Preconditions.checkState( + this.outputSymlinks == null, + "Unexpected reassignment of the outputSymlinks of a Fileset from\n:%s to:\n%s", + this.outputSymlinks, + outputSymlinks); + this.outputSymlinks = outputSymlinks; + } + /** * Looks up and returns an action context implementation of the given interface type. */ @@ -259,6 +288,7 @@ public class ActionExecutionContext implements Closeable { metadataHandler, fileOutErr, clientEnv, + inputFilesetMappings, artifactExpander, env); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java index 6a54212377..c3bcb092a7 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContextFactory.java @@ -13,7 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.actions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.cache.MetadataHandler; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; import java.util.Map; @@ -27,6 +30,9 @@ public interface ActionExecutionContextFactory { * Returns an action execution context. This involves creating a new FileOutErr, and it is the * caller's responsibility to close that, e.g. by calling {@link ActionExecutionContext#close}. */ - ActionExecutionContext getContext(ActionInputFileCache graphFileCache, - MetadataHandler metadataHandler, Map<Artifact, Collection<Artifact>> expandedInputs); + ActionExecutionContext getContext( + ActionInputFileCache graphFileCache, + MetadataHandler metadataHandler, + Map<Artifact, Collection<Artifact>> expandedInputs, + ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index 437f4b1773..aa49f2f354 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -81,11 +81,6 @@ public class BaseSpawn implements Spawn { } @Override - public ImmutableList<Artifact> getFilesetManifests() { - return ImmutableList.<Artifact>of(); - } - - @Override public ImmutableList<String> getArguments() { // TODO(bazel-team): this method should be final, as the correct value of the args can be // injected in the ctor. @@ -93,6 +88,11 @@ public class BaseSpawn implements Spawn { } @Override + public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() { + return ImmutableMap.of(); + } + + @Override public ImmutableMap<String, String> getEnvironment() { PathFragment runfilesRoot = getRunfilesRoot(); if (runfilesRoot == null diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java index eb09c6ee8e..3b6447a9f8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; import javax.annotation.Nullable; @@ -38,11 +39,6 @@ public class DelegateSpawn implements Spawn { } @Override - public ImmutableList<Artifact> getFilesetManifests() { - return spawn.getFilesetManifests(); - } - - @Override public RunfilesSupplier getRunfilesSupplier() { return spawn.getRunfilesSupplier(); } @@ -58,6 +54,11 @@ public class DelegateSpawn implements Spawn { } @Override + public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() { + return spawn.getFilesetMappings(); + } + + @Override public Iterable<? extends ActionInput> getToolFiles() { return spawn.getToolFiles(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java index 4daffaf209..c5abd15869 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java @@ -18,6 +18,8 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Map; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -34,7 +36,7 @@ public final class SimpleSpawn implements Spawn { private final ImmutableList<? extends ActionInput> inputs; private final ImmutableList<? extends ActionInput> tools; private final RunfilesSupplier runfilesSupplier; - private final ImmutableList<Artifact> filesetManifests; + private final Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings; private final ImmutableList<? extends ActionInput> outputs; private final ResourceSet localResources; @@ -44,9 +46,9 @@ public final class SimpleSpawn implements Spawn { ImmutableMap<String, String> environment, ImmutableMap<String, String> executionInfo, RunfilesSupplier runfilesSupplier, + Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings, ImmutableList<? extends ActionInput> inputs, ImmutableList<? extends ActionInput> tools, - ImmutableList<Artifact> filesetManifests, ImmutableList<? extends ActionInput> outputs, ResourceSet localResources) { this.owner = Preconditions.checkNotNull(owner); @@ -57,7 +59,7 @@ public final class SimpleSpawn implements Spawn { this.tools = Preconditions.checkNotNull(tools); this.runfilesSupplier = runfilesSupplier == null ? EmptyRunfilesSupplier.INSTANCE : runfilesSupplier; - this.filesetManifests = Preconditions.checkNotNull(filesetManifests); + this.filesetMappings = filesetMappings; this.outputs = Preconditions.checkNotNull(outputs); this.localResources = Preconditions.checkNotNull(localResources); } @@ -76,9 +78,9 @@ public final class SimpleSpawn implements Spawn { environment, executionInfo, null, + ImmutableMap.of(), inputs, ImmutableList.<Artifact>of(), - ImmutableList.<Artifact>of(), outputs, localResources); } @@ -94,11 +96,6 @@ public final class SimpleSpawn implements Spawn { } @Override - public ImmutableList<Artifact> getFilesetManifests() { - return filesetManifests; - } - - @Override public ImmutableList<String> getArguments() { return arguments; } @@ -109,6 +106,11 @@ public final class SimpleSpawn implements Spawn { } @Override + public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() { + return ImmutableMap.copyOf(filesetMappings); + } + + @Override public ImmutableList<? extends ActionInput> getInputFiles() { return inputs; } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index 7e51517483..0440d22db5 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; import javax.annotation.Nullable; @@ -43,11 +44,6 @@ public interface Spawn { RunfilesSupplier getRunfilesSupplier(); /** - * Returns artifacts for filesets, so they can be scheduled on remote execution. - */ - ImmutableList<Artifact> getFilesetManifests(); - - /** * Returns the command (the first element) and its arguments. */ ImmutableList<String> getArguments(); @@ -59,6 +55,12 @@ public interface Spawn { ImmutableMap<String, String> getEnvironment(); /** + * Map of the execpath at which we expect the Fileset symlink trees, to a list of + * FilesetOutputSymlinks which contains the details of the Symlink trees. + */ + ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings(); + + /** * Returns the list of files that are required to execute this spawn (e.g. the compiler binary), * in contrast to files necessary for the tool to do its work (e.g. source code to be compiled). * diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index a7a56c32a7..77785abc12 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier; import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.ParamFileInfo; import com.google.devtools.build.lib.actions.ParameterFile; import com.google.devtools.build.lib.actions.ResourceSet; @@ -340,10 +341,11 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie * * <p>This method is final, as it is merely a shorthand use of the generic way to obtain a spawn, * which also depends on the client environment. Subclasses that which to override the way to get - * a spawn should override {@link #getSpawn()} instead. + * a spawn should override the other GetSpawn() methods instead. */ public final Spawn getSpawn() throws CommandLineExpansionException { - return new ActionSpawn(commandLines.allArguments(), null, ImmutableList.of()); + return new ActionSpawn( + commandLines.allArguments(), null, ImmutableList.of(), ImmutableMap.of()); } /** @@ -353,17 +355,23 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie public Spawn getSpawn(ActionExecutionContext actionExecutionContext) throws CommandLineExpansionException { return getSpawn( - actionExecutionContext.getArtifactExpander(), actionExecutionContext.getClientEnv()); + actionExecutionContext.getArtifactExpander(), + actionExecutionContext.getClientEnv(), + actionExecutionContext.getInputFilesetMappings()); } - Spawn getSpawn(ArtifactExpander artifactExpander, Map<String, String> clientEnv) + Spawn getSpawn( + ArtifactExpander artifactExpander, + Map<String, String> clientEnv, + Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings) throws CommandLineExpansionException { ExpandedCommandLines expandedCommandLines = commandLines.expand(artifactExpander, getPrimaryOutput().getExecPath(), commandLineLimits); return new ActionSpawn( ImmutableList.copyOf(expandedCommandLines.arguments()), clientEnv, - expandedCommandLines.getParamFiles()); + expandedCommandLines.getParamFiles(), + filesetMappings); } @Override @@ -497,7 +505,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private class ActionSpawn extends BaseSpawn { private final ImmutableList<ActionInput> inputs; - private final ImmutableList<Artifact> filesets; + private final Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings; private final ImmutableMap<String, String> effectiveEnvironment; /** @@ -509,7 +517,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie private ActionSpawn( ImmutableList<String> arguments, Map<String, String> clientEnv, - Iterable<? extends ActionInput> additionalInputs) { + Iterable<? extends ActionInput> additionalInputs, + Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings) { super( arguments, ImmutableMap.<String, String>of(), @@ -518,18 +527,15 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie SpawnAction.this, resourceSet); ImmutableList.Builder<ActionInput> inputs = ImmutableList.builder(); - ImmutableList.Builder<Artifact> filesets = ImmutableList.builder(); ImmutableList<Artifact> manifests = getRunfilesSupplier().getManifests(); for (Artifact input : getInputs()) { - if (input.isFileset()) { - filesets.add(input); - } else if (!manifests.contains(input)) { + if (!input.isFileset() && !manifests.contains(input)) { inputs.add(input); } } inputs.addAll(additionalInputs); - this.filesets = filesets.build(); this.inputs = inputs.build(); + this.filesetMappings = filesetMappings; LinkedHashMap<String, String> env = new LinkedHashMap<>(SpawnAction.this.getEnvironment()); if (clientEnv != null) { for (String var : SpawnAction.this.getClientEnvironmentVariables()) { @@ -550,8 +556,8 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie } @Override - public ImmutableList<Artifact> getFilesetManifests() { - return filesets; + public ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> getFilesetMappings() { + return ImmutableMap.copyOf(filesetMappings); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index c516b7875a..d9ae236622 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; -import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; @@ -233,11 +232,11 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte @Override public SortedMap<PathFragment, ActionInput> getInputMapping() throws IOException { if (lazyInputMapping == null) { - lazyInputMapping = spawnInputExpander.getInputMapping( - spawn, - actionExecutionContext.getArtifactExpander(), - actionExecutionContext.getActionInputFileCache(), - actionExecutionContext.getContext(FilesetActionContext.class)); + lazyInputMapping = + spawnInputExpander.getInputMapping( + spawn, + actionExecutionContext.getArtifactExpander(), + actionExecutionContext.getActionInputFileCache()); } return lazyInputMapping; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java index 319ea6bfa8..05dee2b1b8 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java +++ b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java @@ -16,7 +16,7 @@ package com.google.devtools.build.lib.exec; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.io.LineProcessor; -import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -25,6 +25,7 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; /** @@ -52,19 +53,6 @@ public final class FilesetManifest { } public static FilesetManifest parseManifestFile( - Artifact manifest, - Path execRoot, - String workspaceName, - RelativeSymlinkBehavior relSymlinkBehavior) - throws IOException { - return parseManifestFile( - manifest.getExecPath(), - execRoot, - workspaceName, - relSymlinkBehavior); - } - - public static FilesetManifest parseManifestFile( PathFragment manifest, Path execRoot, String workspaceName, @@ -82,13 +70,29 @@ public final class FilesetManifest { } } + public static FilesetManifest constructFilesetManifest( + List<FilesetOutputSymlink> outputSymlinks, + PathFragment targetPrefix, + RelativeSymlinkBehavior relSymlinkbehavior) + throws IOException { + LinkedHashMap<PathFragment, String> entries = new LinkedHashMap<>(); + Map<PathFragment, String> relativeLinks = new HashMap<>(); + for (FilesetOutputSymlink outputSymlink : outputSymlinks) { + PathFragment fullLocation = targetPrefix.getRelative(outputSymlink.name); + String artifact = outputSymlink.target.getPathString(); + artifact = artifact.isEmpty() ? null : artifact; + addSymlinkEntry(artifact, fullLocation, relSymlinkbehavior, entries, relativeLinks); + } + return constructFilesetManifest(entries, relativeLinks); + } + private static final class ManifestLineProcessor implements LineProcessor<FilesetManifest> { private final String workspaceName; private final PathFragment targetPrefix; private final RelativeSymlinkBehavior relSymlinkBehavior; private int lineNum; - private final Map<PathFragment, String> entries = new LinkedHashMap<>(); + private final LinkedHashMap<PathFragment, String> entries = new LinkedHashMap<>(); private final Map<PathFragment, String> relativeLinks = new HashMap<>(); ManifestLineProcessor( @@ -134,44 +138,59 @@ public final class FilesetManifest { } PathFragment fullLocation = targetPrefix.getRelative(location); - if (!entries.containsKey(fullLocation)) { - boolean isRelativeSymlink = artifact != null && !artifact.startsWith("/"); - if (isRelativeSymlink && relSymlinkBehavior.equals(RelativeSymlinkBehavior.ERROR)) { - throw new IOException(String.format("runfiles target is not absolute: %s", artifact)); - } - if (!isRelativeSymlink - || relSymlinkBehavior.equals(RelativeSymlinkBehavior.RESOLVE)) { - entries.put(fullLocation, artifact); - if (artifact != null && !artifact.startsWith("/")) { - relativeLinks.put(fullLocation, artifact); - } - } - } + addSymlinkEntry(artifact, fullLocation, relSymlinkBehavior, entries, relativeLinks); return true; } @Override public FilesetManifest getResult() { - // Resolve relative symlinks if possible. Note that relativeLinks only contains entries in - // RESOLVE mode. - for (Map.Entry<PathFragment, String> e : relativeLinks.entrySet()) { - PathFragment location = e.getKey(); - String value = e.getValue(); - PathFragment actualLocation = location.getParentDirectory().getRelative(value); - String actual = entries.get(actualLocation); - boolean isActualAcceptable = actual == null || actual.startsWith("/"); - if (!entries.containsKey(actualLocation) || !isActualAcceptable) { - throw new IllegalStateException( - String.format( - "runfiles target '%s' is not absolute, and could not be resolved in the same " - + "Fileset", value)); + return constructFilesetManifest(entries, relativeLinks); + } + } + + private static void addSymlinkEntry( + String artifact, + PathFragment fullLocation, + RelativeSymlinkBehavior relSymlinkBehavior, + LinkedHashMap<PathFragment, String> entries, + Map<PathFragment, String> relativeLinks) + throws IOException { + if (!entries.containsKey(fullLocation)) { + boolean isRelativeSymlink = artifact != null && !artifact.startsWith("/"); + if (isRelativeSymlink && relSymlinkBehavior.equals(RelativeSymlinkBehavior.ERROR)) { + throw new IOException(String.format("runfiles target is not absolute: %s", artifact)); + } + if (!isRelativeSymlink || relSymlinkBehavior.equals(RelativeSymlinkBehavior.RESOLVE)) { + entries.put(fullLocation, artifact); + if (artifact != null && !artifact.startsWith("/")) { + relativeLinks.put(fullLocation, artifact); } - entries.put(location, actual); } - return new FilesetManifest(entries); } } + private static FilesetManifest constructFilesetManifest( + Map<PathFragment, String> entries, Map<PathFragment, String> relativeLinks) { + // Resolve relative symlinks if possible. Note that relativeLinks only contains entries in + // RESOLVE mode. + for (Map.Entry<PathFragment, String> e : relativeLinks.entrySet()) { + PathFragment location = e.getKey(); + String value = e.getValue(); + PathFragment actualLocation = location.getParentDirectory().getRelative(value); + String actual = entries.get(actualLocation); + boolean isActualAcceptable = actual == null || actual.startsWith("/"); + if (!entries.containsKey(actualLocation) || !isActualAcceptable) { + throw new IllegalStateException( + String.format( + "runfiles target '%s' is not absolute, and could not be resolved in the same " + + "Fileset", + value)); + } + entries.put(location, actual); + } + return new FilesetManifest(entries); + } + private final Map<PathFragment, String> entries; private FilesetManifest(Map<PathFragment, String> entries) { 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 f57968984d..cff57d121f 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 @@ -17,16 +17,16 @@ import static com.google.devtools.build.lib.exec.FilesetManifest.RelativeSymlink import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; -import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; -import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; @@ -109,12 +109,13 @@ public class SpawnInputExpander { * Parses the fileset manifest file, adding to the inputMappings where appropriate. Lines * referring to directories are recursed. */ + // TODO(kush): make tests use the method with in-memory fileset data. @VisibleForTesting void parseFilesetManifest( Map<PathFragment, ActionInput> inputMappings, Artifact manifest, String workspaceName) throws IOException { FilesetManifest filesetManifest = - FilesetManifest.parseManifestFile(manifest, execRoot, workspaceName, ERROR); + FilesetManifest.parseManifestFile(manifest.getExecPath(), execRoot, workspaceName, ERROR); for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) { String value = mapping.getValue(); ActionInput artifact = value == null ? EMPTY_FILE : ActionInputHelper.fromPath(value); @@ -122,6 +123,23 @@ public class SpawnInputExpander { } } + public void addFilesetManifests( + Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings, + Map<PathFragment, ActionInput> inputMappings) + throws IOException { + for (PathFragment manifestExecpath : filesetMappings.keySet()) { + ImmutableList<FilesetOutputSymlink> outputSymlinks = filesetMappings.get(manifestExecpath); + FilesetManifest filesetManifest = + FilesetManifest.constructFilesetManifest(outputSymlinks, manifestExecpath, ERROR); + + for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) { + String value = mapping.getValue(); + ActionInput artifact = value == null ? EMPTY_FILE : ActionInputHelper.fromPath(value); + addMapping(inputMappings, mapping.getKey(), artifact); + } + } + } + private void addInputs( Map<PathFragment, ActionInput> inputMap, Spawn spawn, ArtifactExpander artifactExpander) { List<ActionInput> inputs = @@ -134,34 +152,17 @@ public class SpawnInputExpander { /** * Convert the inputs of the given spawn to a map from exec-root relative paths to action inputs. * The returned map never contains {@code null} values; it uses {@link #EMPTY_FILE} for empty - * files, which is an instance of {@link VirtualActionInput}. - */ - public SortedMap<PathFragment, ActionInput> getInputMapping( - Spawn spawn, ArtifactExpander artifactExpander, ActionInputFileCache actionInputFileCache, - FilesetActionContext filesetContext) - throws IOException { - return getInputMapping( - spawn, - artifactExpander, - actionInputFileCache, - filesetContext == null ? null : filesetContext.getWorkspaceName()); - } - - /** - * Convert the inputs of the given spawn to a map from exec-root relative paths to action inputs. - * In some cases, this generates empty files, for which it uses {@code null}. + * files, which is an instance of {@link + * com.google.devtools.build.lib.actions.cache.VirtualActionInput}. */ public SortedMap<PathFragment, ActionInput> getInputMapping( - Spawn spawn, ArtifactExpander artifactExpander, ActionInputFileCache actionInputFileCache, - String workspaceName) - throws IOException { + Spawn spawn, ArtifactExpander artifactExpander, ActionInputFileCache actionInputFileCache) + throws IOException { TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>(); addInputs(inputMap, spawn, artifactExpander); addRunfilesToInputs( inputMap, spawn.getRunfilesSupplier(), actionInputFileCache); - for (Artifact manifest : spawn.getFilesetManifests()) { - parseFilesetManifest(inputMap, manifest, workspaceName); - } + addFilesetManifests(spawn.getFilesetMappings(), inputMap); return inputMap; } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index c1f04356cc..c1248a6e4b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -124,9 +124,9 @@ public class StandaloneTestStrategy extends TestStrategy { ImmutableMap.copyOf(executionInfo), new RunfilesSupplierImpl( runfilesDir.relativeTo(execRoot), action.getExecutionSettings().getRunfiles()), + ImmutableMap.of(), /*inputs=*/ ImmutableList.copyOf(action.getInputs()), /*tools=*/ ImmutableList.<Artifact>of(), - /*filesetManifests=*/ ImmutableList.<Artifact>of(), ImmutableList.copyOf(action.getSpawnOutputs()), localResourceUsage); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java index 5d29cb7c5c..fe35d84f29 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java @@ -55,9 +55,9 @@ public class SpawnGccStrategy implements CppCompileActionContext { ImmutableMap.copyOf(action.getEnvironment()), ImmutableMap.copyOf(action.getExecutionInfo()), EmptyRunfilesSupplier.INSTANCE, + ImmutableMap.of(), ImmutableList.copyOf(inputs), /* tools= */ ImmutableList.of(), - /* filesetManifests= */ ImmutableList.of(), action.getOutputs().asList(), action.estimateResourceConsumptionLocal()); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 77f7254639..aca1c5c8a6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -17,6 +17,7 @@ import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; @@ -27,8 +28,10 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.ActionLookupValue; +import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.MissingInputFileException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.actions.PackageRootResolver; @@ -223,8 +226,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver * @throws ActionExecutionFunctionException */ @Nullable - private AllInputs collectInputs(Action action, Environment env) - throws ActionExecutionFunctionException, InterruptedException { + private AllInputs collectInputs(Action action, Environment env) throws InterruptedException { Iterable<Artifact> allKnownInputs = Iterables.concat( action.getInputs(), action.getRunfilesSupplier().getArtifacts()); if (action.inputsDiscovered()) { @@ -373,10 +375,16 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver if (state.token == null) { // We got a hit from the action cache -- no need to execute. + Preconditions.checkState( + !(action instanceof SkyframeAwareAction), + "Error, we're not re-executing a " + + "SkyframeAwareAction which should be re-executed unconditionally. Action: %s", + action); return new ActionExecutionValue( metadataHandler.getOutputArtifactData(), metadataHandler.getOutputTreeArtifactData(), - metadataHandler.getAdditionalOutputData()); + metadataHandler.getAdditionalOutputData(), + /*outputSymlinks=*/ null); } // Delete the metadataHandler's cache of the action's outputs, since they are being deleted. @@ -429,11 +437,35 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver metadataHandler.discardOutputMetadata(); } + ImmutableMap.Builder<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings = + ImmutableMap.builder(); + for (Artifact actionInput : action.getInputs()) { + if (!actionInput.isFileset()) { + continue; + } + + ActionLookupKey filesetActionLookupKey = (ActionLookupKey) actionInput.getArtifactOwner(); + // Index 0 for the Fileset ConfiguredTarget indicates the SkyframeFilesetManifestAction where + // we compute the fileset's outputSymlinks. + SkyKey filesetActionKey = ActionExecutionValue.key(filesetActionLookupKey, 0); + ActionExecutionValue filesetValue = (ActionExecutionValue) env.getValue(filesetActionKey); + if (filesetValue == null) { + // At this point skyframe does not guarantee that the filesetValue will be ready, since + // the current action does not directly depend on the outputs of the + // SkyframeFilesetManifestAction whose ActionExecutionValue (filesetValue) is needed here. + // TODO(kush): Get rid of this hack by making the outputSymlinks available in the Fileset + // artifact, which this action depends on, so its value will be guaranteed to be present. + return null; + } + filesetMappings.put(actionInput.getExecPath(), filesetValue.getOutputSymlinks()); + } + try (ActionExecutionContext actionExecutionContext = skyframeActionExecutor.getContext( perActionFileCache, metadataHandler, - Collections.unmodifiableMap(state.expandedArtifacts))) { + Collections.unmodifiableMap(state.expandedArtifacts), + filesetMappings.build())) { if (!state.hasExecutedAction()) { state.value = skyframeActionExecutor.executeAction( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java index 105886a845..7c7150bffb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java @@ -17,10 +17,12 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.ActionLookupValue; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.skyframe.SkyKey; @@ -63,22 +65,27 @@ public class ActionExecutionValue implements SkyValue { */ private final ImmutableMap<Artifact, FileArtifactValue> additionalOutputData; + @Nullable private final ImmutableList<FilesetOutputSymlink> outputSymlinks; + /** * @param artifactData Map from Artifacts to corresponding FileValues. * @param treeArtifactData All tree artifact data. * @param additionalOutputData Map from Artifacts to values if the FileArtifactValue for this * artifact cannot be derived from the corresponding FileValue (see {@link - * ActionMetadataHandler#getAdditionalOutputData} for when this is necessary). - * These output data are not used by the {@link FilesystemValueChecker} - * to invalidate ActionExecutionValues. + * ActionMetadataHandler#getAdditionalOutputData} for when this is necessary). These output + * data are not used by the {@link FilesystemValueChecker} to invalidate + * ActionExecutionValues. + * @param outputSymlinks This represents the SymlinkTree which is the output of a fileset action. */ ActionExecutionValue( Map<Artifact, FileValue> artifactData, Map<Artifact, TreeArtifactValue> treeArtifactData, - Map<Artifact, FileArtifactValue> additionalOutputData) { + Map<Artifact, FileArtifactValue> additionalOutputData, + @Nullable ImmutableList<FilesetOutputSymlink> outputSymlinks) { this.artifactData = ImmutableMap.<Artifact, FileValue>copyOf(artifactData); this.additionalOutputData = ImmutableMap.copyOf(additionalOutputData); this.treeArtifactData = ImmutableMap.copyOf(treeArtifactData); + this.outputSymlinks = outputSymlinks; } /** @@ -124,6 +131,11 @@ public class ActionExecutionValue implements SkyValue { return treeArtifactData; } + @Nullable + ImmutableList<FilesetOutputSymlink> getOutputSymlinks() { + return outputSymlinks; + } + /** * @param lookupKey A {@link SkyKey} whose argument is an {@code ActionLookupKey}, whose * corresponding {@code ActionLookupValue} contains the action to be executed. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 9fd7ce9e33..221fc9206d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -17,6 +17,7 @@ import static com.google.devtools.build.lib.vfs.FileSystemUtils.createDirectoryA import com.google.common.base.Preconditions; import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.eventbus.EventBus; @@ -55,6 +56,7 @@ import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; import com.google.devtools.build.lib.actions.CachedActionEvent; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.MapBasedActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; @@ -481,8 +483,10 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto */ @Override public ActionExecutionContext getContext( - ActionInputFileCache graphFileCache, MetadataHandler metadataHandler, - Map<Artifact, Collection<Artifact>> expandedInputs) { + ActionInputFileCache graphFileCache, + MetadataHandler metadataHandler, + Map<Artifact, Collection<Artifact>> expandedInputs, + ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings) { FileOutErr fileOutErr = actionLogBufferPathGenerator.generate(); return new ActionExecutionContext( executorEngine, @@ -492,6 +496,7 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto metadataHandler, fileOutErr, clientEnv, + inputFilesetMappings, new ArtifactExpanderImpl(expandedInputs)); } @@ -706,10 +711,18 @@ public final class SkyframeActionExecutor implements ActionExecutionContextFacto "%s %s", actionExecutionContext.getMetadataHandler(), metadataHandler); prepareScheduleExecuteAndCompleteAction( eventHandler, action, actionExecutionContext, actionStartTime, actionLookupData); + Preconditions.checkState( + actionExecutionContext.getOutputSymlinks() == null + || action instanceof SkyframeAwareAction, + "Unexpected to find outputSymlinks set" + + " in an action which is not a SkyframeAwareAction. Action: %s\n symlinks:%s", + action, + actionExecutionContext.getOutputSymlinks()); return new ActionExecutionValue( metadataHandler.getOutputArtifactData(), metadataHandler.getOutputTreeArtifactData(), - metadataHandler.getAdditionalOutputData()); + metadataHandler.getAdditionalOutputData(), + actionExecutionContext.getOutputSymlinks()); } finally { profiler.completeTask(ProfilerTask.ACTION); } diff --git a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java index 83e3f31030..6124b914b8 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java @@ -64,6 +64,7 @@ public class ExecutableSymlinkActionTest { null, outErr, ImmutableMap.<String, String>of(), + ImmutableMap.of(), null); } diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 753a73e221..29e9c230c7 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -143,6 +143,7 @@ public final class ActionsTestUtil { metadataHandler, fileOutErr, ImmutableMap.copyOf(clientEnv), + ImmutableMap.of(), actionGraph == null ? createDummyArtifactExpander() : ActionInputHelper.actionGraphArtifactExpander(actionGraph)); @@ -177,6 +178,7 @@ public final class ActionsTestUtil { null, null, ImmutableMap.of(), + ImmutableMap.of(), createDummyArtifactExpander()); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java index 84acbdf725..147c6922a6 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java @@ -66,6 +66,7 @@ public abstract class FileWriteActionTestCase extends BuildViewTestCase { null, new FileOutErr(), ImmutableMap.<String, String>of(), + ImmutableMap.of(), null); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java index b72c51ea55..a212893fac 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java @@ -178,6 +178,7 @@ public class ParamFileWriteActionTest extends BuildViewTestCase { null, new FileOutErr(), ImmutableMap.<String, String>of(), + ImmutableMap.of(), artifactExpander); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java index 48428ecbe4..2fdc15f929 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java @@ -191,7 +191,9 @@ public class SpawnActionTest extends BuildViewTestCase { "/bin/java", "-Xverify:none", "-jvmarg", "-cp", "pkg/exe.jar", "MyMainClass", "-X") .inOrder(); - Spawn spawn = action.getSpawn((artifact, outputs) -> outputs.add(artifact), ImmutableMap.of()); + Spawn spawn = + action.getSpawn( + (artifact, outputs) -> outputs.add(artifact), ImmutableMap.of(), ImmutableMap.of()); String paramFileName = output.getExecPathString() + "-0.params"; // The spawn's primary arguments should reference the param file assertThat(spawn.getArguments()) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java index 81979db7e7..75446137cd 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java @@ -88,6 +88,7 @@ public class SymlinkActionTest extends BuildViewTestCase { null, null, ImmutableMap.<String, String>of(), + ImmutableMap.of(), null)); assertThat(actionResult.spawnResults()).isEmpty(); assertThat(output.isSymbolicLink()).isTrue(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java index b2e3006f30..9f2cd5ea04 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java @@ -191,6 +191,7 @@ public class TemplateExpansionActionTest extends FoundationTestCase { null, new FileOutErr(), ImmutableMap.<String, String>of(), + ImmutableMap.of(), null); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index f80db7e6d1..eaca063305 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -2090,6 +2090,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { /*metadataHandler=*/ null, actionLogBufferPathGenerator.generate(), clientEnv, + ImmutableMap.of(), artifactExpander); } } diff --git a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java index fc13b44653..391cd16d41 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/FilesetManifestTest.java @@ -62,7 +62,7 @@ public class FilesetManifestTest { Artifact artifact = new Artifact(fs.getPath("/root/foo"), ArtifactRoot.asSourceRoot(Root.fromPath(execRoot))); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()).isEmpty(); } @@ -78,7 +78,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()) .containsExactly(PathFragment.create("out/foo/bar"), "/dir/file"); } @@ -97,7 +97,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()) .containsExactly( PathFragment.create("out/foo/bar"), "/dir/file", @@ -116,7 +116,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()) .containsExactly(PathFragment.create("out/foo/bar"), "/some"); } @@ -134,7 +134,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()).containsExactly(PathFragment.create("out/foo/bar"), null); } @@ -150,7 +150,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); try { - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); fail(); } catch (IOException e) { assertThat(e) @@ -173,7 +173,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); try { - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", ERROR); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", ERROR); fail(); } catch (IOException e) { assertThat(e).hasMessageThat().isEqualTo("runfiles target is not absolute: foo"); @@ -194,7 +194,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()) .containsExactly(PathFragment.create("out/foo/foo"), "/foo/bar"); } @@ -213,7 +213,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", RESOLVE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", RESOLVE); assertThat(manifest.getEntries()) .containsExactly( PathFragment.create("out/foo/bar"), "/foo/bar", @@ -234,7 +234,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", RESOLVE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", RESOLVE); assertThat(manifest.getEntries()) .containsExactly( PathFragment.create("out/foo/bar"), "/foo/bar", @@ -255,7 +255,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", RESOLVE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", RESOLVE); assertThat(manifest.getEntries()) .containsExactly( PathFragment.create("out/foo/bar/bar"), "/foo/bar", @@ -274,7 +274,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); try { - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", RESOLVE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", RESOLVE); fail(); } catch (IOException e) { assertThat(e).hasMessageThat() @@ -299,7 +299,7 @@ public class FilesetManifestTest { ArtifactRoot.asDerivedRoot(fs.getPath("/root"), fs.getPath("/root/out")); Artifact artifact = new Artifact(fs.getPath("/root/out/foo"), outputRoot); FilesetManifest manifest = - FilesetManifest.parseManifestFile(artifact, execRoot, "workspace", IGNORE); + FilesetManifest.parseManifestFile(artifact.getExecPath(), execRoot, "workspace", IGNORE); assertThat(manifest.getEntries()) .containsExactly( PathFragment.create("out/foo/bar"), "/foo/bar"); diff --git a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java index b98af7ea2a..1c51ca370a 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java +++ b/src/test/java/com/google/devtools/build/lib/exec/util/SpawnBuilder.java @@ -54,10 +54,10 @@ public final class SpawnBuilder { ImmutableList.copyOf(args), ImmutableMap.copyOf(environment), ImmutableMap.copyOf(executionInfo), - /*runfilesSupplier=*/EmptyRunfilesSupplier.INSTANCE, + /*runfilesSupplier=*/ EmptyRunfilesSupplier.INSTANCE, + ImmutableMap.of(), ImmutableList.copyOf(inputs), - /*tools=*/ImmutableList.<Artifact>of(), - /*filesetManifests=*/ImmutableList.<Artifact>of(), + /*tools=*/ ImmutableList.<Artifact>of(), ImmutableList.copyOf(outputs), ResourceSet.ZERO); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java index 1e52b0bbbe..c6758c185d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java @@ -173,7 +173,7 @@ public class GrpcRemoteExecutionClientTest { @Override public SortedMap<PathFragment, ActionInput> getInputMapping() throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache, "workspace"); + .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 5304562f99..997d3dc67d 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -141,7 +141,7 @@ public class RemoteSpawnCacheTest { @Override public SortedMap<PathFragment, ActionInput> getInputMapping() throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache, "workspace"); + .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, fakeFileCache); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index ebf3f5a0d0..3c8293a0f5 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -968,7 +968,7 @@ public class RemoteSpawnRunnerTest { @Override public SortedMap<PathFragment, ActionInput> getInputMapping() throws IOException { return new SpawnInputExpander(execRoot, /*strict*/ false) - .getInputMapping(spawn, artifactExpander, fakeFileCache, "workspace"); + .getInputMapping(spawn, artifactExpander, fakeFileCache); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java index 75d05ce460..9408edab41 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java @@ -115,7 +115,7 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase { private ActionExecutionContext makeDummyContext() { DummyExecutor executor = new DummyExecutor(fileSystem, rootDirectory); return new ActionExecutionContext( - executor, null, null, null, null, null, ImmutableMap.of(), null); + executor, null, null, null, null, null, ImmutableMap.of(), ImmutableMap.of(), null); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java index dbf62828a2..59fd9bfbae 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java @@ -87,6 +87,7 @@ public class LtoBackendActionTest extends BuildViewTestCase { null, new FileOutErr(), ImmutableMap.<String, String>of(), + ImmutableMap.of(), null); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java index 88d1babc68..d12ce09c2e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java @@ -743,6 +743,7 @@ public class BazelJ2ObjcLibraryTest extends J2ObjcLibraryTest { null, null, ImmutableMap.<String, String>of(), + ImmutableMap.of(), DUMMY_ARTIFACT_EXPANDER); ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream(); ByteArrayOutputStream umbrellaHeaderStream = new ByteArrayOutputStream(); @@ -792,6 +793,7 @@ public class BazelJ2ObjcLibraryTest extends J2ObjcLibraryTest { null, null, ImmutableMap.<String, String>of(), + ImmutableMap.of(), DUMMY_ARTIFACT_EXPANDER); ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java index 2fa5749af0..96fd9fc9eb 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java @@ -376,9 +376,7 @@ public class ArtifactFunctionTest extends ArtifactFunctionTestCase { throw new IllegalStateException(e); } return new ActionExecutionValue( - artifactData, - treeArtifactData, - additionalOutputData); + artifactData, treeArtifactData, additionalOutputData, /*outputSymlinks=*/ null); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index 014ba2a200..261692338a 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -740,7 +740,8 @@ public class FilesystemValueCheckerTest { return new ActionExecutionValue( artifactData, ImmutableMap.<Artifact, TreeArtifactValue>of(), - ImmutableMap.<Artifact, FileArtifactValue>of()); + ImmutableMap.<Artifact, FileArtifactValue>of(), + /*outputSymlinks=*/ null); } private ActionExecutionValue actionValueWithEmptyDirectory(Artifact emptyDir) { @@ -750,7 +751,8 @@ public class FilesystemValueCheckerTest { return new ActionExecutionValue( ImmutableMap.<Artifact, FileValue>of(), ImmutableMap.of(emptyDir, emptyValue), - ImmutableMap.<Artifact, FileArtifactValue>of()); + ImmutableMap.<Artifact, FileArtifactValue>of(), + /*outputSymlinks=*/ null); } private ActionExecutionValue actionValueWithTreeArtifacts(List<TreeFileArtifact> contents) { @@ -779,8 +781,11 @@ public class FilesystemValueCheckerTest { treeArtifactData.put(dirDatum.getKey(), TreeArtifactValue.create(dirDatum.getValue())); } - return new ActionExecutionValue(fileData, treeArtifactData, - ImmutableMap.<Artifact, FileArtifactValue>of()); + return new ActionExecutionValue( + fileData, + treeArtifactData, + ImmutableMap.<Artifact, FileArtifactValue>of(), + /*outputSymlinks=*/ null); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java index 47ca57b142..54a9302f46 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java @@ -274,7 +274,8 @@ public class TreeArtifactMetadataTest extends ArtifactFunctionTestCase { return new ActionExecutionValue( fileData, ImmutableMap.of(output, treeArtifactValue), - ImmutableMap.<Artifact, FileArtifactValue>of()); + ImmutableMap.<Artifact, FileArtifactValue>of(), + /*outputSymlinks=*/ null); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java index 6a46b8832e..04d416be01 100644 --- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java @@ -191,6 +191,7 @@ public class StandaloneSpawnStrategyTest { null, outErr, ImmutableMap.<String, String>of(), + ImmutableMap.of(), SIMPLE_ARTIFACT_EXPANDER); } |