aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com
diff options
context:
space:
mode:
Diffstat (limited to 'src/main/java/com')
-rw-r--r--src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java51
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java140
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java24
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java51
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java10
6 files changed, 184 insertions, 98 deletions
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 444ffa51b6..0e965b32dd 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
@@ -20,6 +20,8 @@ import com.google.devtools.build.lib.actions.ActionInput;
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.Artifact.TreeFileArtifact;
+import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
@@ -29,6 +31,7 @@ import com.google.devtools.build.lib.exec.FilesetManifest.RelativeSymlinkBehavio
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
@@ -40,8 +43,7 @@ import java.util.TreeMap;
* laid out.
*/
public class SpawnInputExpander {
- @VisibleForTesting
- static final ActionInput EMPTY_FILE = new EmptyActionInput("/dev/null");
+ @VisibleForTesting static final ActionInput EMPTY_FILE = new EmptyActionInput("/dev/null");
private final Path execRoot;
private final boolean strict;
@@ -103,10 +105,11 @@ public class SpawnInputExpander {
void addRunfilesToInputs(
Map<PathFragment, ActionInput> inputMap,
RunfilesSupplier runfilesSupplier,
- MetadataProvider actionFileCache)
+ MetadataProvider actionFileCache,
+ ArtifactExpander artifactExpander)
throws IOException {
- Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings = null;
- rootsAndMappings = runfilesSupplier.getMappings();
+ Map<PathFragment, Map<PathFragment, Artifact>> rootsAndMappings =
+ runfilesSupplier.getMappings();
for (Map.Entry<PathFragment, Map<PathFragment, Artifact>> rootAndMappings :
rootsAndMappings.entrySet()) {
@@ -116,10 +119,23 @@ public class SpawnInputExpander {
PathFragment location = root.getRelative(mapping.getKey());
Artifact localArtifact = mapping.getValue();
if (localArtifact != null) {
- if (strict && !actionFileCache.getMetadata(localArtifact).getType().isFile()) {
- throw new IOException("Not a file: " + localArtifact.getPath().getPathString());
+ Preconditions.checkState(!localArtifact.isMiddlemanArtifact());
+ if (localArtifact.isTreeArtifact()) {
+ List<ActionInput> expandedInputs =
+ ActionInputHelper.expandArtifacts(
+ Collections.singletonList(localArtifact), artifactExpander);
+ for (ActionInput input : expandedInputs) {
+ addMapping(
+ inputMap,
+ location.getRelative(((TreeFileArtifact) input).getParentRelativePath()),
+ input);
+ }
+ } else {
+ if (strict) {
+ failIfDirectory(actionFileCache, localArtifact);
+ }
+ addMapping(inputMap, location, localArtifact);
}
- addMapping(inputMap, location, localArtifact);
} else {
addMapping(inputMap, location, EMPTY_FILE);
}
@@ -127,6 +143,14 @@ public class SpawnInputExpander {
}
}
+ private static void failIfDirectory(MetadataProvider actionFileCache, ActionInput input)
+ throws IOException {
+ FileArtifactValue metadata = actionFileCache.getMetadata(input);
+ if (metadata != null && !metadata.getType().isFile()) {
+ throw new IOException("Not a file: " + input.getExecPathString());
+ }
+ }
+
/**
* Parses the fileset manifest file, adding to the inputMappings where appropriate. Lines
* referring to directories are recursed.
@@ -175,10 +199,15 @@ 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
+ * Convert the inputs and runfiles of the given spawn to a map from exec-root relative paths to
+ * {@link ActionInput}s. The returned map does not contain tree artifacts as they are expanded
+ * to file artifacts.
+ *
+ * <p>The returned map never contains {@code null} values; it uses {@link #EMPTY_FILE} for empty
* files, which is an instance of {@link
* com.google.devtools.build.lib.actions.cache.VirtualActionInput}.
+ *
+ * <p>The returned map contains all runfiles, but not the {@code MANIFEST}.
*/
public SortedMap<PathFragment, ActionInput> getInputMapping(
Spawn spawn, ArtifactExpander artifactExpander, MetadataProvider actionInputFileCache)
@@ -186,7 +215,7 @@ public class SpawnInputExpander {
TreeMap<PathFragment, ActionInput> inputMap = new TreeMap<>();
addInputs(inputMap, spawn, artifactExpander);
addRunfilesToInputs(
- inputMap, spawn.getRunfilesSupplier(), actionInputFileCache);
+ inputMap, spawn.getRunfilesSupplier(), actionInputFileCache, artifactExpander);
addFilesetManifests(spawn.getFilesetMappings(), inputMap);
return inputMap;
}
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 6d8a90d524..38fb99a283 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
@@ -74,14 +74,15 @@ import javax.annotation.Nullable;
/**
* A {@link SkyFunction} that creates {@link ActionExecutionValue}s. There are four points where
* this function can abort due to missing values in the graph:
+ *
* <ol>
* <li>For actions that discover inputs, if missing metadata needed to resolve an artifact from a
- * string input in the action cache.</li>
- * <li>If missing metadata for artifacts in inputs (including the artifacts above).</li>
+ * string input in the action cache.
+ * <li>If missing metadata for artifacts in inputs (including the artifacts above).
* <li>For actions that discover inputs, if missing metadata for inputs discovered prior to
- * execution.</li>
+ * execution.
* <li>For actions that discover inputs, but do so during execution, if missing metadata for
- * inputs discovered during execution.</li>
+ * inputs discovered during execution.
* </ol>
*/
public class ActionExecutionFunction implements SkyFunction, CompletionReceiver {
@@ -90,7 +91,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
private final AtomicReference<TimestampGranularityMonitor> tsgm;
private ConcurrentMap<Action, ContinuationState> stateMap;
- public ActionExecutionFunction(SkyframeActionExecutor skyframeActionExecutor,
+ public ActionExecutionFunction(
+ SkyframeActionExecutor skyframeActionExecutor,
BlazeDirectories directories,
AtomicReference<TimestampGranularityMonitor> tsgm) {
this.skyframeActionExecutor = skyframeActionExecutor;
@@ -100,8 +102,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
}
@Override
- public SkyValue compute(SkyKey skyKey, Environment env) throws ActionExecutionFunctionException,
- InterruptedException {
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws ActionExecutionFunctionException, InterruptedException {
ActionLookupData actionLookupData = (ActionLookupData) skyKey.argument();
ActionLookupValue actionLookupValue =
(ActionLookupValue) env.getValue(actionLookupData.getActionLookupKey());
@@ -159,10 +161,14 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
try {
// Declare deps on known inputs to action. We do this unconditionally to maintain our
// invariant of asking for the same deps each build.
- Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps
- = env.getValuesOrThrow(toKeys(state.allInputs.getAllInputs(),
- action.discoversInputs() ? action.getMandatoryInputs() : null),
- MissingInputFileException.class, ActionExecutionException.class);
+ Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>>
+ inputDeps =
+ env.getValuesOrThrow(
+ toKeys(
+ state.allInputs.getAllInputs(),
+ action.discoversInputs() ? action.getMandatoryInputs() : null),
+ MissingInputFileException.class,
+ ActionExecutionException.class);
if (!sharedActionAlreadyRan && !state.hasArtifactData()) {
// Do we actually need to find our metadata?
@@ -274,10 +280,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
private static class AllInputs {
final Iterable<Artifact> defaultInputs;
- @Nullable
- final Iterable<Artifact> actionCacheInputs;
- @Nullable
- final List<SkyKey> keysRequested;
+ @Nullable final Iterable<Artifact> actionCacheInputs;
+ @Nullable final List<SkyKey> keysRequested;
AllInputs(Iterable<Artifact> defaultInputs) {
this.defaultInputs = Preconditions.checkNotNull(defaultInputs);
@@ -285,7 +289,9 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
this.keysRequested = null;
}
- AllInputs(Iterable<Artifact> defaultInputs, Iterable<Artifact> actionCacheInputs,
+ AllInputs(
+ Iterable<Artifact> defaultInputs,
+ Iterable<Artifact> actionCacheInputs,
List<SkyKey> keysRequested) {
this.defaultInputs = Preconditions.checkNotNull(defaultInputs);
this.actionCacheInputs = Preconditions.checkNotNull(actionCacheInputs);
@@ -314,13 +320,17 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
@Override
public Map<PathFragment, Root> findPackageRootsForFiles(Iterable<PathFragment> execPaths)
throws InterruptedException {
- Preconditions.checkState(keysRequested.isEmpty(),
- "resolver should only be called once: %s %s", keysRequested, execPaths);
+ Preconditions.checkState(
+ keysRequested.isEmpty(),
+ "resolver should only be called once: %s %s",
+ keysRequested,
+ execPaths);
// Create SkyKeys list based on execPaths.
Map<PathFragment, SkyKey> depKeys = new HashMap<>();
for (PathFragment path : execPaths) {
- PathFragment parent = Preconditions.checkNotNull(
- path.getParentDirectory(), "Must pass in files, not root directory");
+ PathFragment parent =
+ Preconditions.checkNotNull(
+ path.getParentDirectory(), "Must pass in files, not root directory");
Preconditions.checkArgument(!parent.isAbsolute(), path);
try {
SkyKey depKey =
@@ -616,8 +626,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
}
}
- private static Iterable<SkyKey> toKeys(Iterable<Artifact> inputs,
- Iterable<Artifact> mandatoryInputs) {
+ private static Iterable<SkyKey> toKeys(
+ Iterable<Artifact> inputs, Iterable<Artifact> mandatoryInputs) {
if (mandatoryInputs == null) {
// This is a non inputs-discovering action, so no need to distinguish mandatory from regular
// inputs.
@@ -661,39 +671,44 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
new HashMap<>(populateInputData ? 128 : 0);
ActionExecutionException firstActionExecutionException = null;
- for (Map.Entry<SkyKey, ValueOrException2<MissingInputFileException,
- ActionExecutionException>> depsEntry : inputDeps.entrySet()) {
+ for (Map.Entry<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>>
+ depsEntry : inputDeps.entrySet()) {
Artifact input = ArtifactSkyKey.artifact(depsEntry.getKey());
try {
SkyValue value = depsEntry.getValue().get();
if (populateInputData) {
if (value instanceof AggregatingArtifactValue) {
AggregatingArtifactValue aggregatingValue = (AggregatingArtifactValue) value;
- for (Pair<Artifact, FileArtifactValue> entry : aggregatingValue.getInputs()) {
+ for (Pair<Artifact, FileArtifactValue> entry : aggregatingValue.getFileArtifacts()) {
inputArtifactData.put(entry.first, entry.second);
}
+ for (Pair<Artifact, TreeArtifactValue> entry : aggregatingValue.getTreeArtifacts()) {
+ expandTreeArtifactAndPopulateArtifactData(
+ entry.getFirst(),
+ Preconditions.checkNotNull(entry.getSecond()),
+ expandedArtifacts,
+ inputArtifactData);
+ }
// We have to cache the "digest" of the aggregating value itself,
// because the action cache checker may want it.
inputArtifactData.put(input, aggregatingValue.getSelfData());
- // Runfiles artifacts are not expanded into the action's inputs but their metadata is
- // available from the action file cache.
+ // While not obvious at all this code exists to ensure that we don't expand the
+ // .runfiles/MANIFEST file into the inputs. The reason for that being that the MANIFEST
+ // file contains absolute paths that don't work with remote execution.
+ // Instead, the way the SpawnInputExpander expands runfiles is via the Runfiles class
+ // which contains all artifacts in the runfiles tree minus the MANIFEST file.
+ // TODO(buchgr): Clean this up and get rid of the RunfilesArtifactValue type.
if (!(value instanceof RunfilesArtifactValue)) {
ImmutableList.Builder<Artifact> expansionBuilder = ImmutableList.builder();
- for (Pair<Artifact, FileArtifactValue> pair : aggregatingValue.getInputs()) {
- expansionBuilder.add(pair.first);
+ for (Pair<Artifact, FileArtifactValue> pair : aggregatingValue.getFileArtifacts()) {
+ expansionBuilder.add(Preconditions.checkNotNull(pair.getFirst()));
}
expandedArtifacts.put(input, expansionBuilder.build());
}
} else if (value instanceof TreeArtifactValue) {
- TreeArtifactValue treeValue = (TreeArtifactValue) value;
- expandedArtifacts.put(input, ImmutableSet.<Artifact>copyOf(treeValue.getChildren()));
- for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
- treeValue.getChildValues().entrySet()) {
- inputArtifactData.put(child.getKey(), child.getValue());
- }
+ expandTreeArtifactAndPopulateArtifactData(
+ input, (TreeArtifactValue) value, expandedArtifacts, inputArtifactData);
- // Again, we cache the "digest" of the value for cache checking.
- inputArtifactData.put(input, treeValue.getSelfData());
} else {
Preconditions.checkState(value instanceof FileArtifactValue, depsEntry);
inputArtifactData.put(input, (FileArtifactValue) value);
@@ -740,20 +755,37 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
"%s: missing input file '%s'",
action.getOwner().getLabel(), missingInput.getLabel())));
}
- throw new ActionExecutionException(missingCount + " input file(s) do not exist", action,
- rootCauses.build(), /*catastrophe=*/false);
+ throw new ActionExecutionException(
+ missingCount + " input file(s) do not exist",
+ action,
+ rootCauses.build(),
+ /*catastrophe=*/ false);
}
return Pair.of(inputArtifactData, expandedArtifacts);
}
+ private static void expandTreeArtifactAndPopulateArtifactData(
+ Artifact treeArtifact,
+ TreeArtifactValue value,
+ Map<Artifact, Collection<Artifact>> expandedArtifacts,
+ ActionInputMap inputArtifactData) {
+ ImmutableSet.Builder<Artifact> children = ImmutableSet.builder();
+ for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
+ value.getChildValues().entrySet()) {
+ children.add(child.getKey());
+ inputArtifactData.put(child.getKey(), child.getValue());
+ }
+ expandedArtifacts.put(treeArtifact, children.build());
+ // Again, we cache the "digest" of the value for cache checking.
+ inputArtifactData.put(treeArtifact, value.getSelfData());
+ }
+
private static Iterable<Artifact> filterKnownInputs(
Iterable<Artifact> newInputs, ActionInputMap inputArtifactData) {
return Iterables.filter(newInputs, input -> inputArtifactData.getMetadata(input) == null);
}
- /**
- * All info/warning messages associated with actions should be always displayed.
- */
+ /** All info/warning messages associated with actions should be always displayed. */
@Override
public String extractTag(SkyKey skyKey) {
return null;
@@ -783,15 +815,16 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
* State to save work across restarts of ActionExecutionFunction due to missing values in the
* graph for actions that discover inputs. There are three places where we save work, all for
* actions that discover inputs:
+ *
* <ol>
* <li>If not all known input metadata (coming from Action#getInputs) is available yet, then the
- * calculated set of inputs (including the inputs resolved from the action cache) is saved.</li>
+ * calculated set of inputs (including the inputs resolved from the action cache) is saved.
* <li>If not all discovered inputs' metadata is available yet, then the known input metadata
- * together with the set of discovered inputs is saved, as well as the Token used to identify
- * this action to the action cache.</li>
+ * together with the set of discovered inputs is saved, as well as the Token used to
+ * identify this action to the action cache.
* <li>If, after execution, new inputs are discovered whose metadata is not yet available, then
- * the same data as in the previous case is saved, along with the actual result of execution.
- * </li>
+ * the same data as in the previous case is saved, along with the actual result of
+ * execution.
* </ol>
*/
private static class ContinuationState {
@@ -838,14 +871,21 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver
@Override
public String toString() {
- return token + ", " + value + ", " + allInputs + ", " + inputArtifactData + ", "
+ return token
+ + ", "
+ + value
+ + ", "
+ + allInputs
+ + ", "
+ + inputArtifactData
+ + ", "
+ discoveredInputs;
}
}
/**
- * Used to declare all the exception types that can be wrapped in the exception thrown by
- * {@link ActionExecutionFunction#compute}.
+ * Used to declare all the exception types that can be wrapped in the exception thrown by {@link
+ * ActionExecutionFunction#compute}.
*/
private static final class ActionExecutionFunctionException extends SkyFunctionException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java
index e6e045bce4..c8a56a2772 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AggregatingArtifactValue.java
@@ -23,17 +23,29 @@ import java.util.Collection;
/** Value for aggregating artifacts, which must be expanded to a set of other artifacts. */
class AggregatingArtifactValue implements SkyValue {
private final FileArtifactValue selfData;
- private final ImmutableList<Pair<Artifact, FileArtifactValue>> inputs;
+ private final ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs;
+ private final ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs;
- AggregatingArtifactValue(ImmutableList<Pair<Artifact, FileArtifactValue>> inputs,
+ AggregatingArtifactValue(
+ ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs,
+ ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs,
FileArtifactValue selfData) {
- this.inputs = inputs;
+ this.fileInputs = fileInputs;
+ this.directoryInputs = directoryInputs;
this.selfData = selfData;
}
- /** Returns the artifacts that this artifact expands to, together with their data. */
- Collection<Pair<Artifact, FileArtifactValue>> getInputs() {
- return inputs;
+ /** Returns the none tree artifacts that this artifact expands to, together with their data. */
+ Collection<Pair<Artifact, FileArtifactValue>> getFileArtifacts() {
+ return fileInputs;
+ }
+
+ /**
+ * Returns the tree artifacts that this artifact expands to, together with the information
+ * to which artifacts the tree artifacts expand to.
+ */
+ Collection<Pair<Artifact, TreeArtifactValue>> getTreeArtifacts() {
+ return directoryInputs;
}
/** Returns the data of the artifact for this value, as computed by the action cache checker. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index 467391af18..484eda71c3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -122,8 +122,8 @@ class ArtifactFunction implements SkyFunction {
actionLookupValue,
actionIndex);
if (isAggregatingValue(action)) {
- return createAggregatingValue(artifact, action,
- actionValue.getArtifactValue(artifact), env);
+ return createAggregatingValue(
+ artifact, action, actionValue.getArtifactValue(artifact), env);
}
}
return createSimpleFileArtifactValue(artifact, actionValue);
@@ -243,8 +243,8 @@ class ArtifactFunction implements SkyFunction {
private static MissingInputFileException makeMissingInputFileException(
Artifact artifact, boolean mandatory, Exception failure, EventHandler reporter) {
String extraMsg = (failure == null) ? "" : (":" + failure.getMessage());
- MissingInputFileException ex = new MissingInputFileException(
- constructErrorMessage(artifact) + extraMsg, null);
+ MissingInputFileException ex =
+ new MissingInputFileException(constructErrorMessage(artifact) + extraMsg, null);
if (mandatory) {
reporter.handle(Event.error(ex.getLocation(), ex.getMessage()));
}
@@ -262,10 +262,10 @@ class ArtifactFunction implements SkyFunction {
// Middleman artifacts have no corresponding files, so their ArtifactValues should have already
// been constructed during execution of the action.
Preconditions.checkState(!artifact.isMiddlemanArtifact(), artifact);
- FileValue data = Preconditions.checkNotNull(actionValue.getData(artifact),
- "%s %s", artifact, actionValue);
- Preconditions.checkNotNull(data.getDigest(),
- "Digest should already have been calculated for %s (%s)", artifact, data);
+ FileValue data =
+ Preconditions.checkNotNull(actionValue.getData(artifact), "%s %s", artifact, actionValue);
+ Preconditions.checkNotNull(
+ data.getDigest(), "Digest should already have been calculated for %s (%s)", artifact, data);
// Directories are special-cased because their mtimes are used, so should have been constructed
// during execution of the action (in ActionMetadataHandler#maybeStoreAdditionalData).
Preconditions.checkState(data.isFile(), "Unexpected not file %s (%s)", artifact, data);
@@ -279,8 +279,10 @@ class ArtifactFunction implements SkyFunction {
FileArtifactValue value,
SkyFunction.Environment env)
throws InterruptedException {
- // This artifact aggregates other artifacts. Keep track of them so callers can find them.
- ImmutableList.Builder<Pair<Artifact, FileArtifactValue>> inputs = ImmutableList.builder();
+ ImmutableList.Builder<Pair<Artifact, FileArtifactValue>> fileInputsBuilder =
+ ImmutableList.builder();
+ ImmutableList.Builder<Pair<Artifact, TreeArtifactValue>> directoryInputsBuilder =
+ ImmutableList.builder();
for (Map.Entry<SkyKey, SkyValue> entry : env.getValues(action.getInputs()).entrySet()) {
Artifact input = ArtifactSkyKey.artifact(entry.getKey());
SkyValue inputValue = entry.getValue();
@@ -288,21 +290,32 @@ class ArtifactFunction implements SkyFunction {
return null;
}
if (inputValue instanceof FileArtifactValue) {
- inputs.add(Pair.of(input, (FileArtifactValue) inputValue));
+ fileInputsBuilder.add(Pair.of(input, (FileArtifactValue) inputValue));
} else if (inputValue instanceof TreeArtifactValue) {
- inputs.add(Pair.of(input, ((TreeArtifactValue) inputValue).getSelfData()));
+ directoryInputsBuilder.add(Pair.of(input, (TreeArtifactValue) inputValue));
} else {
// We do not recurse in aggregating middleman artifacts.
- Preconditions.checkState(!(inputValue instanceof AggregatingArtifactValue),
- "%s %s %s", artifact, action, inputValue);
+ Preconditions.checkState(
+ !(inputValue instanceof AggregatingArtifactValue),
+ "%s %s %s",
+ artifact,
+ action,
+ inputValue);
}
}
+
+ ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs =
+ ImmutableList.sortedCopyOf(
+ Comparator.comparing(pair -> pair.getFirst().getExecPathString()),
+ fileInputsBuilder.build());
+ ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs =
+ ImmutableList.sortedCopyOf(
+ Comparator.comparing(pair -> pair.getFirst().getExecPathString()),
+ directoryInputsBuilder.build());
+
return (action.getActionType() == MiddlemanType.AGGREGATING_MIDDLEMAN)
- ? new AggregatingArtifactValue(
- ImmutableList.sortedCopyOf(
- Comparator.comparing(pair -> pair.first.getExecPathString()), inputs.build()),
- value)
- : new RunfilesArtifactValue(inputs.build(), value);
+ ? new AggregatingArtifactValue(fileInputs, directoryInputs, value)
+ : new RunfilesArtifactValue(fileInputs, directoryInputs, value);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
index 644bd5e331..ebb4acf87c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RunfilesArtifactValue.java
@@ -21,7 +21,9 @@ import com.google.devtools.build.lib.util.Pair;
/** The artifacts behind a runfiles middleman. */
class RunfilesArtifactValue extends AggregatingArtifactValue {
RunfilesArtifactValue(
- ImmutableList<Pair<Artifact, FileArtifactValue>> inputs, FileArtifactValue selfData) {
- super(inputs, selfData);
+ ImmutableList<Pair<Artifact, FileArtifactValue>> fileInputs,
+ ImmutableList<Pair<Artifact, TreeArtifactValue>> directoryInputs,
+ FileArtifactValue selfData) {
+ super(fileInputs, directoryInputs, selfData);
}
}
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 ee7f348e2c..49b864e563 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
@@ -505,16 +505,6 @@ public final class SkyframeActionExecutor {
Preconditions.checkState(artifact.isMiddlemanArtifact() || artifact.isTreeArtifact(),
artifact);
Collection<Artifact> result = expandedInputs.get(artifact);
-
- // Note that the result can be empty but not null for TreeArtifacts. And it may be null for
- // non-aggregating middlemen.
- if (artifact.isTreeArtifact()) {
- Preconditions.checkNotNull(
- result,
- "TreeArtifact %s cannot be expanded because it is not an input for the action",
- artifact);
- }
-
if (result != null) {
output.addAll(result);
}