Create a clearer distinction between normal tree artifact children and action template expansion outputs.
Two different factory methods are used: createTreeOutput for normal tree artifact children and createTemplateExpansionOutput for template expansion outputs. Additional documentation and validation is added for action templates to ensure that implementations follow the contract.
After this, I plan to add a method on TreeFileArtifact to distinguish between the two so that various places can properly treat these two different tree file artifact types differently instead of always having to handle both possibilities.
A lot of tests needed to be cleaned up, but the only test that needed a major overhaul was TreeArtifactBuildTest since it was generally declaring the outputs before creating the action.
RELNOTES: None.
PiperOrigin-RevId: 312128057
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java
index 3e7db23..f267b50 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputHelper.java
@@ -18,31 +18,21 @@
import com.google.common.base.Function;
import com.google.common.base.Functions;
import com.google.common.base.Preconditions;
-import com.google.common.collect.Collections2;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import java.util.Set;
-/**
- * Helper utility to create ActionInput instances.
- */
+/** Helper utility to create ActionInput instances. */
public final class ActionInputHelper {
- private ActionInputHelper() {
- }
+ private ActionInputHelper() {}
@VisibleForTesting
- public static ArtifactExpander actionGraphArtifactExpander(
- final ActionGraph actionGraph) {
+ public static ArtifactExpander actionGraphArtifactExpander(ActionGraph actionGraph) {
return new ArtifactExpander() {
@Override
public void expand(Artifact mm, Collection<? super Artifact> output) {
@@ -138,64 +128,6 @@
}
/**
- * Creates a sequence of {@link ActionInput}s from a sequence of string paths.
- */
- public static Collection<ActionInput> fromPaths(Collection<String> paths) {
- return Collections2.transform(paths, ActionInputHelper::fromPath);
- }
-
- /**
- * Instantiates a concrete TreeFileArtifact with the given parent Artifact and path relative to
- * that Artifact.
- */
- public static TreeFileArtifact treeFileArtifact(
- Artifact.SpecialArtifact parent, PathFragment relativePath) {
- TreeFileArtifact result =
- treeFileArtifactWithNoGeneratingActionSet(parent, relativePath, parent.getArtifactOwner());
- result.setGeneratingActionKey(parent.getGeneratingActionKey());
- return result;
- }
-
- /**
- * Instantiates a concrete TreeFileArtifact with the given parent Artifact and path relative to
- * that Artifact.
- */
- public static TreeFileArtifact treeFileArtifact(
- Artifact.SpecialArtifact parent, String relativePath) {
- return treeFileArtifact(parent, PathFragment.create(relativePath));
- }
-
- public static TreeFileArtifact treeFileArtifactWithNoGeneratingActionSet(
- SpecialArtifact parent, PathFragment relativePath, ActionLookupKey artifactOwner) {
- Preconditions.checkState(
- parent.isTreeArtifact(), "Given parent %s must be a TreeArtifact", parent);
- return new TreeFileArtifact(parent, relativePath, artifactOwner);
- }
-
- /** Returns an Iterable of TreeFileArtifacts with the given parent and parent relative paths. */
- public static Iterable<TreeFileArtifact> asTreeFileArtifacts(
- final Artifact.SpecialArtifact parent, Iterable<? extends PathFragment> parentRelativePaths) {
- Preconditions.checkState(parent.isTreeArtifact(),
- "Given parent %s must be a TreeArtifact", parent);
- return Iterables.transform(
- parentRelativePaths, pathFragment -> treeFileArtifact(parent, pathFragment));
- }
-
- /** Returns a Set of TreeFileArtifacts with the given parent and parent-relative paths. */
- public static Set<TreeFileArtifact> asTreeFileArtifacts(
- final Artifact.SpecialArtifact parent, Set<? extends PathFragment> parentRelativePaths) {
- Preconditions.checkState(parent.isTreeArtifact(),
- "Given parent %s must be a TreeArtifact", parent);
-
- ImmutableSet.Builder<TreeFileArtifact> builder = ImmutableSet.builder();
- for (PathFragment path : parentRelativePaths) {
- builder.add(treeFileArtifact(parent, path));
- }
-
- return builder.build();
- }
-
- /**
* Expands middleman artifacts in a sequence of {@link ActionInput}s.
*
* <p>Non-middleman artifacts are returned untouched.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
index fc33a23..25bf89f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupData.java
@@ -18,6 +18,7 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -26,9 +27,6 @@
/** Data that uniquely identifies an action. */
@AutoCodec
public class ActionLookupData implements SkyKey {
- // Test actions are not shareable.
- // Action execution can be nondeterministic, so is semi-hermetic.
- public static final SkyFunctionName NAME = SkyFunctionName.createSemiHermetic("ACTION_EXECUTION");
private final ActionLookupKey actionLookupKey;
private final int actionIndex;
@@ -98,6 +96,6 @@
@Override
public SkyFunctionName functionName() {
- return NAME;
+ return SkyFunctions.ACTION_EXECUTION;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java b/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java
index f783941..8301961 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionTemplate.java
@@ -13,8 +13,11 @@
// 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.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import javax.annotation.Nullable;
@@ -26,18 +29,16 @@
* <p>ActionTemplate is for users who want to dynamically register Actions operating on individual
* {@link TreeFileArtifact} inside input and output TreeArtifacts at execution time.
*
- * <p>It takes in one TreeArtifact and generates one TreeArtifact. The following happens at
- * execution time for ActionTemplate:
+ * <p>It takes in one TreeArtifact and generates one or more output TreeArtifacts. The following
+ * happens at execution time for ActionTemplate:
*
* <ol>
* <li>Input TreeArtifact is resolved.
- * <li>For each individual {@link TreeFileArtifact} inside input TreeArtifact, generate an output
- * {@link TreeFileArtifact} inside output TreeArtifact.
- * <li>For each pair of input and output {@link TreeFileArtifact}s, generate an associated {@link
- * Action}.
+ * <li>Given the set of {@link TreeFileArtifact}s inside input TreeArtifact, generate actions with
+ * outputs inside output TreeArtifact(s).
* <li>All expanded {@link Action}s are executed and their output {@link TreeFileArtifact}s
* collected.
- * <li>Output TreeArtifact is resolved.
+ * <li>Output TreeArtifact(s) are resolved.
* </ol>
*
* <p>Implementations of ActionTemplate must follow the contract of this interface and also make
@@ -50,8 +51,8 @@
* properly represent the expanded actions at analysis time, and the action graph at analysis
* time is correct. This is important because the action graph is walked in a lot of places
* for correctness checks and build analysis.
- * <li>The outputs of expanded actions must be under the output TreeArtifact and must not have
- * artifact or artifact path prefix conflicts.
+ * <li>The outputs of expanded actions must be under one of the output TreeArtifact(s) and must
+ * not have artifact or artifact path prefix conflicts.
* </ol>
*/
public interface ActionTemplate<T extends Action> extends ActionAnalysisMetadata {
@@ -69,29 +70,51 @@
}
/**
- * Given a list of input TreeFileArtifacts resolved at execution time, returns a list of expanded
- * SpawnActions to be executed.
+ * Given a set of input TreeFileArtifacts resolved at execution time, returns a list of expanded
+ * actions to be executed.
*
- * @param inputTreeFileArtifacts the list of {@link TreeFileArtifact}s inside input TreeArtifact
- * resolved at execution time
+ * <p>Each of the expanded actions' outputs must be a {@link TreeFileArtifact} owned by {@code
+ * artifactOwner} with a parent in {@link #getOutputs}. This is generally satisfied by calling
+ * {@link TreeFileArtifact#createTemplateExpansionOutput}.
+ *
+ * @param inputTreeFileArtifacts the set of {@link TreeFileArtifact}s inside {@link
+ * #getInputTreeArtifact}
* @param artifactOwner the {@link ArtifactOwner} of the generated output {@link
* TreeFileArtifact}s
- * @return a list of expanded {@link Action}s to execute, one for each input {@link
- * TreeFileArtifact}
+ * @return a list of expanded {@link Action}s to execute
*/
- Iterable<T> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner)
+ ImmutableList<T> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner)
throws ActionTemplateExpansionException;
/** Returns the input TreeArtifact. */
- Artifact getInputTreeArtifact();
+ SpecialArtifact getInputTreeArtifact();
/** Returns the output TreeArtifact. */
- Artifact getOutputTreeArtifact();
+ SpecialArtifact getOutputTreeArtifact();
+
+ @Override
+ default SpecialArtifact getPrimaryInput() {
+ return getInputTreeArtifact();
+ }
+
+ /**
+ * By default, returns just {@link #getOutputTreeArtifact}, but may be overridden with additional
+ * tree artifacts.
+ */
+ @Override
+ default ImmutableSet<Artifact> getOutputs() {
+ return ImmutableSet.of(getOutputTreeArtifact());
+ }
+
+ @Override
+ default SpecialArtifact getPrimaryOutput() {
+ return getOutputTreeArtifact();
+ }
@Override
default ImmutableMap<String, String> getExecProperties() {
- return ImmutableMap.<String, String>of();
+ return ImmutableMap.of();
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index ab9e6e9..4e99b2f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext;
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
import com.google.devtools.build.lib.skyframe.serialization.SerializationContext;
@@ -784,78 +785,128 @@
/**
* A special kind of artifact that represents a concrete file created at execution time under its
- * associated TreeArtifact.
+ * associated parent TreeArtifact.
*
* <p>TreeFileArtifacts should be only created during execution time inside some special actions
* to support action inputs and outputs that are unpredictable at analysis time. TreeFileArtifacts
* should not be created directly by any rules at analysis time.
*
- * <p>We subclass {@link DerivedArtifact} instead of storing the extra fields directly inside in
- * order to save memory. The proportion of TreeFileArtifacts is very small, and by not having to
- * keep around the extra fields for the rest we save some memory.
+ * <p>There are two types of TreeFileArtifacts:
+ *
+ * <ol>
+ * <li>Outputs under a directory created by an action using {@code declare_directory}. In this
+ * case, a single action creates both the parent and all of the children. Instances should
+ * be created by calling {@link #createTreeOutput}.
+ * <li>Outputs of an action template expansion. In this case, the parent directory is not
+ * actually produced by any action, but rather serves as a placeholder for dependant actions
+ * to declare a dep on during analysis, before the children are known. The children are
+ * created by various actions (from the template expansion). Instances should be created by
+ * calling {@link #createTemplateExpansionOutput}.
+ * </ol>
*/
@Immutable
@AutoCodec
public static final class TreeFileArtifact extends DerivedArtifact {
- private final SpecialArtifact parentTreeArtifact;
+ private final SpecialArtifact parent;
private final PathFragment parentRelativePath;
/**
- * Constructs a TreeFileArtifact with the given parent-relative path under the given parent
- * TreeArtifact. The {@link ArtifactOwner} of the TreeFileArtifact is the {@link ArtifactOwner}
- * of the parent TreeArtifact.
+ * Creates a {@link TreeFileArtifact} representing a child of the given parent tree artifact.
+ *
+ * <p>The child should already have been created by the parent's generating action. For this
+ * reason, {@link DerivedArtifact#hasGeneratingActionKey} on the parent must be {@code true}
+ * when this is called. The child is set with the same generating action.
*/
- @VisibleForTesting
- public TreeFileArtifact(SpecialArtifact parent, PathFragment parentRelativePath) {
- this(parent, parentRelativePath, parent.getArtifactOwner());
+ public static TreeFileArtifact createTreeOutput(
+ SpecialArtifact parent, PathFragment parentRelativePath) {
+ Preconditions.checkArgument(
+ parent.hasGeneratingActionKey(),
+ "%s has no generating action key (parent owner: %s, parent relative path: %s)",
+ parent,
+ parent.getArtifactOwner(),
+ parentRelativePath);
+ ActionLookupData generatingActionKey = parent.getGeneratingActionKey();
+ Preconditions.checkArgument(
+ !isActionTemplateExpansionKey(generatingActionKey.getActionLookupKey()),
+ "%s owned by action template expansion %s (parent relative path: %s)",
+ parent,
+ generatingActionKey.getActionLookupKey(),
+ parentRelativePath);
+ return createInternal(parent, parentRelativePath, generatingActionKey);
}
/**
- * Constructs a TreeFileArtifact with the given parent-relative path under the given parent
- * TreeArtifact, owned by the given {@code artifactOwner}.
+ * Convenience method for {@link #createTreeOutput(SpecialArtifact, PathFragment)} with a string
+ * relative path.
*/
- TreeFileArtifact(
- SpecialArtifact parentTreeArtifact,
- PathFragment parentRelativePath,
- ActionLookupKey owner) {
+ public static TreeFileArtifact createTreeOutput(
+ SpecialArtifact parent, String parentRelativePath) {
+ return createTreeOutput(parent, PathFragment.create(parentRelativePath));
+ }
+
+ /**
+ * Creates a {@link TreeFileArtifact} representing the output of an action generated dynamically
+ * by an {@link ActionTemplate} during the execution phase.
+ *
+ * <p>The returned artifact does not yet have a generating action set.
+ */
+ public static TreeFileArtifact createTemplateExpansionOutput(
+ SpecialArtifact parent, PathFragment parentRelativePath, ActionLookupKey owner) {
+ Preconditions.checkArgument(
+ isActionTemplateExpansionKey(owner),
+ "Template expansion outputs must be owned by an action template expansion key, but %s is"
+ + " owned by %s (parent relative path: %s)",
+ parent,
+ owner,
+ parentRelativePath);
+ return new TreeFileArtifact(parent, parentRelativePath, owner);
+ }
+
+ /**
+ * Convenience method for {@link #createTemplateExpansionOutput(SpecialArtifact, PathFragment,
+ * ActionLookupKey)} with a string relative path.
+ */
+ public static TreeFileArtifact createTemplateExpansionOutput(
+ SpecialArtifact parent, String parentRelativePath, ActionLookupKey owner) {
+ return createTemplateExpansionOutput(parent, PathFragment.create(parentRelativePath), owner);
+ }
+
+ private TreeFileArtifact(
+ SpecialArtifact parent, PathFragment parentRelativePath, ActionLookupKey owner) {
super(
- parentTreeArtifact.getRoot(),
- parentTreeArtifact.getExecPath().getRelative(parentRelativePath),
+ parent.getRoot(),
+ parent.getExecPath().getRelative(parentRelativePath),
owner,
/*contentBasedPath=*/ false);
Preconditions.checkArgument(
- parentTreeArtifact.isTreeArtifact(),
+ parent.isTreeArtifact(),
"The parent of TreeFileArtifact (parent-relative path: %s) is not a TreeArtifact: %s",
parentRelativePath,
- parentTreeArtifact);
+ parent);
Preconditions.checkArgument(
!parentRelativePath.containsUplevelReferences() && !parentRelativePath.isAbsolute(),
"%s is not a proper normalized relative path",
parentRelativePath);
- Preconditions.checkState(
- parentTreeArtifact.isTreeArtifact(),
- "Given parent %s must be a TreeArtifact",
- parentTreeArtifact);
- this.parentTreeArtifact = parentTreeArtifact;
+ this.parent = parent;
this.parentRelativePath = parentRelativePath;
}
@AutoCodec.VisibleForSerialization
@AutoCodec.Instantiator
- static TreeFileArtifact createForSerialization(
- SpecialArtifact parentTreeArtifact,
+ static TreeFileArtifact createInternal(
+ SpecialArtifact parent,
PathFragment parentRelativePath,
ActionLookupData generatingActionKey) {
TreeFileArtifact result =
new TreeFileArtifact(
- parentTreeArtifact, parentRelativePath, generatingActionKey.getActionLookupKey());
+ parent, parentRelativePath, generatingActionKey.getActionLookupKey());
result.setGeneratingActionKey(generatingActionKey);
return result;
}
@Override
public SpecialArtifact getParent() {
- return parentTreeArtifact;
+ return parent;
}
@Override
@@ -867,6 +918,10 @@
public String getTreeRelativePathString() {
return parentRelativePath.getPathString();
}
+
+ private static boolean isActionTemplateExpansionKey(ActionLookupKey key) {
+ return SkyFunctions.ACTION_TEMPLATE_EXPANSION.equals(key.functionName());
+ }
}
/**
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 47bc74e..abb27df 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -50,12 +50,12 @@
"//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils",
"//src/main/java/com/google/devtools/build/lib/shell",
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skylarkbuildapi",
"//src/main/java/com/google/devtools/build/lib/unix",
"//src/main/java/com/google/devtools/build/lib/unsafe:string",
"//src/main/java/com/google/devtools/build/lib/util",
- "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:command",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:exit_code",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 2b14144..bd82fad 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -1306,6 +1306,7 @@
":analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:action_template_expansion_value",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
index 40ad5eb..2b6a3e4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplate.java
@@ -18,7 +18,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyCacher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
@@ -33,6 +32,7 @@
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.skyframe.ActionTemplateExpansionValue;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Map;
@@ -98,15 +98,16 @@
}
@Override
- public Iterable<SpawnAction> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner) {
- ImmutableList.Builder<SpawnAction> expandedActions = new ImmutableList.Builder<>();
+ public ImmutableList<SpawnAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner) {
+ ImmutableList.Builder<SpawnAction> expandedActions =
+ ImmutableList.builderWithExpectedSize(inputTreeFileArtifacts.size());
for (TreeFileArtifact inputTreeFileArtifact : inputTreeFileArtifacts) {
PathFragment parentRelativeOutputPath =
outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact);
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ TreeFileArtifact.createTemplateExpansionOutput(
outputTreeArtifact, parentRelativeOutputPath, artifactOwner);
expandedActions.add(createAction(inputTreeFileArtifact, outputTreeFileArtifact));
@@ -119,12 +120,13 @@
protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp)
throws CommandLineExpansionException {
TreeFileArtifact inputTreeFileArtifact =
- ActionInputHelper.treeFileArtifact(inputTreeArtifact, "dummy_for_key");
+ TreeFileArtifact.createTreeOutput(inputTreeArtifact, "dummy_for_key");
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ TreeFileArtifact.createTemplateExpansionOutput(
outputTreeArtifact,
outputPathMapper.parentRelativeOutputPath(inputTreeFileArtifact),
- outputTreeArtifact.getArtifactOwner());
+ ActionTemplateExpansionValue.key(
+ outputTreeArtifact.getArtifactOwner(), /*actionIndex=*/ 0));
SpawnAction dummyAction = createAction(inputTreeFileArtifact, outputTreeFileArtifact);
dummyAction.computeKey(actionKeyContext, fp);
}
@@ -154,16 +156,15 @@
*
* <p>This method is called by Skyframe to expand the input TreeArtifact into child
* TreeFileArtifacts. Skyframe then expands this SpawnActionTemplate with the TreeFileArtifacts
- * through {@link #generateActionForInputArtifacts}.
+ * through {@link #generateActionsForInputArtifacts}.
*/
@Override
- public Artifact getInputTreeArtifact() {
+ public SpecialArtifact getInputTreeArtifact() {
return inputTreeArtifact;
}
- /** Returns the output TreeArtifact. */
@Override
- public Artifact getOutputTreeArtifact() {
+ public SpecialArtifact getOutputTreeArtifact() {
return outputTreeArtifact;
}
@@ -193,11 +194,6 @@
}
@Override
- public ImmutableSet<Artifact> getOutputs() {
- return ImmutableSet.of(outputTreeArtifact);
- }
-
- @Override
public NestedSet<Artifact> getMandatoryInputs() {
return getInputs();
}
@@ -214,16 +210,6 @@
}
@Override
- public Artifact getPrimaryInput() {
- return inputTreeArtifact;
- }
-
- @Override
- public Artifact getPrimaryOutput() {
- return outputTreeArtifact;
- }
-
- @Override
public Iterable<String> getClientEnvironmentVariables() {
return spawnActionBuilder.buildForActionTemplate(getOwner()).getClientEnvironmentVariables();
}
@@ -240,8 +226,12 @@
@Override
public String prettyPrint() {
- return String.format("action template with output TreeArtifact %s",
- outputTreeArtifact.prettyPrint());
+ return "SpawnActionTemplate with output TreeArtifact " + outputTreeArtifact.prettyPrint();
+ }
+
+ @Override
+ public String toString() {
+ return prettyPrint();
}
/** Builder class to construct {@link SpawnActionTemplate} instances. */
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
index 1cb1ef0..a7a0bdb 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -39,7 +39,6 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.SettableFuture;
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.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -619,7 +618,7 @@
ImmutableMap.builderWithExpectedSize(directory.files.size());
for (FileMetadata file : directory.files()) {
TreeFileArtifact child =
- ActionInputHelper.treeFileArtifact(parent, file.path().relativeTo(parent.getPath()));
+ TreeFileArtifact.createTreeOutput(parent, file.path().relativeTo(parent.getPath()));
RemoteFileArtifactValue value =
new RemoteFileArtifactValue(
DigestUtil.toBinaryDigest(file.digest()),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
index 5c51208..1c9dc9d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -18,13 +18,13 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyCacher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.ActionTemplate;
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.TreeFileArtifact;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -34,7 +34,6 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
-import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
@@ -43,9 +42,9 @@
public final class CppCompileActionTemplate extends ActionKeyCacher
implements ActionTemplate<CppCompileAction> {
private final CppCompileActionBuilder cppCompileActionBuilder;
- private final Artifact sourceTreeArtifact;
- private final Artifact.SpecialArtifact outputTreeArtifact;
- private final Artifact.SpecialArtifact dotdTreeArtifact;
+ private final SpecialArtifact sourceTreeArtifact;
+ private final SpecialArtifact outputTreeArtifact;
+ private final SpecialArtifact dotdTreeArtifact;
private final CcToolchainProvider toolchain;
private final Iterable<ArtifactCategory> categories;
private final ActionOwner actionOwner;
@@ -67,9 +66,9 @@
* @param actionOwner the owner of this {@link ActionTemplate}.
*/
CppCompileActionTemplate(
- Artifact sourceTreeArtifact,
- Artifact.SpecialArtifact outputTreeArtifact,
- Artifact.SpecialArtifact dotdTreeArtifact,
+ SpecialArtifact sourceTreeArtifact,
+ SpecialArtifact outputTreeArtifact,
+ SpecialArtifact dotdTreeArtifact,
CppCompileActionBuilder cppCompileActionBuilder,
CcToolchainProvider toolchain,
Iterable<ArtifactCategory> categories,
@@ -89,8 +88,8 @@
}
@Override
- public Iterable<CppCompileAction> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner)
+ public ImmutableList<CppCompileAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner)
throws ActionTemplateExpansionException {
ImmutableList.Builder<CppCompileAction> expandedActions = new ImmutableList.Builder<>();
@@ -111,7 +110,7 @@
}
if (isSource || (isHeader && shouldCompileHeaders() && !isTextualInclude)) {
sourcesBuilder.add(inputTreeFileArtifact);
- } else if (!isSource && !isHeader) {
+ } else if (!isHeader) {
throw new ActionTemplateExpansionException(
String.format(
"Artifact '%s' expanded from the directory artifact '%s' is neither header "
@@ -126,13 +125,13 @@
try {
String outputName = outputTreeFileArtifactName(inputTreeFileArtifact);
TreeFileArtifact outputTreeFileArtifact =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- outputTreeArtifact, PathFragment.create(outputName), artifactOwner);
+ TreeFileArtifact.createTemplateExpansionOutput(
+ outputTreeArtifact, outputName, artifactOwner);
TreeFileArtifact dotdFileArtifact = null;
if (dotdTreeArtifact != null) {
dotdFileArtifact =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- dotdTreeArtifact, PathFragment.create(outputName + ".d"), artifactOwner);
+ TreeFileArtifact.createTemplateExpansionOutput(
+ dotdTreeArtifact, outputName + ".d", artifactOwner);
}
expandedActions.add(
createAction(
@@ -228,12 +227,12 @@
}
@Override
- public Artifact getInputTreeArtifact() {
+ public SpecialArtifact getInputTreeArtifact() {
return sourceTreeArtifact;
}
@Override
- public Artifact getOutputTreeArtifact() {
+ public SpecialArtifact getOutputTreeArtifact() {
return outputTreeArtifact;
}
@@ -268,7 +267,7 @@
@Override
public ImmutableSet<Artifact> getMandatoryOutputs() {
- return ImmutableSet.<Artifact>of();
+ return ImmutableSet.of();
}
@Override
@@ -294,17 +293,7 @@
@Override
public Iterable<String> getClientEnvironmentVariables() {
- return ImmutableList.<String>of();
- }
-
- @Override
- public Artifact getPrimaryInput() {
- return sourceTreeArtifact;
- }
-
- @Override
- public Artifact getPrimaryOutput() {
- return outputTreeArtifact;
+ return ImmutableList.of();
}
@Override
@@ -326,7 +315,11 @@
@Override
public String prettyPrint() {
- return String.format(
- "CppCompileActionTemplate compiling " + sourceTreeArtifact.getExecPathString());
+ return "CppCompileActionTemplate compiling " + sourceTreeArtifact.getExecPathString();
+ }
+
+ @Override
+ public String toString() {
+ return prettyPrint();
}
}
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 9a3aacb..1a2587e 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
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -282,7 +281,7 @@
artifact,
newArtifactMap);
transformedArtifact =
- ActionInputHelper.treeFileArtifact(
+ TreeFileArtifact.createTreeOutput(
(Artifact.SpecialArtifact) newParent, artifact.getParentRelativePath());
}
result.put(transformedArtifact, entry.getValue());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index 1efd143..8acefb5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -23,7 +23,6 @@
import com.google.common.flogger.GoogleLogger;
import com.google.common.io.BaseEncoding;
import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputMap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
@@ -374,7 +373,7 @@
setTreeReadOnlyAndExecutable(artifact, PathFragment.EMPTY_FRAGMENT);
} else {
setPathReadOnlyAndExecutable(
- ActionInputHelper.treeFileArtifact(artifact, PathFragment.EMPTY_FRAGMENT));
+ TreeFileArtifact.createTreeOutput(artifact, PathFragment.EMPTY_FRAGMENT));
}
}
@@ -383,12 +382,22 @@
return value;
}
- private TreeArtifactValue constructTreeArtifactValue(Collection<TreeFileArtifact> contents)
+ private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifact parent)
throws IOException {
- Map<TreeFileArtifact, FileArtifactValue> values =
- Maps.newHashMapWithExpectedSize(contents.size());
+ Preconditions.checkState(parent.isTreeArtifact(), parent);
- for (TreeFileArtifact treeFileArtifact : contents) {
+ // Make sure the tree artifact root is a regular directory. Note that this is how the Action
+ // is initialized, so this should hold unless the Action itself has deleted the root.
+ if (!artifactPathResolver.toPath(parent).isDirectory(Symlinks.NOFOLLOW)) {
+ return TreeArtifactValue.MISSING_TREE_ARTIFACT;
+ }
+
+ Set<PathFragment> paths =
+ TreeArtifactValue.explodeDirectory(artifactPathResolver.toPath(parent));
+
+ Map<TreeFileArtifact, FileArtifactValue> values = Maps.newHashMapWithExpectedSize(paths.size());
+ for (PathFragment path : paths) {
+ TreeFileArtifact treeFileArtifact = TreeFileArtifact.createTreeOutput(parent, path);
FileArtifactValue fileMetadata = store.getArtifactData(treeFileArtifact);
// This is similar to what's present in getRealMetadataForArtifact, except
// we get back the ArtifactFileMetadata, not the metadata.
@@ -420,21 +429,6 @@
return TreeArtifactValue.create(values);
}
- private TreeArtifactValue constructTreeArtifactValueFromFilesystem(SpecialArtifact artifact)
- throws IOException {
- Preconditions.checkState(artifact.isTreeArtifact(), artifact);
-
- // Make sure the tree artifact root is a regular directory. Note that this is how the Action
- // is initialized, so this should hold unless the Action itself has deleted the root.
- if (!artifactPathResolver.toPath(artifact).isDirectory(Symlinks.NOFOLLOW)) {
- return TreeArtifactValue.MISSING_TREE_ARTIFACT;
- }
-
- Set<PathFragment> paths =
- TreeArtifactValue.explodeDirectory(artifactPathResolver.toPath(artifact));
- return constructTreeArtifactValue(ActionInputHelper.asTreeFileArtifacts(artifact, paths));
- }
-
@Override
public ImmutableSet<TreeFileArtifact> getExpandedOutputs(Artifact artifact) {
TreeArtifactValue treeArtifact = store.getTreeArtifactData(artifact);
@@ -696,7 +690,7 @@
setTreeReadOnlyAndExecutable(parent, subpath.getChild(dirent.getName()));
} else {
setPathReadOnlyAndExecutable(
- ActionInputHelper.treeFileArtifact(parent, subpath.getChild(dirent.getName())));
+ TreeFileArtifact.createTreeOutput(parent, subpath.getChild(dirent.getName())));
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
index 018e7c1..e5499a6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java
@@ -13,8 +13,10 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -40,6 +42,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.TreeMap;
import javax.annotation.Nullable;
@@ -78,15 +81,15 @@
if (env.valuesMissing()) {
return null;
}
- Iterable<TreeFileArtifact> inputTreeFileArtifacts = treeArtifactValue.getChildren();
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts = treeArtifactValue.getChildren();
GeneratingActions generatingActions;
try {
// Expand the action template using the list of expanded input TreeFileArtifacts.
// TODO(rduan): Add a check to verify the inputs of expanded actions are subsets of inputs
// of the ActionTemplate.
- generatingActions =
- checkActionAndArtifactConflicts(
- actionTemplate.generateActionForInputArtifacts(inputTreeFileArtifacts, key), key);
+ ImmutableList<? extends Action> actions =
+ generateAndValidateActionsFromTemplate(actionTemplate, inputTreeFileArtifacts, key);
+ generatingActions = checkActionAndArtifactConflicts(actions, key);
} catch (ActionConflictException e) {
e.reportTo(env.getListener());
throw new ActionTemplateExpansionFunctionException(e);
@@ -116,12 +119,49 @@
}
}
+ private static ImmutableList<? extends Action> generateAndValidateActionsFromTemplate(
+ ActionTemplate<?> actionTemplate,
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts,
+ ActionTemplateExpansionKey key)
+ throws ActionTemplateExpansionException {
+ Set<Artifact> outputs = actionTemplate.getOutputs();
+ for (Artifact output : outputs) {
+ Preconditions.checkState(
+ output.isTreeArtifact(),
+ "%s declares an output which is not a tree artifact: %s",
+ actionTemplate,
+ output);
+ }
+ ImmutableList<? extends Action> actions =
+ actionTemplate.generateActionsForInputArtifacts(inputTreeFileArtifacts, key);
+ for (Action action : actions) {
+ for (Artifact output : action.getOutputs()) {
+ Preconditions.checkState(
+ output.getArtifactOwner().equals(key),
+ "%s generated an action with an output owned by the wrong owner: %s",
+ actionTemplate,
+ action);
+ Preconditions.checkState(
+ output.hasParent(),
+ "%s generated an action which outputs a non-TreeFileArtifact: %s",
+ actionTemplate,
+ action);
+ Preconditions.checkState(
+ outputs.contains(output.getParent()),
+ "%s generated an action with an output under an undeclared tree: %s",
+ actionTemplate,
+ action);
+ }
+ }
+ return actions;
+ }
+
private GeneratingActions checkActionAndArtifactConflicts(
- Iterable<? extends Action> actions, ActionLookupValue.ActionLookupKey actionLookupKey)
+ ImmutableList<? extends Action> actions, ActionTemplateExpansionKey key)
throws ActionConflictException, ArtifactPrefixConflictException {
GeneratingActions generatingActions =
Actions.assignOwnersAndFindAndThrowActionConflict(
- actionKeyContext, ImmutableList.copyOf(actions), actionLookupKey);
+ actionKeyContext, ImmutableList.copyOf(actions), key);
Map<ActionAnalysisMetadata, ArtifactPrefixConflictException> artifactPrefixConflictMap =
findArtifactPrefixConflicts(getMapForConsistencyCheck(generatingActions.getActions()));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
index 8ee7db4..ae4e53a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
@@ -30,7 +30,7 @@
super(generatingActions);
}
- static ActionTemplateExpansionKey key(ActionLookupKey actionLookupKey, int actionIndex) {
+ public static ActionTemplateExpansionKey key(ActionLookupKey actionLookupKey, int actionIndex) {
return ActionTemplateExpansionKey.of(actionLookupKey, actionIndex);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index e27d90c..5bcc96b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2222,7 +2222,6 @@
name = "sky_functions",
srcs = ["SkyFunctions.java"],
deps = [
- "//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
],
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index 726b636..39b0d87 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -15,7 +15,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicate;
-import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.skyframe.FunctionHermeticity;
import com.google.devtools.build.skyframe.ShareabilityOfValue;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -106,7 +105,9 @@
"TEST_COMPLETION", ShareabilityOfValue.NEVER, FunctionHermeticity.HERMETIC);
public static final SkyFunctionName BUILD_CONFIGURATION =
SkyFunctionName.createHermetic("BUILD_CONFIGURATION");
- public static final SkyFunctionName ACTION_EXECUTION = ActionLookupData.NAME;
+ // Test actions are not shareable. Action execution can be nondeterministic, so is semi-hermetic.
+ public static final SkyFunctionName ACTION_EXECUTION =
+ SkyFunctionName.createSemiHermetic("ACTION_EXECUTION");
public static final SkyFunctionName ARTIFACT_NESTED_SET =
SkyFunctionName.createHermetic("ARTIFACT_NESTED_SET");
public static final SkyFunctionName PATH_CASING_LOOKUP =
diff --git a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
index d09c3c4..7c527ac 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java
@@ -22,7 +22,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
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.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
@@ -901,11 +900,9 @@
.build();
TreeFileArtifact treeFileArtifactOne =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifactOne, "children/child1");
+ TreeFileArtifact.createTreeOutput(treeArtifactOne, "children/child1");
TreeFileArtifact treeFileArtifactTwo =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifactTwo, "children/child2");
+ TreeFileArtifact.createTreeOutput(treeArtifactTwo, "children/child2");
CustomCommandLine commandLine = commandLineTemplate.evaluateTreeFileArtifacts(
ImmutableList.of(treeFileArtifactOne, treeFileArtifactTwo));
@@ -924,11 +921,9 @@
SpecialArtifact treeArtifact = createTreeArtifact("myArtifact/treeArtifact");
TreeFileArtifact treeFileArtifactOne =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifact, "children/child1");
+ TreeFileArtifact.createTreeOutput(treeArtifact, "children/child1");
TreeFileArtifact treeFileArtifactTwo =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifact, "children/child2");
+ TreeFileArtifact.createTreeOutput(treeArtifact, "children/child2");
CommandLineItem.MapFn<Artifact> expandParentRelativePath =
(src, args) -> {
@@ -1004,12 +999,8 @@
}
private SpecialArtifact createTreeArtifact(String rootRelativePath) {
- PathFragment relpath = PathFragment.create(rootRelativePath);
- return new SpecialArtifact(
- rootDir,
- rootDir.getExecPath().getRelative(relpath),
- ActionsTestUtil.NULL_ACTION_LOOKUP_DATA.getActionLookupKey(),
- SpecialArtifactType.TREE);
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
+ rootDir, rootDir.getExecPath().getRelative(rootRelativePath));
}
private static <T> ImmutableList<T> list(T... objects) {
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 16d8aa6..7b75fee 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
@@ -45,6 +45,7 @@
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.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.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactResolver;
@@ -73,6 +74,8 @@
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue;
+import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -245,10 +248,12 @@
: new Artifact.DerivedArtifact(root, execPath, NULL_ARTIFACT_OWNER);
}
- public static TreeFileArtifact createTreeFileArtifactWithNoGeneratingAction(
- SpecialArtifact parent, String relativePath) {
- return ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- parent, PathFragment.create(relativePath), parent.getArtifactOwner());
+ public static SpecialArtifact createTreeArtifactWithGeneratingAction(
+ ArtifactRoot root, PathFragment execPath) {
+ SpecialArtifact treeArtifact =
+ new SpecialArtifact(root, execPath, NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
+ treeArtifact.setGeneratingActionKey(NULL_ACTION_LOOKUP_DATA);
+ return treeArtifact;
}
public static void assertNoArtifactEndingWith(RuleConfiguredTarget target, String path) {
@@ -354,6 +359,9 @@
}
};
+ public static final ActionTemplateExpansionKey NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER =
+ ActionTemplateExpansionValue.key(NULL_ARTIFACT_OWNER, /*actionIndex=*/ 0);
+
public static final Artifact DUMMY_ARTIFACT =
new Artifact.SourceArtifact(
ArtifactRoot.asSourceRoot(Root.absoluteRoot(new InMemoryFileSystem())),
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/BUILD b/src/test/java/com/google/devtools/build/lib/actions/util/BUILD
index 1bf7b60..c3dd8e6 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/BUILD
@@ -26,6 +26,7 @@
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:action_template_expansion_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java b/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java
index afaeeb3..35e98e3 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java
@@ -20,7 +20,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.AbstractAction;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionKeyContext;
@@ -168,6 +167,10 @@
this(inputs, output, MiddlemanType.NORMAL);
}
+ public DummyAction(Artifact input, Artifact output) {
+ this(NestedSetBuilder.create(Order.STABLE_ORDER, input), output);
+ }
+
@Override
public MiddlemanType getActionType() {
return type;
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 bf5e221..cddda1c 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
@@ -20,13 +20,11 @@
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext.LostInputsCheck;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionResult;
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.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.CommandLine;
@@ -45,10 +43,10 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Collection;
+import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -66,7 +64,7 @@
Path execRoot = scratch.getFileSystem().getPath("/exec");
rootDir = ArtifactRoot.asDerivedRoot(execRoot, "out");
outputArtifact = getBinArtifactWithNoOwner("destination.txt");
- FileSystemUtils.createDirectoryAndParents(outputArtifact.getPath().getParentDirectory());
+ outputArtifact.getPath().getParentDirectory().createDirectoryAndParents();
treeArtifact = createTreeArtifact("artifact/myTreeFileArtifact");
}
@@ -136,20 +134,8 @@
}
private SpecialArtifact createTreeArtifact(String rootRelativePath) {
- PathFragment relpath = PathFragment.create(rootRelativePath);
- return new SpecialArtifact(
- rootDir,
- rootDir.getExecPath().getRelative(relpath),
- ActionsTestUtil.NULL_ARTIFACT_OWNER,
- SpecialArtifactType.TREE);
- }
-
- private TreeFileArtifact createTreeFileArtifact(
- SpecialArtifact inputTreeArtifact, String parentRelativePath) {
- return ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- inputTreeArtifact,
- PathFragment.create(parentRelativePath),
- inputTreeArtifact.getArtifactOwner());
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
+ rootDir, rootDir.getExecPath().getRelative(rootRelativePath));
}
private ParameterFileWriteAction createParameterFileWriteAction(
@@ -186,9 +172,10 @@
}
private ActionExecutionContext actionExecutionContext() throws Exception {
- final Iterable<TreeFileArtifact> treeFileArtifacts = ImmutableList.of(
- createTreeFileArtifact(treeArtifact, "artifacts/treeFileArtifact1"),
- createTreeFileArtifact(treeArtifact, "artifacts/treeFileArtifact2"));
+ List<TreeFileArtifact> treeFileArtifacts =
+ ImmutableList.of(
+ TreeFileArtifact.createTreeOutput(treeArtifact, "artifacts/treeFileArtifact1"),
+ TreeFileArtifact.createTreeOutput(treeArtifact, "artifacts/treeFileArtifact2"));
ArtifactExpander artifactExpander = new ArtifactExpander() {
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
index da2483a..69d7897 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTemplateTest.java
@@ -18,7 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
@@ -190,58 +190,59 @@
}
@Test
- public void testExpandedAction_inputAndOutputTreeFileArtifacts() throws Exception {
+ public void testExpandedAction_inputAndOutputTreeFileArtifacts() {
SpawnActionTemplate actionTemplate = createSimpleSpawnActionTemplate();
SpecialArtifact inputTreeArtifact = createInputTreeArtifact();
SpecialArtifact outputTreeArtifact = createOutputTreeArtifact();
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts =
createInputTreeFileArtifacts(inputTreeArtifact);
List<SpawnAction> expandedActions =
- ImmutableList.copyOf(
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ActionsTestUtil.NULL_ARTIFACT_OWNER));
+ actionTemplate.generateActionsForInputArtifacts(
+ inputTreeFileArtifacts, ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER);
assertThat(expandedActions).hasSize(3);
for (int i = 0; i < expandedActions.size(); ++i) {
- String baseName = String.format("child%d", i);
+ String baseName = "child" + i;
assertThat(expandedActions.get(i).getInputs().toList())
.containsExactly(
- ActionInputHelper.treeFileArtifact(
- inputTreeArtifact, PathFragment.create("children/" + baseName)));
- assertThat(expandedActions.get(i).getOutputs()).containsExactly(
- ActionInputHelper.treeFileArtifact(
- outputTreeArtifact, PathFragment.create("children/" + baseName)));
+ TreeFileArtifact.createTreeOutput(inputTreeArtifact, "children/" + baseName));
+ assertThat(expandedActions.get(i).getOutputs())
+ .containsExactly(
+ TreeFileArtifact.createTemplateExpansionOutput(
+ outputTreeArtifact,
+ "children/" + baseName,
+ ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER));
}
}
@Test
- public void testExpandedAction_commonToolsAndInputs() throws Exception {
+ public void testExpandedAction_commonToolsAndInputs() {
SpecialArtifact inputTreeArtifact = createInputTreeArtifact();
SpecialArtifact outputTreeArtifact = createOutputTreeArtifact();
Artifact commonInput = createDerivedArtifact("common/input");
Artifact commonTool = createDerivedArtifact("common/tool");
Artifact executable = createDerivedArtifact("bin/cp");
- SpawnActionTemplate actionTemplate = builder(inputTreeArtifact, outputTreeArtifact)
- .setExecutionInfo(ImmutableMap.<String, String>of("local", ""))
- .setExecutable(executable)
- .setCommandLineTemplate(
- createSimpleCommandLineTemplate(inputTreeArtifact, outputTreeArtifact))
- .setOutputPathMapper(IDENTITY_MAPPER)
- .setMnemonics("ActionTemplate", "ExpandedAction")
- .addCommonTools(ImmutableList.of(commonTool))
- .addCommonInputs(ImmutableList.of(commonInput))
- .build(ActionsTestUtil.NULL_ACTION_OWNER);
+ SpawnActionTemplate actionTemplate =
+ builder(inputTreeArtifact, outputTreeArtifact)
+ .setExecutionInfo(ImmutableMap.of("local", ""))
+ .setExecutable(executable)
+ .setCommandLineTemplate(
+ createSimpleCommandLineTemplate(inputTreeArtifact, outputTreeArtifact))
+ .setOutputPathMapper(IDENTITY_MAPPER)
+ .setMnemonics("ActionTemplate", "ExpandedAction")
+ .addCommonTools(ImmutableList.of(commonTool))
+ .addCommonInputs(ImmutableList.of(commonInput))
+ .build(ActionsTestUtil.NULL_ACTION_OWNER);
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts =
createInputTreeFileArtifacts(inputTreeArtifact);
List<SpawnAction> expandedActions =
- ImmutableList.copyOf(
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ActionsTestUtil.NULL_ARTIFACT_OWNER));
+ actionTemplate.generateActionsForInputArtifacts(
+ inputTreeFileArtifacts, ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER);
for (int i = 0; i < expandedActions.size(); ++i) {
assertThat(expandedActions.get(i).getInputs().toList())
@@ -257,13 +258,12 @@
SpecialArtifact inputTreeArtifact = createInputTreeArtifact();
SpecialArtifact outputTreeArtifact = createOutputTreeArtifact();
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts =
createInputTreeFileArtifacts(inputTreeArtifact);
List<SpawnAction> expandedActions =
- ImmutableList.copyOf(
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ActionsTestUtil.NULL_ARTIFACT_OWNER));
+ actionTemplate.generateActionsForInputArtifacts(
+ inputTreeFileArtifacts, ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER);
assertThat(expandedActions).hasSize(3);
@@ -279,16 +279,15 @@
}
@Test
- public void testExpandedAction_executionInfoAndEnvironment() throws Exception {
+ public void testExpandedAction_executionInfoAndEnvironment() {
SpawnActionTemplate actionTemplate = createSimpleSpawnActionTemplate();
SpecialArtifact inputTreeArtifact = createInputTreeArtifact();
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts =
createInputTreeFileArtifacts(inputTreeArtifact);
List<SpawnAction> expandedActions =
- ImmutableList.copyOf(
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ActionsTestUtil.NULL_ARTIFACT_OWNER));
+ actionTemplate.generateActionsForInputArtifacts(
+ inputTreeFileArtifacts, ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER);
assertThat(expandedActions).hasSize(3);
@@ -303,7 +302,7 @@
public void testExpandedAction_illegalOutputPath() throws Exception {
SpecialArtifact inputTreeArtifact = createInputTreeArtifact();
SpecialArtifact outputTreeArtifact = createOutputTreeArtifact();
- Iterable<TreeFileArtifact> inputTreeFileArtifacts =
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts =
createInputTreeFileArtifacts(inputTreeArtifact);
SpawnActionTemplate.Builder builder = builder(inputTreeArtifact, outputTreeArtifact)
@@ -325,8 +324,8 @@
"Absolute output paths not allowed, expected IllegalArgumentException",
IllegalArgumentException.class,
() ->
- actionTemplate.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ActionsTestUtil.NULL_ARTIFACT_OWNER));
+ actionTemplate.generateActionsForInputArtifacts(
+ inputTreeFileArtifacts, ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER));
mapper = new OutputPathMapper() {
@Override
@@ -342,8 +341,8 @@
"Output paths containing '..' not allowed, expected IllegalArgumentException",
IllegalArgumentException.class,
() ->
- actionTemplate2.generateActionForInputArtifacts(
- inputTreeFileArtifacts, ActionsTestUtil.NULL_ARTIFACT_OWNER));
+ actionTemplate2.generateActionsForInputArtifacts(
+ inputTreeFileArtifacts, ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER));
}
private SpawnActionTemplate.Builder builder(
@@ -356,8 +355,8 @@
SpecialArtifact outputTreeArtifact = createOutputTreeArtifact();
return builder(inputTreeArtifact, outputTreeArtifact)
- .setExecutionInfo(ImmutableMap.<String, String>of("local", ""))
- .setEnvironment(ImmutableMap.<String, String>of("env", "value"))
+ .setExecutionInfo(ImmutableMap.of("local", ""))
+ .setEnvironment(ImmutableMap.of("env", "value"))
.setExecutable(PathFragment.create("/bin/cp"))
.setCommandLineTemplate(
createSimpleCommandLineTemplate(inputTreeArtifact, outputTreeArtifact))
@@ -398,13 +397,11 @@
.build();
}
- private Iterable<TreeFileArtifact> createInputTreeFileArtifacts(
+ private static ImmutableSet<TreeFileArtifact> createInputTreeFileArtifacts(
SpecialArtifact inputTreeArtifact) {
- return ActionInputHelper.asTreeFileArtifacts(
- inputTreeArtifact,
- ImmutableList.of(
- PathFragment.create("children/child0"),
- PathFragment.create("children/child1"),
- PathFragment.create("children/child2")));
+ return ImmutableSet.of(
+ TreeFileArtifact.createTreeOutput(inputTreeArtifact, "children/child0"),
+ TreeFileArtifact.createTreeOutput(inputTreeArtifact, "children/child1"),
+ TreeFileArtifact.createTreeOutput(inputTreeArtifact, "children/child2"));
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
index 062ea75..16eac40 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java
@@ -270,10 +270,8 @@
public void testRunfilesWithTreeArtifacts() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("treeArtifact");
assertThat(treeArtifact.isTreeArtifact()).isTrue();
- TreeFileArtifact file1 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file1");
- TreeFileArtifact file2 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file2");
+ TreeFileArtifact file1 = TreeFileArtifact.createTreeOutput(treeArtifact, "file1");
+ TreeFileArtifact file2 = TreeFileArtifact.createTreeOutput(treeArtifact, "file2");
FileSystemUtils.writeContentAsLatin1(file1.getPath(), "foo");
FileSystemUtils.writeContentAsLatin1(file2.getPath(), "bar");
@@ -302,10 +300,8 @@
public void testRunfilesWithTreeArtifactsInSymlinks() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("treeArtifact");
assertThat(treeArtifact.isTreeArtifact()).isTrue();
- TreeFileArtifact file1 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file1");
- TreeFileArtifact file2 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file2");
+ TreeFileArtifact file1 = TreeFileArtifact.createTreeOutput(treeArtifact, "file1");
+ TreeFileArtifact file2 = TreeFileArtifact.createTreeOutput(treeArtifact, "file2");
FileSystemUtils.writeContentAsLatin1(file1.getPath(), "foo");
FileSystemUtils.writeContentAsLatin1(file2.getPath(), "bar");
Runfiles runfiles =
@@ -337,10 +333,8 @@
public void testTreeArtifactsInInputs() throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact("treeArtifact");
assertThat(treeArtifact.isTreeArtifact()).isTrue();
- TreeFileArtifact file1 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file1");
- TreeFileArtifact file2 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "file2");
+ TreeFileArtifact file1 = TreeFileArtifact.createTreeOutput(treeArtifact, "file1");
+ TreeFileArtifact file2 = TreeFileArtifact.createTreeOutput(treeArtifact, "file2");
FileSystemUtils.writeContentAsLatin1(file1.getPath(), "foo");
FileSystemUtils.writeContentAsLatin1(file2.getPath(), "bar");
@@ -363,7 +357,9 @@
}
private SpecialArtifact createTreeArtifact(String relPath) throws IOException {
- return createSpecialArtifact(relPath, SpecialArtifactType.TREE);
+ SpecialArtifact treeArtifact = createSpecialArtifact(relPath, SpecialArtifactType.TREE);
+ treeArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ return treeArtifact;
}
private SpecialArtifact createFilesetArtifact(String relPath) throws IOException {
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
index 13790d5..e95fab7 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
+import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
@@ -991,11 +992,11 @@
// assert
assertThat(inMemoryOutput).isNull();
- Map<Artifact.TreeFileArtifact, RemoteFileArtifactValue> m =
+ Map<TreeFileArtifact, RemoteFileArtifactValue> m =
ImmutableMap.of(
- ActionInputHelper.treeFileArtifact(dir, "file1"),
+ TreeFileArtifact.createTreeOutput(dir, "file1"),
new RemoteFileArtifactValue(toBinaryDigest(d1), d1.getSizeBytes(), 1, "action-id"),
- ActionInputHelper.treeFileArtifact(dir, "a/file2"),
+ TreeFileArtifact.createTreeOutput(dir, "a/file2"),
new RemoteFileArtifactValue(toBinaryDigest(d2), d2.getSizeBytes(), 1, "action-id"));
verify(injector).injectRemoteDirectory(eq(dir), eq(m));
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
index 9a622e7..40ffbc6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -28,7 +28,6 @@
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.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ResourceSet;
@@ -888,11 +887,8 @@
FileSystem fs = scratch.getFileSystem();
Path execRoot = fs.getPath(TestUtils.tmpDir());
PathFragment execPath = PathFragment.create("out").getRelative(name);
- return new SpecialArtifact(
- ArtifactRoot.asDerivedRoot(execRoot, "out"),
- execPath,
- ActionsTestUtil.NULL_ARTIFACT_OWNER,
- SpecialArtifactType.TREE);
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
+ ArtifactRoot.asDerivedRoot(execRoot, "out"), execPath);
}
private void verifyArguments(
@@ -909,12 +905,8 @@
SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory");
- TreeFileArtifact library0 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- testTreeArtifact, "library0.o");
- TreeFileArtifact library1 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- testTreeArtifact, "library1.o");
+ TreeFileArtifact library0 = TreeFileArtifact.createTreeOutput(testTreeArtifact, "library0.o");
+ TreeFileArtifact library1 = TreeFileArtifact.createTreeOutput(testTreeArtifact, "library1.o");
ArtifactExpander expander =
new ArtifactExpander() {
@@ -964,12 +956,8 @@
SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory");
- TreeFileArtifact library0 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- testTreeArtifact, "library0.o");
- TreeFileArtifact library1 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- testTreeArtifact, "library1.o");
+ TreeFileArtifact library0 = TreeFileArtifact.createTreeOutput(testTreeArtifact, "library0.o");
+ TreeFileArtifact library1 = TreeFileArtifact.createTreeOutput(testTreeArtifact, "library1.o");
ArtifactExpander expander =
(artifact, output) -> {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLineTest.java
index 820bd9e..448d820 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLineTest.java
@@ -23,7 +23,6 @@
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.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
@@ -392,11 +391,8 @@
FileSystem fs = scratch.getFileSystem();
Path execRoot = fs.getPath(TestUtils.tmpDir());
PathFragment execPath = PathFragment.create("out").getRelative(name);
- return new SpecialArtifact(
- ArtifactRoot.asDerivedRoot(execRoot, "out"),
- execPath,
- ActionsTestUtil.NULL_ARTIFACT_OWNER,
- SpecialArtifactType.TREE);
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
+ ArtifactRoot.asDerivedRoot(execRoot, "out"), execPath);
}
private void verifyArguments(
@@ -411,12 +407,8 @@
public void testTreeArtifactLink() throws Exception {
SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory");
- TreeFileArtifact library0 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- testTreeArtifact, "library0.o");
- TreeFileArtifact library1 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- testTreeArtifact, "library1.o");
+ TreeFileArtifact library0 = TreeFileArtifact.createTreeOutput(testTreeArtifact, "library0.o");
+ TreeFileArtifact library1 = TreeFileArtifact.createTreeOutput(testTreeArtifact, "library1.o");
ArtifactExpander expander =
new ArtifactExpander() {
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 0d24d16..7ac240b 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
@@ -21,13 +21,13 @@
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionContext.LostInputsCheck;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionTemplate.ActionTemplateExpansionException;
import com.google.devtools.build.lib.actions.Artifact;
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType;
import com.google.devtools.build.lib.rules.apple.AppleToolchain;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
+import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionTemplate;
import com.google.devtools.build.lib.rules.cpp.CppModuleMapAction;
import com.google.devtools.build.lib.rules.cpp.UmbrellaHeaderAction;
@@ -1174,16 +1175,13 @@
}
/** Returns the actions created by the action template corresponding to given artifact. */
- protected Iterable<CommandAction> getActionsForInputsOfGeneratingActionTemplate(
+ protected ImmutableList<CppCompileAction> getActionsForInputsOfGeneratingActionTemplate(
Artifact artifact, TreeFileArtifact treeFileArtifact)
throws ActionTemplateExpansionException {
CppCompileActionTemplate template =
(CppCompileActionTemplate) getActionGraph().getGeneratingAction(artifact);
- return ImmutableList.<CommandAction>builder()
- .addAll(
- template.generateActionForInputArtifacts(
- ImmutableList.of(treeFileArtifact), ActionsTestUtil.NULL_ARTIFACT_OWNER))
- .build();
+ return template.generateActionsForInputArtifacts(
+ ImmutableSet.of(treeFileArtifact), ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER);
}
@Test
@@ -1214,10 +1212,10 @@
// Therefore we need to fake some files inside them to test the action template in this
// analysis-time test.
TreeFileArtifact oneSourceFileFromGenJar =
- ActionInputHelper.treeFileArtifact((SpecialArtifact) sourceFilesFromGenJar, "children1.m");
+ TreeFileArtifact.createTreeOutput((SpecialArtifact) sourceFilesFromGenJar, "children1.m");
TreeFileArtifact oneObjFileFromGenJar =
- ActionInputHelper.treeFileArtifact((SpecialArtifact) objectFilesFromGenJar, "children1.o");
- Iterable<CommandAction> compileActions =
+ TreeFileArtifact.createTreeOutput((SpecialArtifact) objectFilesFromGenJar, "children1.o");
+ Iterable<CppCompileAction> compileActions =
getActionsForInputsOfGeneratingActionTemplate(
objectFilesFromGenJar, oneSourceFileFromGenJar);
CommandAction compileAction = Iterables.getOnlyElement(compileActions);
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibraryTest.java
index b354483..e607c4d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibraryTest.java
@@ -14,10 +14,10 @@
package com.google.devtools.build.lib.rules.objc;
-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.SpecialArtifact;
+import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.util.MockJ2ObjcSupport;
@@ -35,8 +35,9 @@
new ArtifactExpander() {
@Override
public void expand(Artifact artifact, Collection<? super Artifact> output) {
- output.add(ActionInputHelper.treeFileArtifact((SpecialArtifact) artifact, "children1"));
- output.add(ActionInputHelper.treeFileArtifact((SpecialArtifact) artifact, "children2"));
+ SpecialArtifact parent = (SpecialArtifact) artifact;
+ output.add(TreeFileArtifact.createTreeOutput(parent, "children1"));
+ output.add(TreeFileArtifact.createTreeOutput(parent, "children2"));
}
};
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
index 4534431..db232b7 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandlerTest.java
@@ -23,7 +23,6 @@
import com.google.devtools.build.lib.actions.ActionInputMap;
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.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ArtifactRoot;
@@ -219,9 +218,8 @@
public void withUnknownOutputArtifactMissingAllowedTreeArtifact() throws Exception {
PathFragment path = PathFragment.create("bin/foo/bar");
SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ActionsTestUtil.NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
- Artifact artifact = new TreeFileArtifact(treeArtifact, PathFragment.create("baz"));
+ ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, path);
+ Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz");
ActionInputMap map = new ActionInputMap(1);
ActionMetadataHandler handler =
new ActionMetadataHandler(
@@ -241,9 +239,8 @@
scratch.file("/output/bin/foo/bar/baz", "not empty");
PathFragment path = PathFragment.create("bin/foo/bar");
SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ActionsTestUtil.NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
- Artifact artifact = new TreeFileArtifact(treeArtifact, PathFragment.create("baz"));
+ ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, path);
+ Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz");
assertThat(artifact.getPath().exists()).isTrue();
ActionInputMap map = new ActionInputMap(1);
ActionMetadataHandler handler =
@@ -260,12 +257,11 @@
}
@Test
- public void withUnknownOutputArtifactMissingDisallowedTreeArtifact() throws Exception {
+ public void withUnknownOutputArtifactMissingDisallowedTreeArtifact() {
PathFragment path = PathFragment.create("bin/foo/bar");
SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ActionsTestUtil.NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
- Artifact artifact = new TreeFileArtifact(treeArtifact, PathFragment.create("baz"));
+ ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, path);
+ Artifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, "baz");
ActionInputMap map = new ActionInputMap(1);
ActionMetadataHandler handler =
new ActionMetadataHandler(
@@ -372,9 +368,7 @@
public void injectRemoteTreeArtifactMetadata() throws Exception {
PathFragment path = PathFragment.create("bin/dir");
SpecialArtifact treeArtifact =
- new SpecialArtifact(
- outputRoot, path, ActionsTestUtil.NULL_ARTIFACT_OWNER, SpecialArtifactType.TREE);
- treeArtifact.setGeneratingActionKey(ActionsTestUtil.NULL_ACTION_LOOKUP_DATA);
+ ActionsTestUtil.createTreeArtifactWithGeneratingAction(outputRoot, path);
OutputStore store = new OutputStore();
ActionMetadataHandler handler =
new ActionMetadataHandler(
@@ -394,8 +388,8 @@
new RemoteFileArtifactValue(new byte[] {4, 5, 6}, 10, 1, "bar");
Map<TreeFileArtifact, RemoteFileArtifactValue> children =
ImmutableMap.of(
- ActionInputHelper.treeFileArtifact(treeArtifact, "foo"), fooValue,
- ActionInputHelper.treeFileArtifact(treeArtifact, "bar"), barValue);
+ TreeFileArtifact.createTreeOutput(treeArtifact, "foo"), fooValue,
+ TreeFileArtifact.createTreeOutput(treeArtifact, "bar"), barValue);
handler.injectRemoteDirectory(treeArtifact, children);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
index 720090b..22b13e4 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java
@@ -19,13 +19,20 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionKeyContext;
+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.ActionOwner;
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
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;
@@ -36,33 +43,33 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.InjectedActionLookupKey;
+import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate.OutputPathMapper;
+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.events.NullEventHandler;
-import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
+import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
-import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.SequencedRecordingDifferencer;
import com.google.devtools.build.skyframe.SequentialBuildDriver;
import com.google.devtools.build.skyframe.SkyFunction;
-import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
-import java.util.concurrent.atomic.AtomicReference;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -72,32 +79,28 @@
/** Tests for {@link ActionTemplateExpansionFunction}. */
@RunWith(JUnit4.class)
public final class ActionTemplateExpansionFunctionTest extends FoundationTestCase {
- private Map<Artifact, TreeArtifactValue> artifactValueMap;
- private SequentialBuildDriver driver;
- private SequencedRecordingDifferencer differencer;
+
+ private final Map<Artifact, TreeArtifactValue> artifactValueMap = new LinkedHashMap<>();
+ private final SequencedRecordingDifferencer differencer = new SequencedRecordingDifferencer();
+ private final SequentialBuildDriver driver =
+ new SequentialBuildDriver(
+ new InMemoryMemoizingEvaluator(
+ ImmutableMap.of(
+ Artifact.ARTIFACT,
+ new DummyArtifactFunction(artifactValueMap),
+ SkyFunctions.ACTION_TEMPLATE_EXPANSION,
+ new ActionTemplateExpansionFunction(new ActionKeyContext())),
+ differencer));
@Before
- public void setUp() throws Exception {
- artifactValueMap = new LinkedHashMap<>();
- AtomicReference<PathPackageLocator> pkgLocator =
- new AtomicReference<>(
- new PathPackageLocator(
- rootDirectory.getFileSystem().getPath("/outputbase"),
- ImmutableList.of(Root.fromPath(rootDirectory)),
- BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY));
- differencer = new SequencedRecordingDifferencer();
- MemoizingEvaluator evaluator =
- new InMemoryMemoizingEvaluator(
- ImmutableMap.<SkyFunctionName, SkyFunction>builder()
- .put(Artifact.ARTIFACT, new DummyArtifactFunction(artifactValueMap))
- .put(
- SkyFunctions.ACTION_TEMPLATE_EXPANSION,
- new ActionTemplateExpansionFunction(new ActionKeyContext()))
- .build(),
- differencer);
- driver = new SequentialBuildDriver(evaluator);
+ public void setUp() {
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
- PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+ PrecomputedValue.PATH_PACKAGE_LOCATOR.set(
+ differencer,
+ new PathPackageLocator(
+ rootDirectory.getFileSystem().getPath("/outputbase"),
+ ImmutableList.of(Root.fromPath(rootDirectory)),
+ BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY));
}
@Test
@@ -182,10 +185,161 @@
assertThrows(ArtifactPrefixConflictException.class, () -> evaluate(spawnActionTemplate));
}
+ @Test
+ public void cannotDeclareNonTreeOutput() throws Exception {
+ SpecialArtifact inputTree = createAndPopulateTreeArtifact("input", "child");
+ SpecialArtifact outputTree = createTreeArtifact("output");
+
+ ActionTemplate<DummyAction> template =
+ new TestActionTemplate(inputTree, outputTree) {
+ @Override
+ public ImmutableList<DummyAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts,
+ ActionLookupKey artifactOwner) {
+ return ImmutableList.of();
+ }
+
+ @Override
+ public ImmutableSet<Artifact> getOutputs() {
+ return ImmutableSet.of(
+ outputTree,
+ new DerivedArtifact(
+ outputTree.getRoot(),
+ outputTree.getRoot().getExecPath().getRelative("not_tree"),
+ outputTree.getArtifactOwner()));
+ }
+ };
+
+ Exception e = assertThrows(RuntimeException.class, () -> evaluate(template));
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+ assertThat(e)
+ .hasCauseThat()
+ .hasMessageThat()
+ .contains(template + " declares an output which is not a tree artifact");
+ }
+
+ @Test
+ public void cannotGenerateOutputWithWrongOwner() throws Exception {
+ SpecialArtifact inputTree = createAndPopulateTreeArtifact("input", "child");
+ SpecialArtifact outputTree = createTreeArtifact("output");
+
+ ActionTemplate<DummyAction> template =
+ new TestActionTemplate(inputTree, outputTree) {
+ @Override
+ public ImmutableList<DummyAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts,
+ ActionLookupKey artifactOwner) {
+ TreeFileArtifact input = Iterables.getOnlyElement(inputTreeFileArtifacts);
+ TreeFileArtifact outputWithWrongOwner =
+ TreeFileArtifact.createTemplateExpansionOutput(
+ outputTree, "child", ActionsTestUtil.NULL_TEMPLATE_EXPANSION_ARTIFACT_OWNER);
+ assertThat(outputWithWrongOwner.getArtifactOwner()).isNotEqualTo(artifactOwner);
+ return ImmutableList.of(new DummyAction(input, outputWithWrongOwner));
+ }
+ };
+
+ Exception e = assertThrows(RuntimeException.class, () -> evaluate(template));
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+ assertThat(e)
+ .hasCauseThat()
+ .hasMessageThat()
+ .contains(template + " generated an action with an output owned by the wrong owner");
+ }
+
+ @Test
+ public void cannotGenerateNonTreeFileArtifactOutput() throws Exception {
+ SpecialArtifact inputTree = createAndPopulateTreeArtifact("input", "child");
+ SpecialArtifact outputTree = createTreeArtifact("output");
+
+ ActionTemplate<DummyAction> template =
+ new TestActionTemplate(inputTree, outputTree) {
+ @Override
+ public ImmutableList<DummyAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts,
+ ActionLookupKey artifactOwner) {
+ TreeFileArtifact input = Iterables.getOnlyElement(inputTreeFileArtifacts);
+ Artifact notTreeFileArtifact =
+ new DerivedArtifact(
+ input.getRoot(),
+ input.getRoot().getExecPath().getRelative("a.txt"),
+ artifactOwner);
+ assertThat(notTreeFileArtifact.isTreeArtifact()).isFalse();
+ return ImmutableList.of(new DummyAction(input, notTreeFileArtifact));
+ }
+ };
+
+ Exception e = assertThrows(RuntimeException.class, () -> evaluate(template));
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+ assertThat(e)
+ .hasCauseThat()
+ .hasMessageThat()
+ .contains(template + " generated an action which outputs a non-TreeFileArtifact");
+ }
+
+ @Test
+ public void cannotGenerateOutputUnderUndeclaredTree() throws Exception {
+ SpecialArtifact inputTree = createAndPopulateTreeArtifact("input", "child");
+ SpecialArtifact outputTree = createTreeArtifact("output");
+
+ ActionTemplate<DummyAction> template =
+ new TestActionTemplate(inputTree, outputTree) {
+ @Override
+ public ImmutableList<DummyAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts,
+ ActionLookupKey artifactOwner) {
+ TreeFileArtifact input = Iterables.getOnlyElement(inputTreeFileArtifacts);
+ TreeFileArtifact outputUnderWrongTree =
+ TreeFileArtifact.createTemplateExpansionOutput(
+ createTreeArtifact("undeclared"), "child", artifactOwner);
+ return ImmutableList.of(new DummyAction(input, outputUnderWrongTree));
+ }
+ };
+
+ Exception e = assertThrows(RuntimeException.class, () -> evaluate(template));
+ assertThat(e).hasCauseThat().isInstanceOf(IllegalStateException.class);
+ assertThat(e)
+ .hasCauseThat()
+ .hasMessageThat()
+ .contains(template + " generated an action with an output under an undeclared tree");
+ }
+
+ @Test
+ public void canGenerateOutputUnderAdditionalDeclaredTree() throws Exception {
+ SpecialArtifact inputTree = createAndPopulateTreeArtifact("input", "child");
+ SpecialArtifact outputTree = createTreeArtifact("output");
+ SpecialArtifact additionalOutputTree = createTreeArtifact("additional_output");
+
+ ActionTemplate<DummyAction> template =
+ new TestActionTemplate(inputTree, outputTree) {
+ @Override
+ public ImmutableList<DummyAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts,
+ ActionLookupKey artifactOwner) {
+ TreeFileArtifact input = Iterables.getOnlyElement(inputTreeFileArtifacts);
+ return ImmutableList.of(
+ new DummyAction(
+ input,
+ TreeFileArtifact.createTemplateExpansionOutput(
+ outputTree, "child", artifactOwner)),
+ new DummyAction(
+ input,
+ TreeFileArtifact.createTemplateExpansionOutput(
+ additionalOutputTree, "additional_child", artifactOwner)));
+ }
+
+ @Override
+ public ImmutableSet<Artifact> getOutputs() {
+ return ImmutableSet.of(outputTree, additionalOutputTree);
+ }
+ };
+
+ evaluate(template);
+ }
+
private static final ActionLookupValue.ActionLookupKey CTKEY = new InjectedActionLookupKey("key");
- private List<Action> evaluate(SpawnActionTemplate spawnActionTemplate) throws Exception {
- ConfiguredTargetValue ctValue = createConfiguredTargetValue(spawnActionTemplate);
+ private ImmutableList<Action> evaluate(ActionTemplate<?> actionTemplate) throws Exception {
+ ConfiguredTargetValue ctValue = createConfiguredTargetValue(actionTemplate);
differencer.inject(CTKEY, ctValue);
ActionTemplateExpansionKey templateKey = ActionTemplateExpansionValue.key(CTKEY, 0);
@@ -213,7 +367,7 @@
return new NonRuleConfiguredTargetValue(
Mockito.mock(ConfiguredTarget.class),
Actions.GeneratingActions.fromSingleAction(actionTemplate, CTKEY),
- NestedSetBuilder.<Package>stableOrder().build());
+ NestedSetBuilder.emptySet(Order.STABLE_ORDER));
}
private SpecialArtifact createTreeArtifact(String path) {
@@ -228,12 +382,12 @@
private SpecialArtifact createAndPopulateTreeArtifact(String path, String... childRelativePaths)
throws Exception {
SpecialArtifact treeArtifact = createTreeArtifact(path);
+ treeArtifact.setGeneratingActionKey(ActionLookupData.create(CTKEY, /*actionIndex=*/ 0));
Map<TreeFileArtifact, FileArtifactValue> treeFileArtifactMap = new LinkedHashMap<>();
for (String childRelativePath : childRelativePaths) {
TreeFileArtifact treeFileArtifact =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifact, childRelativePath);
+ TreeFileArtifact.createTreeOutput(treeArtifact, childRelativePath);
scratch.file(treeFileArtifact.getPath().toString(), childRelativePath);
// We do not care about the FileArtifactValues in this test.
treeFileArtifactMap.put(
@@ -247,7 +401,7 @@
}
/** Dummy ArtifactFunction that just returns injected values */
- private static class DummyArtifactFunction implements SkyFunction {
+ private static final class DummyArtifactFunction implements SkyFunction {
private final Map<Artifact, TreeArtifactValue> artifactValueMap;
DummyArtifactFunction(Map<Artifact, TreeArtifactValue> artifactValueMap) {
@@ -263,4 +417,100 @@
return null;
}
}
+
+ private abstract static class TestActionTemplate implements ActionTemplate<DummyAction> {
+ private final SpecialArtifact inputTreeArtifact;
+ private final SpecialArtifact outputTreeArtifact;
+
+ TestActionTemplate(SpecialArtifact inputTreeArtifact, SpecialArtifact outputTreeArtifact) {
+ Preconditions.checkArgument(inputTreeArtifact.isTreeArtifact(), inputTreeArtifact);
+ Preconditions.checkArgument(outputTreeArtifact.isTreeArtifact(), outputTreeArtifact);
+ this.inputTreeArtifact = inputTreeArtifact;
+ this.outputTreeArtifact = outputTreeArtifact;
+ }
+
+ @Override
+ public SpecialArtifact getInputTreeArtifact() {
+ return inputTreeArtifact;
+ }
+
+ @Override
+ public SpecialArtifact getOutputTreeArtifact() {
+ return outputTreeArtifact;
+ }
+
+ @Override
+ public ActionOwner getOwner() {
+ return ActionsTestUtil.NULL_ACTION_OWNER;
+ }
+
+ @Override
+ public boolean isShareable() {
+ return false;
+ }
+
+ @Override
+ public String getMnemonic() {
+ return "TestActionTemplate";
+ }
+
+ @Override
+ public String getKey(ActionKeyContext actionKeyContext) {
+ Fingerprint fp = new Fingerprint();
+ fp.addPath(inputTreeArtifact.getPath());
+ fp.addPath(outputTreeArtifact.getPath());
+ return fp.hexDigestAndReset();
+ }
+
+ @Override
+ public String prettyPrint() {
+ return "TestActionTemplate for " + outputTreeArtifact;
+ }
+
+ @Override
+ public NestedSet<Artifact> getTools() {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
+
+ @Override
+ public NestedSet<Artifact> getInputs() {
+ return NestedSetBuilder.create(Order.STABLE_ORDER, inputTreeArtifact);
+ }
+
+ @Override
+ public Iterable<String> getClientEnvironmentVariables() {
+ return ImmutableList.of();
+ }
+
+ @Override
+ public NestedSet<Artifact> getInputFilesForExtraAction(
+ ActionExecutionContext actionExecutionContext) {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
+
+ @Override
+ public ImmutableSet<Artifact> getMandatoryOutputs() {
+ return ImmutableSet.of();
+ }
+
+ @Override
+ public NestedSet<Artifact> getMandatoryInputs() {
+ return NestedSetBuilder.create(Order.STABLE_ORDER, inputTreeArtifact);
+ }
+
+ @Override
+ public boolean shouldReportPathPrefixConflict(ActionAnalysisMetadata action) {
+ return false;
+ }
+
+ @Override
+ public MiddlemanType getActionType() {
+ return MiddlemanType.NORMAL;
+ }
+
+ @Override
+ public String toString() {
+ return prettyPrint();
+ }
+ }
}
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 28e4fef..9bb433f 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
@@ -23,7 +23,6 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Actions;
@@ -204,15 +203,9 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
TreeFileArtifact treeFileArtifact1 =
- createFakeTreeFileArtifact(
- artifact2,
- "child1",
- "hello1");
+ createFakeExpansionTreeFileArtifact(artifact2, "child1", "hello1");
TreeFileArtifact treeFileArtifact2 =
- createFakeTreeFileArtifact(
- artifact2,
- "child2",
- "hello2");
+ createFakeExpansionTreeFileArtifact(artifact2, "child2", "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
@@ -233,23 +226,17 @@
// artifact2 is a tree artifact generated by action template.
SpecialArtifact artifact2 = createDerivedTreeArtifactOnly("treeArtifact2");
- createFakeTreeFileArtifact(artifact2, "child1", "hello1");
- createFakeTreeFileArtifact(artifact2, "child2", "hello2");
+ createFakeExpansionTreeFileArtifact(artifact2, "child1", "hello1");
+ createFakeExpansionTreeFileArtifact(artifact2, "child2", "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact1, artifact2));
// artifact3 is a tree artifact generated by action template.
SpecialArtifact artifact3 = createDerivedTreeArtifactOnly("treeArtifact3");
TreeFileArtifact treeFileArtifact1 =
- createFakeTreeFileArtifact(
- artifact3,
- "child1",
- "hello1");
+ createFakeExpansionTreeFileArtifact(artifact3, "child1", "hello1");
TreeFileArtifact treeFileArtifact2 =
- createFakeTreeFileArtifact(
- artifact3,
- "child2",
- "hello2");
+ createFakeExpansionTreeFileArtifact(artifact3, "child2", "hello2");
actions.add(
ActionsTestUtil.createDummySpawnActionTemplate(artifact2, artifact3));
@@ -268,9 +255,9 @@
ActionMetadataHandler.fileArtifactValueFromArtifact(artifact1, null, null);
SpecialArtifact treeArtifact = createDerivedTreeArtifactOnly("tree");
treeArtifact.setGeneratingActionKey(dummyData);
- TreeFileArtifact treeFileArtifact = ActionInputHelper.treeFileArtifact(treeArtifact, "subpath");
+ TreeFileArtifact treeFileArtifact = TreeFileArtifact.createTreeOutput(treeArtifact, "subpath");
Path path = treeFileArtifact.getPath();
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
writeFile(path, "contents");
TreeArtifactValue treeArtifactValue =
TreeArtifactValue.create(
@@ -324,6 +311,7 @@
private SpecialArtifact createDerivedTreeArtifactWithAction(String path) {
SpecialArtifact treeArtifact = createDerivedTreeArtifactOnly(path);
actions.add(new DummyAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), treeArtifact));
+ treeArtifact.setGeneratingActionKey(ActionLookupData.create(ALL_OWNER, actions.size() - 1));
return treeArtifact;
}
@@ -336,10 +324,22 @@
private static TreeFileArtifact createFakeTreeFileArtifact(
SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
TreeFileArtifact treeFileArtifact =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(
- treeArtifact, parentRelativePath);
+ TreeFileArtifact.createTreeOutput(treeArtifact, parentRelativePath);
Path path = treeFileArtifact.getPath();
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
+ writeFile(path, content);
+ return treeFileArtifact;
+ }
+
+ private static TreeFileArtifact createFakeExpansionTreeFileArtifact(
+ SpecialArtifact treeArtifact, String parentRelativePath, String content) throws Exception {
+ TreeFileArtifact treeFileArtifact =
+ TreeFileArtifact.createTemplateExpansionOutput(
+ treeArtifact,
+ parentRelativePath,
+ ActionTemplateExpansionValue.key(ALL_OWNER, /*actionIndex=*/ 0));
+ Path path = treeFileArtifact.getPath();
+ path.getParentDirectory().createDirectoryAndParents();
writeFile(path, content);
return treeFileArtifact;
}
@@ -413,10 +413,10 @@
try {
if (output.isTreeArtifact()) {
- TreeFileArtifact treeFileArtifact1 = ActionInputHelper.treeFileArtifact(
- (SpecialArtifact) output, PathFragment.create("child1"));
- TreeFileArtifact treeFileArtifact2 = ActionInputHelper.treeFileArtifact(
- (SpecialArtifact) output, PathFragment.create("child2"));
+ TreeFileArtifact treeFileArtifact1 =
+ TreeFileArtifact.createTreeOutput((SpecialArtifact) output, "child1");
+ TreeFileArtifact treeFileArtifact2 =
+ TreeFileArtifact.createTreeOutput((SpecialArtifact) output, "child2");
TreeArtifactValue treeArtifactValue =
TreeArtifactValue.create(
ImmutableMap.of(
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 f12f1b6..2732dc1 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
@@ -23,12 +23,10 @@
import com.google.common.hash.HashCode;
import com.google.common.util.concurrent.Runnables;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -507,28 +505,25 @@
// contents into ActionExecutionValues.
SpecialArtifact out1 = createTreeArtifact("one");
- TreeFileArtifact file11 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out1, "fizz");
- FileSystemUtils.createDirectoryAndParents(out1.getPath());
+ TreeFileArtifact file11 = TreeFileArtifact.createTreeOutput(out1, "fizz");
+ out1.getPath().createDirectoryAndParents();
FileSystemUtils.writeContentAsLatin1(file11.getPath(), "buzz");
SpecialArtifact out2 = createTreeArtifact("two");
- FileSystemUtils.createDirectoryAndParents(out2.getPath().getChild("subdir"));
- TreeFileArtifact file21 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out2, "moony");
- TreeFileArtifact file22 =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out2, "subdir/wormtail");
+ out2.getPath().getChild("subdir").createDirectoryAndParents();
+ TreeFileArtifact file21 = TreeFileArtifact.createTreeOutput(out2, "moony");
+ TreeFileArtifact file22 = TreeFileArtifact.createTreeOutput(out2, "subdir/wormtail");
FileSystemUtils.writeContentAsLatin1(file21.getPath(), "padfoot");
FileSystemUtils.writeContentAsLatin1(file22.getPath(), "prongs");
SpecialArtifact outEmpty = createTreeArtifact("empty");
- FileSystemUtils.createDirectoryAndParents(outEmpty.getPath());
+ outEmpty.getPath().createDirectoryAndParents();
SpecialArtifact outUnchanging = createTreeArtifact("untouched");
- FileSystemUtils.createDirectoryAndParents(outUnchanging.getPath());
+ outUnchanging.getPath().createDirectoryAndParents();
SpecialArtifact last = createTreeArtifact("zzzzzzzzzz");
- FileSystemUtils.createDirectoryAndParents(last.getPath());
+ last.getPath().createDirectoryAndParents();
ActionLookupKey actionLookupKey =
new ActionLookupKey() {
@@ -561,7 +556,7 @@
.setNumThreads(1)
.setEventHander(NullEventHandler.INSTANCE)
.build();
- assertThat(driver.evaluate(ImmutableList.<SkyKey>of(), evaluationContext).hasError()).isFalse();
+ assertThat(driver.evaluate(ImmutableList.of(), evaluationContext).hasError()).isFalse();
assertThat(
new FilesystemValueChecker(
/* tsgm= */ null, /* lastExecutionTimeRange= */ null, FSVC_THREADS_FOR_TEST)
@@ -643,13 +638,11 @@
.containsExactly(actionKey1);
// Test that directory contents (and nested contents) matter
- Artifact out1new =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(out1, "julius/caesar");
+ Artifact out1new = TreeFileArtifact.createTreeOutput(out1, "julius/caesar");
FileSystemUtils.createDirectoryAndParents(out1.getPath().getChild("julius"));
FileSystemUtils.writeContentAsLatin1(out1new.getPath(), "octavian");
// even for empty directories
- Artifact outEmptyNew =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outEmpty, "marcus");
+ Artifact outEmptyNew = TreeFileArtifact.createTreeOutput(outEmpty, "marcus");
FileSystemUtils.writeContentAsLatin1(outEmptyNew.getPath(), "aurelius");
// so does removing
file21.getPath().delete();
@@ -760,11 +753,9 @@
Path outputPath = outputDir.getRelative(relPath);
outputDir.createDirectory();
ArtifactRoot derivedRoot = ArtifactRoot.asDerivedRoot(fs.getPath("/"), outSegment);
- return new SpecialArtifact(
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
derivedRoot,
- derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)),
- ActionsTestUtil.NULL_ARTIFACT_OWNER,
- SpecialArtifactType.TREE);
+ derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)));
}
@Test
@@ -947,9 +938,7 @@
ImmutableMap.builder();
for (Map.Entry<PathFragment, RemoteFileArtifactValue> child : children.entrySet()) {
childFileValues.put(
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- output, child.getKey(), output.getArtifactOwner()),
- child.getValue());
+ TreeFileArtifact.createTreeOutput(output, child.getKey()), child.getValue());
}
TreeArtifactValue treeArtifactValue = TreeArtifactValue.create(childFileValues.build());
return ActionExecutionValue.create(
@@ -1081,8 +1070,7 @@
.isEmpty();
// Create dir/foo on the local disk and test that it invalidates the associated sky key.
- TreeFileArtifact fooArtifact =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeArtifact, "foo");
+ TreeFileArtifact fooArtifact = TreeFileArtifact.createTreeOutput(treeArtifact, "foo");
FileSystemUtils.writeContentAsLatin1(fooArtifact.getPath(), "new-foo-content");
assertThat(
new FilesystemValueChecker(
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
index 9b63767..316efdb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -32,7 +32,6 @@
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -191,7 +190,7 @@
// case of a generated directory, which we have test coverage for.
skyFunctions.put(Artifact.ARTIFACT, new ArtifactFakeFunction());
artifactFunction = new NonHermeticArtifactFakeFunction();
- skyFunctions.put(ActionLookupData.NAME, new ActionFakeFunction());
+ skyFunctions.put(SkyFunctions.ACTION_EXECUTION, new ActionFakeFunction());
skyFunctions.put(NONHERMETIC_ARTIFACT, artifactFunction);
progressReceiver = new RecordingEvaluationProgressReceiver();
@@ -214,20 +213,13 @@
}
private SpecialArtifact treeArtifact(String path) {
- SpecialArtifact treeArtifact =
- new SpecialArtifact(
- ArtifactRoot.asDerivedRoot(rootDirectory, "out"),
- PathFragment.create("out/" + path),
- ActionsTestUtil.NULL_ARTIFACT_OWNER,
- SpecialArtifactType.TREE);
- assertThat(treeArtifact.isTreeArtifact()).isTrue();
- return treeArtifact;
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
+ ArtifactRoot.asDerivedRoot(rootDirectory, "out"), PathFragment.create("out/" + path));
}
private void addNewTreeFileArtifact(SpecialArtifact parent, String relatedPath)
throws IOException {
- TreeFileArtifact treeFileArtifact =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(parent, relatedPath);
+ TreeFileArtifact treeFileArtifact = TreeFileArtifact.createTreeOutput(parent, relatedPath);
artifactFunction.addNewTreeFileArtifact(treeFileArtifact);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
index 1262cfe..20419ca 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.devtools.build.lib.actions.util.ActionCacheTestHelper.AMNESIAC_CACHE;
@@ -44,7 +45,6 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupData;
@@ -56,6 +56,7 @@
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Actions.GeneratingActions;
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.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.ArtifactResolver;
@@ -953,8 +954,8 @@
// We create two "configured targets" and two copies of the same artifact, each generated by
// an action from its respective configured target.
ActionLookupValue.ActionLookupKey lc1 = new InjectedActionLookupKey("lc1");
- Artifact.SpecialArtifact output1 =
- new Artifact.SpecialArtifact(
+ SpecialArtifact output1 =
+ new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, "out"),
execPath,
lc1,
@@ -964,8 +965,8 @@
new TreeArtifactAction(NestedSetBuilder.emptySet(Order.STABLE_ORDER), output1, children);
ConfiguredTargetValue ctValue1 = createConfiguredTargetValue(action1, lc1);
ActionLookupValue.ActionLookupKey lc2 = new InjectedActionLookupKey("lc2");
- Artifact.SpecialArtifact output2 =
- new Artifact.SpecialArtifact(
+ SpecialArtifact output2 =
+ new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, "out"),
execPath,
lc2,
@@ -1009,15 +1010,13 @@
@AutoCodec.VisibleForSerialization
static class TreeArtifactAction extends TestAction {
@SuppressWarnings("unused") // Only needed for serialization.
- private final Artifact.SpecialArtifact output;
+ private final SpecialArtifact output;
@SuppressWarnings("unused") // Only needed for serialization.
private final Iterable<PathFragment> children;
TreeArtifactAction(
- NestedSet<Artifact> inputs,
- Artifact.SpecialArtifact output,
- Iterable<PathFragment> children) {
+ NestedSet<Artifact> inputs, SpecialArtifact output, Iterable<PathFragment> children) {
super(() -> createDirectoryAndFiles(output, children), inputs, ImmutableSet.of(output));
Preconditions.checkState(output.isTreeArtifact(), output);
this.output = output;
@@ -1025,7 +1024,7 @@
}
private static void createDirectoryAndFiles(
- Artifact.SpecialArtifact output, Iterable<PathFragment> children) {
+ SpecialArtifact output, Iterable<PathFragment> children) {
Path directory = output.getPath();
try {
directory.createDirectoryAndParents();
@@ -1046,8 +1045,8 @@
// We create two "configured targets" and two copies of the same artifact, each generated by
// an action from its respective configured target.
ActionLookupValue.ActionLookupKey baseKey = new InjectedActionLookupKey("base");
- Artifact.SpecialArtifact baseOutput =
- new Artifact.SpecialArtifact(
+ SpecialArtifact baseOutput =
+ new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, "out"),
execPath,
baseKey,
@@ -1058,8 +1057,8 @@
ConfiguredTargetValue baseCt = createConfiguredTargetValue(action1, baseKey);
ActionLookupValue.ActionLookupKey shared1 = new InjectedActionLookupKey("shared1");
PathFragment execPath2 = PathFragment.create("out").getRelative("treesShared");
- Artifact.SpecialArtifact sharedOutput1 =
- new Artifact.SpecialArtifact(
+ SpecialArtifact sharedOutput1 =
+ new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, "out"),
execPath2,
shared1,
@@ -1068,8 +1067,8 @@
new DummyActionTemplate(baseOutput, sharedOutput1, ActionOwner.SYSTEM_ACTION_OWNER);
ConfiguredTargetValue shared1Ct = createConfiguredTargetValue(template1, shared1);
ActionLookupValue.ActionLookupKey shared2 = new InjectedActionLookupKey("shared2");
- Artifact.SpecialArtifact sharedOutput2 =
- new Artifact.SpecialArtifact(
+ SpecialArtifact sharedOutput2 =
+ new SpecialArtifact(
ArtifactRoot.asDerivedRoot(root, "out"),
execPath2,
shared2,
@@ -1107,15 +1106,13 @@
evaluate(ImmutableList.of(sharedOutput1, sharedOutput2));
}
- private static class DummyActionTemplate implements ActionTemplate<DummyAction> {
- private final Artifact.SpecialArtifact inputArtifact;
- private final Artifact.SpecialArtifact outputArtifact;
+ private static final class DummyActionTemplate implements ActionTemplate<DummyAction> {
+ private final SpecialArtifact inputArtifact;
+ private final SpecialArtifact outputArtifact;
private final ActionOwner actionOwner;
private DummyActionTemplate(
- Artifact.SpecialArtifact inputArtifact,
- Artifact.SpecialArtifact outputArtifact,
- ActionOwner actionOwner) {
+ SpecialArtifact inputArtifact, SpecialArtifact outputArtifact, ActionOwner actionOwner) {
this.inputArtifact = inputArtifact;
this.outputArtifact = outputArtifact;
this.actionOwner = actionOwner;
@@ -1127,17 +1124,17 @@
}
@Override
- public Iterable<DummyAction> generateActionForInputArtifacts(
- Iterable<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner) {
- return ImmutableList.copyOf(inputTreeFileArtifacts).stream()
+ public ImmutableList<DummyAction> generateActionsForInputArtifacts(
+ ImmutableSet<TreeFileArtifact> inputTreeFileArtifacts, ActionLookupKey artifactOwner) {
+ return inputTreeFileArtifacts.stream()
.map(
input -> {
- Artifact.TreeFileArtifact output =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+ TreeFileArtifact output =
+ TreeFileArtifact.createTemplateExpansionOutput(
outputArtifact, input.getParentRelativePath(), artifactOwner);
- return new DummyAction(NestedSetBuilder.create(Order.STABLE_ORDER, input), output);
+ return new DummyAction(input, output);
})
- .collect(ImmutableList.toImmutableList());
+ .collect(toImmutableList());
}
@Override
@@ -1149,12 +1146,12 @@
}
@Override
- public Artifact getInputTreeArtifact() {
+ public SpecialArtifact getInputTreeArtifact() {
return inputArtifact;
}
@Override
- public Artifact getOutputTreeArtifact() {
+ public SpecialArtifact getOutputTreeArtifact() {
return outputArtifact;
}
@@ -1189,11 +1186,6 @@
}
@Override
- public ImmutableSet<Artifact> getOutputs() {
- return ImmutableSet.of(outputArtifact);
- }
-
- @Override
public NestedSet<Artifact> getInputFilesForExtraAction(
ActionExecutionContext actionExecutionContext) {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
@@ -1205,16 +1197,6 @@
}
@Override
- public Artifact getPrimaryInput() {
- return inputArtifact;
- }
-
- @Override
- public Artifact getPrimaryOutput() {
- return outputArtifact;
- }
-
- @Override
public NestedSet<Artifact> getMandatoryInputs() {
return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index 40892d5..e3cf371 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -107,7 +107,6 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.common.options.OptionsParser;
-import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.OptionsProvider;
import java.io.IOException;
import java.io.PrintStream;
@@ -176,24 +175,22 @@
return action;
}
- protected Builder createBuilder(ActionCache actionCache) throws Exception {
+ protected BuilderWithResult createBuilder(ActionCache actionCache) throws Exception {
return createBuilder(actionCache, 1, /*keepGoing=*/ false);
}
- /**
- * Create a ParallelBuilder with a DatabaseDependencyChecker using the
- * specified ActionCache.
- */
- protected Builder createBuilder(
+ /** Create a ParallelBuilder with a DatabaseDependencyChecker using the specified ActionCache. */
+ protected BuilderWithResult createBuilder(
ActionCache actionCache, final int threadCount, final boolean keepGoing) throws Exception {
return createBuilder(actionCache, threadCount, keepGoing, null);
}
- protected Builder createBuilder(
+ protected BuilderWithResult createBuilder(
final ActionCache actionCache,
final int threadCount,
final boolean keepGoing,
- @Nullable EvaluationProgressReceiver evaluationProgressReceiver) throws Exception {
+ @Nullable EvaluationProgressReceiver evaluationProgressReceiver)
+ throws Exception {
AtomicReference<PathPackageLocator> pkgLocator =
new AtomicReference<>(
new PathPackageLocator(
@@ -282,7 +279,14 @@
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
PrecomputedValue.REMOTE_OUTPUTS_MODE.set(differencer, RemoteOutputsMode.ALL);
- return new Builder() {
+ return new BuilderWithResult() {
+ @Nullable EvaluationResult<SkyValue> latestResult = null;
+
+ @Override
+ public EvaluationResult<SkyValue> getLatestResult() {
+ return Preconditions.checkNotNull(latestResult);
+ }
+
private void setGeneratingActions() throws ActionConflictException {
if (evaluator.getExistingValue(ACTION_LOOKUP_KEY) == null) {
differencer.inject(
@@ -313,8 +317,8 @@
Range<Long> lastExecutionTimeRange,
TopLevelArtifactContext topLevelArtifactContext,
boolean trustRemoteArtifacts)
- throws BuildFailedException, AbruptExitException, InterruptedException,
- TestExecException {
+ throws BuildFailedException, InterruptedException, TestExecException {
+ latestResult = null;
skyframeActionExecutor.prepareForExecution(
reporter,
executor,
@@ -344,6 +348,7 @@
.setEventHander(reporter)
.build();
EvaluationResult<SkyValue> result = driver.evaluate(keys, evaluationContext);
+ this.latestResult = result;
if (result.hasError()) {
boolean hasCycles = false;
@@ -409,20 +414,21 @@
ArtifactRoot.asDerivedRoot(execRoot, "out"), execPath, ACTION_LOOKUP_KEY);
}
- /**
- * Creates and returns a new "amnesiac" builder based on the amnesiac cache.
- */
- protected Builder amnesiacBuilder() throws Exception {
+ /** Creates and returns a new "amnesiac" builder based on the amnesiac cache. */
+ protected BuilderWithResult amnesiacBuilder() throws Exception {
return createBuilder(AMNESIAC_CACHE);
}
- /**
- * Creates and returns a new caching builder based on the inMemoryCache.
- */
- protected Builder cachingBuilder() throws Exception {
+ /** Creates and returns a new caching builder based on the inMemoryCache. */
+ protected BuilderWithResult cachingBuilder() throws Exception {
return createBuilder(inMemoryCache);
}
+ /** {@link Builder} that saves its most recent {@link EvaluationResult}. */
+ protected interface BuilderWithResult extends Builder {
+ EvaluationResult<SkyValue> getLatestResult();
+ }
+
/**
* Creates a TestAction from 'inputs' to 'outputs', and a new button, such that executing the
* action causes the button to be pressed. The button is returned.
@@ -449,8 +455,7 @@
NestedSetBuilder.emptySet(Order.STABLE_ORDER);
protected void buildArtifacts(Builder builder, Artifact... artifacts)
- throws BuildFailedException, AbruptExitException, InterruptedException, TestExecException,
- OptionsParsingException {
+ throws BuildFailedException, AbruptExitException, InterruptedException, TestExecException {
buildArtifacts(builder, new DummyExecutor(fileSystem, rootDirectory), artifacts);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index 8350e0e..2552a36 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -13,27 +13,24 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertThrows;
-import com.google.common.base.Function;
import com.google.common.base.Preconditions;
-import com.google.common.collect.Collections2;
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.Lists;
import com.google.common.hash.Hashing;
-import com.google.common.util.concurrent.Runnables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
-import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionKeyContext;
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.ActionResult;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
@@ -42,6 +39,7 @@
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.BuildFailedException;
+import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
@@ -49,12 +47,11 @@
import com.google.devtools.build.lib.actions.util.TestAction;
import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate;
-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.events.Event;
+import com.google.devtools.build.lib.events.EventCollector;
import com.google.devtools.build.lib.events.EventKind;
-import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.testutil.TestUtils;
@@ -69,60 +66,23 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
-import java.nio.charset.Charset;
+import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
-import java.util.Iterator;
import java.util.List;
-import javax.annotation.Nullable;
-import org.junit.Before;
+import java.util.Set;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
/** Timestamp builder tests for TreeArtifacts. */
@RunWith(JUnit4.class)
-public class TreeArtifactBuildTest extends TimestampBuilderTestCase {
-
- // Common Artifacts, TreeFileArtifact, and Buttons. These aren't all used in all tests, but
- // they're used often enough that we can save ourselves a lot of copy-pasted code by creating them
- // in setUp().
-
- Artifact in;
-
- SpecialArtifact outOne;
- TreeFileArtifact outOneFileOne;
- TreeFileArtifact outOneFileTwo;
- Button buttonOne = new Button();
-
- SpecialArtifact outTwo;
- TreeFileArtifact outTwoFileOne;
- TreeFileArtifact outTwoFileTwo;
- Button buttonTwo = new Button();
-
- @Before
- public void setUp() throws Exception {
- in = createSourceArtifact("input");
- writeFile(in, "input_content");
-
- outOne = createTreeArtifact("outputOne");
- outOneFileOne =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outOne, "out_one_file_one");
- outOneFileTwo =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outOne, "out_one_file_two");
-
- outTwo = createTreeArtifact("outputTwo");
- outTwoFileOne =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outTwo, "out_one_file_one");
- outTwoFileTwo =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(outTwo, "out_one_file_two");
- }
+public final class TreeArtifactBuildTest extends TimestampBuilderTestCase {
@Test
- public void testCodec() throws Exception {
+ public void codec() throws Exception {
SpecialArtifact parent = createTreeArtifact("parent");
parent.setGeneratingActionKey(ActionLookupData.create(ACTION_LOOKUP_KEY, 0));
- new SerializationTester(parent, ActionInputHelper.treeFileArtifact(parent, "child"))
+ new SerializationTester(parent, TreeFileArtifact.createTreeOutput(parent, "child"))
.addDependency(FileSystem.class, scratch.getFileSystem())
.addDependency(
Root.RootCodecDependencies.class,
@@ -132,81 +92,74 @@
/** Simple smoke test. If this isn't passing, something is very wrong... */
@Test
- public void testTreeArtifactSimpleCase() throws Exception {
- TouchingTestAction action = new TouchingTestAction(outOneFileOne, outOneFileTwo);
+ public void treeArtifactSimpleCase() throws Exception {
+ SpecialArtifact parent = createTreeArtifact("parent");
+ TouchingTestAction action = new TouchingTestAction(parent, "out1", "out2");
registerAction(action);
- buildArtifact(action.getSoleOutput());
- assertThat(outOneFileOne.getPath().exists()).isTrue();
- assertThat(outOneFileTwo.getPath().exists()).isTrue();
- assertThat(outOneFileOne.getTreeRelativePathString()).isEqualTo("out_one_file_one");
- assertThat(outOneFileTwo.getTreeRelativePathString()).isEqualTo("out_one_file_two");
+ TreeArtifactValue result = buildArtifact(parent);
+
+ verifyOutputTree(result, parent, "out1", "out2");
}
/** Simple test for the case with dependencies. */
@Test
- public void testDependentTreeArtifacts() throws Exception {
- TouchingTestAction actionOne = new TouchingTestAction(outOneFileOne, outOneFileTwo);
- registerAction(actionOne);
+ public void dependentTreeArtifacts() throws Exception {
+ SpecialArtifact tree1 = createTreeArtifact("tree1");
+ TouchingTestAction action1 = new TouchingTestAction(tree1, "out1", "out2");
+ registerAction(action1);
- CopyTreeAction actionTwo =
- new CopyTreeAction(
- ImmutableList.of(outOneFileOne, outOneFileTwo),
- ImmutableList.of(outTwoFileOne, outTwoFileTwo));
- registerAction(actionTwo);
+ SpecialArtifact tree2 = createTreeArtifact("tree2");
+ CopyTreeAction action2 = new CopyTreeAction(tree1, tree2);
+ registerAction(action2);
- buildArtifact(outTwo);
+ TreeArtifactValue result = buildArtifact(tree2);
- assertThat(outOneFileOne.getPath().exists()).isTrue();
- assertThat(outOneFileTwo.getPath().exists()).isTrue();
- assertThat(outTwoFileOne.getPath().exists()).isTrue();
- assertThat(outTwoFileTwo.getPath().exists()).isTrue();
+ assertThat(tree1.getPath().getRelative("out1").exists()).isTrue();
+ assertThat(tree1.getPath().getRelative("out2").exists()).isTrue();
+ verifyOutputTree(result, tree2, "out1", "out2");
}
/** Test for tree artifacts with sub directories. */
@Test
- public void testTreeArtifactWithSubDirectory() throws Exception {
- SpecialArtifact treeOut = createTreeArtifact("output");
- TreeFileArtifact fileOne =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeOut, "sub_one/file_one");
- TreeFileArtifact fileTwo =
- ActionsTestUtil.createTreeFileArtifactWithNoGeneratingAction(treeOut, "sub_two/file_two");
- TouchingTestAction action = new TouchingTestAction(fileOne, fileTwo);
+ public void treeArtifactWithSubDirectory() throws Exception {
+ SpecialArtifact parent = createTreeArtifact("parent");
+ TouchingTestAction action = new TouchingTestAction(parent, "sub1/file1", "sub2/file2");
registerAction(action);
- buildArtifact(action.getSoleOutput());
+ TreeArtifactValue result = buildArtifact(parent);
- assertThat(fileOne.getPath().exists()).isTrue();
- assertThat(fileTwo.getPath().exists()).isTrue();
- assertThat(fileOne.getTreeRelativePathString()).isEqualTo("sub_one/file_one");
- assertThat(fileTwo.getTreeRelativePathString()).isEqualTo("sub_two/file_two");
+ verifyOutputTree(result, parent, "sub1/file1", "sub2/file2");
}
@Test
- public void testInputTreeArtifactMetadataProvider() throws Exception {
- TouchingTestAction actionOne = new TouchingTestAction(outOneFileOne, outOneFileTwo);
- registerAction(actionOne);
+ public void inputTreeArtifactMetadataProvider() throws Exception {
+ SpecialArtifact treeArtifactInput = createTreeArtifact("tree");
+ TouchingTestAction action1 = new TouchingTestAction(treeArtifactInput, "out1", "out2");
+ registerAction(action1);
- final Artifact normalOutput = createDerivedArtifact("normal/out");
+ Artifact normalOutput = createDerivedArtifact("normal/out");
Action testAction =
- new TestAction(
- TestAction.NO_EFFECT,
- NestedSetBuilder.create(Order.STABLE_ORDER, outOne),
- ImmutableSet.of(normalOutput)) {
+ new SimpleTestAction(ImmutableList.of(treeArtifactInput), normalOutput) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- // Check the metadata provider for input TreeFileArtifacts.
- MetadataProvider metadataProvider = actionExecutionContext.getMetadataProvider();
- assertThat(metadataProvider.getMetadata(outOneFileOne).getType().isFile()).isTrue();
- assertThat(metadataProvider.getMetadata(outOneFileTwo).getType().isFile()).isTrue();
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ // Check the metadata provider for input TreeFileArtifacts.
+ MetadataProvider metadataProvider = actionExecutionContext.getMetadataProvider();
+ assertThat(
+ metadataProvider
+ .getMetadata(TreeFileArtifact.createTreeOutput(treeArtifactInput, "out1"))
+ .getType()
+ .isFile())
+ .isTrue();
+ assertThat(
+ metadataProvider
+ .getMetadata(TreeFileArtifact.createTreeOutput(treeArtifactInput, "out2"))
+ .getType()
+ .isFile())
+ .isTrue();
- // Touch the action output.
- touchFile(normalOutput);
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ // Touch the action output.
+ touchFile(normalOutput);
}
};
@@ -216,217 +169,189 @@
/** Unchanged TreeArtifact outputs should not cause reexecution. */
@Test
- public void testCacheCheckingForTreeArtifactsDoesNotCauseReexecution() throws Exception {
- SpecialArtifact outOne = createTreeArtifact("outputOne");
- Button buttonOne = new Button();
+ public void cacheCheckingForTreeArtifactsDoesNotCauseReexecution() throws Exception {
+ SpecialArtifact out1 = createTreeArtifact("out1");
+ Button button1 = new Button();
- SpecialArtifact outTwo = createTreeArtifact("outputTwo");
- Button buttonTwo = new Button();
+ SpecialArtifact out2 = createTreeArtifact("out2");
+ Button button2 = new Button();
- TouchingTestAction actionOne =
- new TouchingTestAction(buttonOne, outOne, "file_one", "file_two");
- registerAction(actionOne);
+ TouchingTestAction action1 = new TouchingTestAction(button1, out1, "file_one", "file_two");
+ registerAction(action1);
- CopyTreeAction actionTwo =
- new CopyTreeAction(buttonTwo, outOne, outTwo, "file_one", "file_two");
- registerAction(actionTwo);
+ CopyTreeAction action2 = new CopyTreeAction(button2, out1, out2);
+ registerAction(action2);
- buttonOne.pressed = buttonTwo.pressed = false;
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isTrue(); // built
- assertThat(buttonTwo.pressed).isTrue(); // built
+ button1.pressed = false;
+ button2.pressed = false;
+ buildArtifact(out2);
+ assertThat(button1.pressed).isTrue(); // built
+ assertThat(button2.pressed).isTrue(); // built
- buttonOne.pressed = buttonTwo.pressed = false;
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isFalse(); // not built
- assertThat(buttonTwo.pressed).isFalse(); // not built
+ button1.pressed = false;
+ button2.pressed = false;
+ buildArtifact(out2);
+ assertThat(button1.pressed).isFalse(); // not built
+ assertThat(button2.pressed).isFalse(); // not built
}
/** Test rebuilding TreeArtifacts for inputs, outputs, and dependents. Also a test for caching. */
@Test
- public void testTransitiveReexecutionForTreeArtifacts() throws Exception {
- WriteInputToFilesAction actionOne =
- new WriteInputToFilesAction(buttonOne, in, outOneFileOne, outOneFileTwo);
- registerAction(actionOne);
+ public void transitiveReexecutionForTreeArtifacts() throws Exception {
+ Artifact in = createSourceArtifact("input");
+ writeFile(in, "input content");
- CopyTreeAction actionTwo =
- new CopyTreeAction(
- buttonTwo,
- ImmutableList.of(outOneFileOne, outOneFileTwo),
- ImmutableList.of(outTwoFileOne, outTwoFileTwo));
- registerAction(actionTwo);
+ Button button1 = new Button();
+ SpecialArtifact out1 = createTreeArtifact("output1");
+ WriteInputToFilesAction action1 =
+ new WriteInputToFilesAction(button1, in, out1, "file1", "file2");
+ registerAction(action1);
- buttonOne.pressed = buttonTwo.pressed = false;
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isTrue(); // built
- assertThat(buttonTwo.pressed).isTrue(); // built
+ Button button2 = new Button();
+ SpecialArtifact out2 = createTreeArtifact("output2");
+ CopyTreeAction action2 = new CopyTreeAction(button2, out1, out2);
+ registerAction(action2);
- buttonOne.pressed = buttonTwo.pressed = false;
- writeFile(in, "modified_input");
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isTrue(); // built
- assertThat(buttonTwo.pressed).isTrue(); // not built
+ button1.pressed = false;
+ button2.pressed = false;
+ buildArtifact(out2);
+ assertThat(button1.pressed).isTrue(); // built
+ assertThat(button2.pressed).isTrue(); // built
- buttonOne.pressed = buttonTwo.pressed = false;
- writeFile(outOneFileOne, "modified_output");
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isTrue(); // built
- assertThat(buttonTwo.pressed).isFalse(); // should have been cached
+ button1.pressed = false;
+ button2.pressed = false;
+ writeFile(in, "modified input");
+ buildArtifact(out2);
+ assertThat(button1.pressed).isTrue(); // built
+ assertThat(button2.pressed).isTrue(); // built
- buttonOne.pressed = buttonTwo.pressed = false;
- writeFile(outTwoFileOne, "more_modified_output");
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isFalse(); // not built
- assertThat(buttonTwo.pressed).isTrue(); // built
+ button1.pressed = false;
+ button2.pressed = false;
+ writeFile(TreeFileArtifact.createTreeOutput(out1, "file1"), "modified output");
+ buildArtifact(out2);
+ assertThat(button1.pressed).isTrue(); // built
+ assertThat(button2.pressed).isFalse(); // should have been cached
+
+ button1.pressed = false;
+ button2.pressed = false;
+ writeFile(TreeFileArtifact.createTreeOutput(out2, "file1"), "more modified output");
+ buildArtifact(out2);
+ assertThat(button1.pressed).isFalse(); // not built
+ assertThat(button2.pressed).isTrue(); // built
}
/** Tests that changing a TreeArtifact directory should cause reexeuction. */
@Test
- public void testDirectoryContentsCachingForTreeArtifacts() throws Exception {
- WriteInputToFilesAction actionOne =
- new WriteInputToFilesAction(buttonOne, in, outOneFileOne, outOneFileTwo);
- registerAction(actionOne);
+ public void directoryContentsCachingForTreeArtifacts() throws Exception {
+ Artifact in = createSourceArtifact("input");
+ writeFile(in, "input content");
- CopyTreeAction actionTwo =
- new CopyTreeAction(
- buttonTwo,
- ImmutableList.of(outOneFileOne, outOneFileTwo),
- ImmutableList.of(outTwoFileOne, outTwoFileTwo));
- registerAction(actionTwo);
+ Button button1 = new Button();
+ SpecialArtifact out1 = createTreeArtifact("output1");
+ WriteInputToFilesAction action1 =
+ new WriteInputToFilesAction(button1, in, out1, "file1", "file2");
+ registerAction(action1);
- buttonOne.pressed = buttonTwo.pressed = false;
- buildArtifact(outTwo);
+ Button button2 = new Button();
+ SpecialArtifact out2 = createTreeArtifact("output2");
+ CopyTreeAction action2 = new CopyTreeAction(button2, out1, out2);
+ registerAction(action2);
+
+ button1.pressed = false;
+ button2.pressed = false;
+ buildArtifact(out2);
// just a smoke test--if these aren't built we have bigger problems!
- assertThat(buttonOne.pressed).isTrue();
- assertThat(buttonTwo.pressed).isTrue();
+ assertThat(button1.pressed).isTrue();
+ assertThat(button2.pressed).isTrue();
// Adding a file to a directory should cause reexecution.
- buttonOne.pressed = buttonTwo.pressed = false;
- Path spuriousOutputOne = outOne.getPath().getRelative("spuriousOutput");
+ button1.pressed = false;
+ button2.pressed = false;
+ Path spuriousOutputOne = out1.getPath().getRelative("spuriousOutput");
touchFile(spuriousOutputOne);
- buildArtifact(outTwo);
+ buildArtifact(out2);
// Should re-execute, and delete spurious output
assertThat(spuriousOutputOne.exists()).isFalse();
- assertThat(buttonOne.pressed).isTrue();
- assertThat(buttonTwo.pressed).isFalse(); // should have been cached
+ assertThat(button1.pressed).isTrue();
+ assertThat(button2.pressed).isFalse(); // should have been cached
- buttonOne.pressed = buttonTwo.pressed = false;
- Path spuriousOutputTwo = outTwo.getPath().getRelative("anotherSpuriousOutput");
+ button1.pressed = false;
+ button2.pressed = false;
+ Path spuriousOutputTwo = out2.getPath().getRelative("anotherSpuriousOutput");
touchFile(spuriousOutputTwo);
- buildArtifact(outTwo);
+ buildArtifact(out2);
assertThat(spuriousOutputTwo.exists()).isFalse();
- assertThat(buttonOne.pressed).isFalse();
- assertThat(buttonTwo.pressed).isTrue();
+ assertThat(button1.pressed).isFalse();
+ assertThat(button2.pressed).isTrue();
// Deleting should cause reexecution.
- buttonOne.pressed = buttonTwo.pressed = false;
- deleteFile(outOneFileOne);
- buildArtifact(outTwo);
- assertThat(outOneFileOne.getPath().exists()).isTrue();
- assertThat(buttonOne.pressed).isTrue();
- assertThat(buttonTwo.pressed).isFalse(); // should have been cached
+ button1.pressed = false;
+ button2.pressed = false;
+ TreeFileArtifact out1File1 = TreeFileArtifact.createTreeOutput(out1, "file1");
+ deleteFile(out1File1);
+ buildArtifact(out2);
+ assertThat(out1File1.getPath().exists()).isTrue();
+ assertThat(button1.pressed).isTrue();
+ assertThat(button2.pressed).isFalse(); // should have been cached
- buttonOne.pressed = buttonTwo.pressed = false;
- deleteFile(outTwoFileOne);
- buildArtifact(outTwo);
- assertThat(outTwoFileOne.getPath().exists()).isTrue();
- assertThat(buttonOne.pressed).isFalse();
- assertThat(buttonTwo.pressed).isTrue();
+ button1.pressed = false;
+ button2.pressed = false;
+ TreeFileArtifact out2File1 = TreeFileArtifact.createTreeOutput(out2, "file1");
+ deleteFile(out2File1);
+ buildArtifact(out2);
+ assertThat(out2File1.getPath().exists()).isTrue();
+ assertThat(button1.pressed).isFalse();
+ assertThat(button2.pressed).isTrue();
}
/** TreeArtifacts don't care about mtime, even when the file is empty. */
@Test
- public void testMTimeForTreeArtifactsDoesNotMatter() throws Exception {
+ public void mTimeForTreeArtifactsDoesNotMatter() throws Exception {
// For this test, we only touch the input file.
Artifact in = createSourceArtifact("touchable_input");
touchFile(in);
- WriteInputToFilesAction actionOne =
- new WriteInputToFilesAction(buttonOne, in, outOneFileOne, outOneFileTwo);
- registerAction(actionOne);
+ Button button1 = new Button();
+ SpecialArtifact out1 = createTreeArtifact("output1");
+ WriteInputToFilesAction action1 =
+ new WriteInputToFilesAction(button1, in, out1, "file1", "file2");
+ registerAction(action1);
- CopyTreeAction actionTwo =
- new CopyTreeAction(
- buttonTwo,
- ImmutableList.of(outOneFileOne, outOneFileTwo),
- ImmutableList.of(outTwoFileOne, outTwoFileTwo));
- registerAction(actionTwo);
+ Button button2 = new Button();
+ SpecialArtifact out2 = createTreeArtifact("output2");
+ CopyTreeAction action2 = new CopyTreeAction(button2, out1, out2);
+ registerAction(action2);
- buttonOne.pressed = buttonTwo.pressed = false;
- buildArtifact(outTwo);
- assertThat(buttonOne.pressed).isTrue(); // built
- assertThat(buttonTwo.pressed).isTrue(); // built
+ button1.pressed = false;
+ button2.pressed = false;
+ buildArtifact(out2);
+ assertThat(button1.pressed).isTrue(); // built
+ assertThat(button2.pressed).isTrue(); // built
- buttonOne.pressed = buttonTwo.pressed = false;
+ button1.pressed = false;
+ button2.pressed = false;
touchFile(in);
- buildArtifact(outTwo);
+ buildArtifact(out2);
// mtime does not matter.
- assertThat(buttonOne.pressed).isFalse();
- assertThat(buttonTwo.pressed).isFalse();
+ assertThat(button1.pressed).isFalse();
+ assertThat(button2.pressed).isFalse();
// None of the below following should result in anything being built.
- buttonOne.pressed = buttonTwo.pressed = false;
- touchFile(outOneFileOne);
- buildArtifact(outTwo);
+ button1.pressed = false;
+ button2.pressed = false;
+ touchFile(TreeFileArtifact.createTreeOutput(out1, "file1"));
+ buildArtifact(out2);
// Nothing should be built.
- assertThat(buttonOne.pressed).isFalse();
- assertThat(buttonTwo.pressed).isFalse();
+ assertThat(button1.pressed).isFalse();
+ assertThat(button2.pressed).isFalse();
- buttonOne.pressed = buttonTwo.pressed = false;
- touchFile(outOneFileTwo);
- buildArtifact(outTwo);
+ button1.pressed = false;
+ button2.pressed = false;
+ touchFile(TreeFileArtifact.createTreeOutput(out1, "file2"));
+ buildArtifact(out2);
// Nothing should be built.
- assertThat(buttonOne.pressed).isFalse();
- assertThat(buttonTwo.pressed).isFalse();
- }
-
- /** Tests that the declared order of TreeArtifact contents does not matter. */
- @Test
- public void testOrderIndependenceOfTreeArtifactContents() throws Exception {
- WriteInputToFilesAction actionOne =
- new WriteInputToFilesAction(
- in,
- // The design of WritingTestAction is s.t.
- // these files will be registered in the given order.
- outOneFileTwo,
- outOneFileOne);
- registerAction(actionOne);
-
- CopyTreeAction actionTwo =
- new CopyTreeAction(
- ImmutableList.of(outOneFileOne, outOneFileTwo),
- ImmutableList.of(outTwoFileOne, outTwoFileTwo));
- registerAction(actionTwo);
-
- buildArtifact(outTwo);
- }
-
- @Test
- public void testActionExpansion() throws Exception {
- WriteInputToFilesAction action = new WriteInputToFilesAction(in, outOneFileOne, outOneFileTwo);
-
- CopyTreeAction actionTwo =
- new CopyTreeAction(
- ImmutableList.of(outOneFileOne, outOneFileTwo),
- ImmutableList.of(outTwoFileOne, outTwoFileTwo)) {
- @Override
- public void executeTestBehavior(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
- super.executeTestBehavior(actionExecutionContext);
-
- Collection<ActionInput> expanded =
- ActionInputHelper.expandArtifacts(
- NestedSetBuilder.create(Order.STABLE_ORDER, outOne),
- actionExecutionContext.getArtifactExpander());
- // Only files registered should show up here.
- assertThat(expanded).containsExactly(outOneFileOne, outOneFileTwo);
- }
- };
-
- registerAction(action);
- registerAction(actionTwo);
-
- buildArtifact(outTwo); // should not fail
+ assertThat(button1.pressed).isFalse();
+ assertThat(button2.pressed).isFalse();
}
private static void checkDirectoryPermissions(Path path) throws IOException {
@@ -444,27 +369,21 @@
}
@Test
- public void testOutputsAreReadOnlyAndExecutable() throws Exception {
- final SpecialArtifact out = createTreeArtifact("output");
+ public void outputsAreReadOnlyAndExecutable() throws Exception {
+ SpecialArtifact out = createTreeArtifact("output");
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- writeFile(out.getPath().getChild("three").getChild("four"), "three/four");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext context) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ writeFile(out.getPath().getChild("three").getChild("four"), "three/four");
}
};
registerAction(action);
-
- buildArtifact(action.getSoleOutput());
+ buildArtifact(out);
checkDirectoryPermissions(out.getPath());
checkFilePermissions(out.getPath().getChild("one"));
@@ -474,160 +393,127 @@
}
@Test
- public void testValidRelativeSymlinkAccepted() throws Exception {
- final SpecialArtifact out = createTreeArtifact("output");
+ public void validRelativeSymlinkAccepted() throws Exception {
+ SpecialArtifact out = createTreeArtifact("output");
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- FileSystemUtils.ensureSymbolicLink(
- out.getPath().getChild("links").getChild("link"), "../one");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"), "../one");
}
};
registerAction(action);
-
- buildArtifact(action.getSoleOutput());
+ buildArtifact(out);
}
@Test
- public void testInvalidSymlinkRejected() throws Exception {
+ public void invalidSymlinkRejected() {
// Failure expected
- StoredEventHandler storingEventHandler = new StoredEventHandler();
+ EventCollector eventCollector = new EventCollector(EventKind.ERROR);
reporter.removeHandler(failFastHandler);
- reporter.addHandler(storingEventHandler);
+ reporter.addHandler(eventCollector);
- final SpecialArtifact out = createTreeArtifact("output");
+ SpecialArtifact out = createTreeArtifact("output");
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- FileSystemUtils.ensureSymbolicLink(
- out.getPath().getChild("links").getChild("link"), "../invalid");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"), "../invalid");
}
};
registerAction(action);
+ assertThrows(BuildFailedException.class, () -> buildArtifact(out));
- assertThrows(BuildFailedException.class, () -> buildArtifact(action.getSoleOutput()));
- List<Event> errors =
- ImmutableList.copyOf(
- Iterables.filter(storingEventHandler.getEvents(), TreeArtifactBuildTest::isErrorEvent));
+ List<Event> errors = ImmutableList.copyOf(eventCollector);
assertThat(errors).hasSize(2);
assertThat(errors.get(0).getMessage()).contains("Failed to resolve relative path links/link");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
}
@Test
- public void testAbsoluteSymlinkBadTargetRejected() throws Exception {
+ public void absoluteSymlinkBadTargetRejected() {
// Failure expected
- StoredEventHandler storingEventHandler = new StoredEventHandler();
+ EventCollector eventCollector = new EventCollector(EventKind.ERROR);
reporter.removeHandler(failFastHandler);
- reporter.addHandler(storingEventHandler);
+ reporter.addHandler(eventCollector);
- final SpecialArtifact out = createTreeArtifact("output");
+ SpecialArtifact out = createTreeArtifact("output");
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- FileSystemUtils.ensureSymbolicLink(
- out.getPath().getChild("links").getChild("link"), "/random/pointer");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"), "/random/pointer");
}
};
registerAction(action);
+ assertThrows(BuildFailedException.class, () -> buildArtifact(out));
- assertThrows(BuildFailedException.class, () -> buildArtifact(action.getSoleOutput()));
- List<Event> errors =
- ImmutableList.copyOf(
- Iterables.filter(storingEventHandler.getEvents(), TreeArtifactBuildTest::isErrorEvent));
+ List<Event> errors = ImmutableList.copyOf(eventCollector);
assertThat(errors).hasSize(2);
assertThat(errors.get(0).getMessage()).contains("Failed to resolve relative path links/link");
assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
}
@Test
- public void testAbsoluteSymlinkAccepted() throws Exception {
+ public void absoluteSymlinkAccepted() throws Exception {
scratch.overwriteFile("/random/pointer");
- final SpecialArtifact out = createTreeArtifact("output");
+ SpecialArtifact out = createTreeArtifact("output");
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- FileSystemUtils.ensureSymbolicLink(
- out.getPath().getChild("links").getChild("link"), "/random/pointer");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"), "/random/pointer");
}
};
registerAction(action);
-
- buildArtifact(action.getSoleOutput());
+ buildArtifact(out);
}
@Test
- public void testRelativeSymlinkTraversingOutsideOfTreeArtifactRejected() throws Exception {
+ public void relativeSymlinkTraversingOutsideOfTreeArtifactRejected() {
// Failure expected
- StoredEventHandler storingEventHandler = new StoredEventHandler();
+ EventCollector eventCollector = new EventCollector(EventKind.ERROR);
reporter.removeHandler(failFastHandler);
- reporter.addHandler(storingEventHandler);
+ reporter.addHandler(eventCollector);
- final SpecialArtifact out = createTreeArtifact("output");
+ SpecialArtifact out = createTreeArtifact("output");
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- FileSystemUtils.ensureSymbolicLink(
- out.getPath().getChild("links").getChild("link"), "../../output/random/pointer");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"), "../../output/random/pointer");
}
};
registerAction(action);
- assertThrows(BuildFailedException.class, () -> buildArtifact(action.getSoleOutput()));
- List<Event> errors =
- ImmutableList.copyOf(
- Iterables.filter(storingEventHandler.getEvents(), TreeArtifactBuildTest::isErrorEvent));
+ assertThrows(BuildFailedException.class, () -> buildArtifact(out));
+ List<Event> errors = ImmutableList.copyOf(eventCollector);
assertThat(errors).hasSize(2);
assertThat(errors.get(0).getMessage())
.contains(
@@ -637,39 +523,32 @@
}
@Test
- public void testRelativeSymlinkTraversingToDirOutsideOfTreeArtifactRejected() throws Exception {
+ public void relativeSymlinkTraversingToDirOutsideOfTreeArtifactRejected() throws Exception {
// Failure expected
- StoredEventHandler storingEventHandler = new StoredEventHandler();
+ EventCollector eventCollector = new EventCollector(EventKind.ERROR);
reporter.removeHandler(failFastHandler);
- reporter.addHandler(storingEventHandler);
+ reporter.addHandler(eventCollector);
- final SpecialArtifact out = createTreeArtifact("output");
+ SpecialArtifact out = createTreeArtifact("output");
// Create a valid directory that can be referenced
scratch.dir(out.getRoot().getRoot().getRelative("some/dir").getPathString());
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(out) {
+ TestAction action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext) {
- try {
- writeFile(out.getPath().getChild("one"), "one");
- writeFile(out.getPath().getChild("two"), "two");
- FileSystemUtils.ensureSymbolicLink(
- out.getPath().getChild("links").getChild("link"), "../../some/dir");
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ writeFile(out.getPath().getChild("one"), "one");
+ writeFile(out.getPath().getChild("two"), "two");
+ FileSystemUtils.ensureSymbolicLink(
+ out.getPath().getChild("links").getChild("link"), "../../some/dir");
}
};
registerAction(action);
- assertThrows(BuildFailedException.class, () -> buildArtifact(action.getSoleOutput()));
- List<Event> errors =
- ImmutableList.copyOf(
- Iterables.filter(storingEventHandler.getEvents(), TreeArtifactBuildTest::isErrorEvent));
+ assertThrows(BuildFailedException.class, () -> buildArtifact(out));
+ List<Event> errors = ImmutableList.copyOf(eventCollector);
assertThat(errors).hasSize(2);
assertThat(errors.get(0).getMessage())
.contains(
@@ -686,52 +565,72 @@
// So all we're really testing here is that injectDigest() doesn't throw a weird exception.
// TODO(bazel-team): write real tests for injectDigest, here and elsewhere.
@Test
- public void testDigestInjection() throws Exception {
- TreeArtifactTestAction action =
- new TreeArtifactTestAction(outOne) {
+ public void digestInjection() throws Exception {
+ SpecialArtifact out = createTreeArtifact("output");
+ Action action =
+ new SimpleTestAction(out) {
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
- try {
- writeFile(outOneFileOne, "one");
- writeFile(outOneFileTwo, "two");
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ TreeFileArtifact child1 = TreeFileArtifact.createTreeOutput(out, "one");
+ TreeFileArtifact child2 = TreeFileArtifact.createTreeOutput(out, "two");
+ writeFile(child1, "one");
+ writeFile(child2, "two");
- MetadataHandler md = actionExecutionContext.getMetadataHandler();
- FileStatus stat = outOneFileOne.getPath().stat(Symlinks.NOFOLLOW);
- md.injectDigest(
- outOneFileOne,
- stat,
- Hashing.md5().hashString("one", Charset.forName("UTF-8")).asBytes());
+ MetadataHandler md = actionExecutionContext.getMetadataHandler();
+ FileStatus stat = child1.getPath().stat(Symlinks.NOFOLLOW);
+ md.injectDigest(child1, stat, Hashing.sha256().hashString("one", UTF_8).asBytes());
- stat = outOneFileTwo.getPath().stat(Symlinks.NOFOLLOW);
- md.injectDigest(
- outOneFileTwo,
- stat,
- Hashing.md5().hashString("two", Charset.forName("UTF-8")).asBytes());
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
- return ActionResult.EMPTY;
+ stat = child2.getPath().stat(Symlinks.NOFOLLOW);
+ md.injectDigest(child2, stat, Hashing.sha256().hashString("two", UTF_8).asBytes());
}
};
registerAction(action);
- buildArtifact(action.getSoleOutput());
+ buildArtifact(out);
}
@Test
- public void testExpandedActionsBuildInActionTemplate() throws Throwable {
+ public void remoteDirectoryInjection() throws Exception {
+ SpecialArtifact out = createTreeArtifact("output");
+ RemoteFileArtifactValue remoteFile1 =
+ new RemoteFileArtifactValue(
+ Hashing.sha256().hashString("one", UTF_8).asBytes(), /*size=*/ 3, /*locationIndex=*/ 1);
+ RemoteFileArtifactValue remoteFile2 =
+ new RemoteFileArtifactValue(
+ Hashing.sha256().hashString("two", UTF_8).asBytes(), /*size=*/ 3, /*locationIndex=*/ 2);
+
+ Action action =
+ new SimpleTestAction(out) {
+ @Override
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ TreeFileArtifact child1 = TreeFileArtifact.createTreeOutput(out, "one");
+ TreeFileArtifact child2 = TreeFileArtifact.createTreeOutput(out, "two");
+ writeFile(child1, "one");
+ writeFile(child2, "two");
+
+ actionExecutionContext
+ .getMetadataHandler()
+ .injectRemoteDirectory(
+ out, ImmutableMap.of(child1, remoteFile1, child2, remoteFile2));
+ }
+ };
+
+ registerAction(action);
+ TreeArtifactValue result = buildArtifact(out);
+
+ assertThat(result.getChildValues())
+ .containsExactly(
+ TreeFileArtifact.createTreeOutput(out, "one"),
+ remoteFile1,
+ TreeFileArtifact.createTreeOutput(out, "two"),
+ remoteFile2);
+ }
+
+ @Test
+ public void expandedActionsBuildInActionTemplate() throws Exception {
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
- TreeFileArtifact treeFileArtifactA =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child1"), ACTION_LOOKUP_KEY);
- TreeFileArtifact treeFileArtifactB =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child2"), ACTION_LOOKUP_KEY);
- registerAction(new TouchingTestAction(treeFileArtifactA, treeFileArtifactB));
- treeFileArtifactA.setGeneratingActionKey(artifact1.getGeneratingActionKey());
- treeFileArtifactB.setGeneratingActionKey(artifact1.getGeneratingActionKey());
+ registerAction(new TouchingTestAction(artifact1, "file1", "file2"));
// artifact2 is a tree artifact generated by an action template.
SpecialArtifact artifact2 = createTreeArtifact("treeArtifact2");
@@ -741,45 +640,36 @@
// We mock out the action template function to expand into two actions that just touch the
// output files.
- ActionTemplateExpansionKey secondOwner = ActionTemplateExpansionKey.of(ACTION_LOOKUP_KEY, 1);
- TreeFileArtifact expectedOutputTreeFileArtifact1 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2, PathFragment.createAlreadyNormalized("child1"), secondOwner);
- TreeFileArtifact expectedOutputTreeFileArtifact2 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2, PathFragment.createAlreadyNormalized("child2"), secondOwner);
- Action generateOutputAction =
+ ActionTemplateExpansionKey secondOwner = ActionTemplateExpansionValue.key(ACTION_LOOKUP_KEY, 1);
+ TreeFileArtifact expectedExpansionOutput1 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child1", secondOwner);
+ TreeFileArtifact expectedExpansionOutput2 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child2", secondOwner);
+ Action expandedAction1 =
new DummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactA),
- expectedOutputTreeFileArtifact1);
- Action noGenerateOutputAction =
+ TreeFileArtifact.createTreeOutput(artifact1, "file1"), expectedExpansionOutput1);
+ Action expandedAction2 =
new DummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactB),
- expectedOutputTreeFileArtifact2);
+ TreeFileArtifact.createTreeOutput(artifact1, "file2"), expectedExpansionOutput2);
actionTemplateExpansionFunction =
new DummyActionTemplateExpansionFunction(
- actionKeyContext, ImmutableList.of(generateOutputAction, noGenerateOutputAction));
+ actionKeyContext, ImmutableList.of(expandedAction1, expandedAction2));
- buildArtifact(artifact2);
+ TreeArtifactValue result = buildArtifact(artifact2);
+
+ assertThat(result.getChildren())
+ .containsExactly(expectedExpansionOutput1, expectedExpansionOutput2);
}
@Test
- public void testExpandedActionDoesNotGenerateOutputInActionTemplate() throws Throwable {
+ public void expandedActionDoesNotGenerateOutputInActionTemplate() {
// expect errors
reporter.removeHandler(failFastHandler);
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
- TreeFileArtifact treeFileArtifactA =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child1"), ACTION_LOOKUP_KEY);
- TreeFileArtifact treeFileArtifactB =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child2"), ACTION_LOOKUP_KEY);
- registerAction(new TouchingTestAction(treeFileArtifactA, treeFileArtifactB));
- treeFileArtifactA.setGeneratingActionKey(artifact1.getGeneratingActionKey());
- treeFileArtifactB.setGeneratingActionKey(artifact1.getGeneratingActionKey());
+ registerAction(new TouchingTestAction(artifact1, "child1", "child2"));
// artifact2 is a tree artifact generated by an action template.
SpecialArtifact artifact2 = createTreeArtifact("treeArtifact2");
@@ -791,20 +681,16 @@
// One Action that touches the output file.
// The other action that does not generate the output file.
ActionTemplateExpansionKey secondOwner = ActionTemplateExpansionKey.of(ACTION_LOOKUP_KEY, 1);
- TreeFileArtifact expectedOutputTreeFileArtifact1 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2, PathFragment.createAlreadyNormalized("child1"), secondOwner);
- TreeFileArtifact expectedOutputTreeFileArtifact2 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2, PathFragment.createAlreadyNormalized("child2"), secondOwner);
+ TreeFileArtifact expectedExpansionOutput1 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child1", secondOwner);
+ TreeFileArtifact expectedExpansionOutput2 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child2", secondOwner);
Action generateOutputAction =
new DummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactA),
- expectedOutputTreeFileArtifact1);
+ TreeFileArtifact.createTreeOutput(artifact1, "child1"), expectedExpansionOutput1);
Action noGenerateOutputAction =
new NoOpDummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactB),
- ImmutableSet.of(expectedOutputTreeFileArtifact2));
+ TreeFileArtifact.createTreeOutput(artifact1, "child2"), expectedExpansionOutput2);
actionTemplateExpansionFunction =
new DummyActionTemplateExpansionFunction(
@@ -816,21 +702,13 @@
}
@Test
- public void testOneExpandedActionThrowsInActionTemplate() throws Throwable {
+ public void oneExpandedActionThrowsInActionTemplate() {
// expect errors
reporter.removeHandler(failFastHandler);
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
- TreeFileArtifact treeFileArtifactA =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child1"), ACTION_LOOKUP_KEY);
- TreeFileArtifact treeFileArtifactB =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child2"), ACTION_LOOKUP_KEY);
- registerAction(new TouchingTestAction(treeFileArtifactA, treeFileArtifactB));
- treeFileArtifactA.setGeneratingActionKey(artifact1.getGeneratingActionKey());
- treeFileArtifactB.setGeneratingActionKey(artifact1.getGeneratingActionKey());
+ registerAction(new TouchingTestAction(artifact1, "child1", "child2"));
// artifact2 is a tree artifact generated by an action template.
SpecialArtifact artifact2 = createTreeArtifact("treeArtifact2");
@@ -841,24 +719,17 @@
// We mock out the action template function to expand into two actions:
// One Action that touches the output file.
// The other action that just throws when executed.
- TreeFileArtifact expectedOutputTreeFileArtifact1 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2,
- PathFragment.createAlreadyNormalized("child1"),
- ActionTemplateExpansionKey.of(artifact1.getArtifactOwner(), 1));
- TreeFileArtifact expectedOutputTreeFileArtifact2 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2,
- PathFragment.createAlreadyNormalized("child2"),
- ActionTemplateExpansionKey.of(artifact1.getArtifactOwner(), 1));
+ ActionTemplateExpansionKey secondOwner = ActionTemplateExpansionKey.of(ACTION_LOOKUP_KEY, 1);
+ TreeFileArtifact expectedExpansionOutput1 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child1", secondOwner);
+ TreeFileArtifact expectedExpansionOutput2 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child2", secondOwner);
Action generateOutputAction =
new DummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactA),
- expectedOutputTreeFileArtifact1);
+ TreeFileArtifact.createTreeOutput(artifact1, "child1"), expectedExpansionOutput1);
Action throwingAction =
new ThrowingDummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactB),
- ImmutableSet.of(expectedOutputTreeFileArtifact2));
+ TreeFileArtifact.createTreeOutput(artifact1, "child2"), expectedExpansionOutput2);
actionTemplateExpansionFunction =
new DummyActionTemplateExpansionFunction(
@@ -870,21 +741,13 @@
}
@Test
- public void testAllExpandedActionsThrowInActionTemplate() throws Throwable {
+ public void allExpandedActionsThrowInActionTemplate() {
// expect errors
reporter.removeHandler(failFastHandler);
// artifact1 is a tree artifact generated by a TouchingTestAction.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
- TreeFileArtifact treeFileArtifactA =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child1"), ACTION_LOOKUP_KEY);
- TreeFileArtifact treeFileArtifactB =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact1, PathFragment.create("child2"), ACTION_LOOKUP_KEY);
- registerAction(new TouchingTestAction(treeFileArtifactA, treeFileArtifactB));
- treeFileArtifactA.setGeneratingActionKey(artifact1.getGeneratingActionKey());
- treeFileArtifactB.setGeneratingActionKey(artifact1.getGeneratingActionKey());
+ registerAction(new TouchingTestAction(artifact1, "child1", "child2"));
// artifact2 is a tree artifact generated by an action template.
SpecialArtifact artifact2 = createTreeArtifact("treeArtifact2");
@@ -894,20 +757,16 @@
// We mock out the action template function to expand into two actions that throw when executed.
ActionTemplateExpansionKey secondOwner = ActionTemplateExpansionKey.of(ACTION_LOOKUP_KEY, 1);
- TreeFileArtifact expectedOutputTreeFileArtifact1 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2, PathFragment.createAlreadyNormalized("child1"), secondOwner);
- TreeFileArtifact expectedOutputTreeFileArtifact2 =
- ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- artifact2, PathFragment.createAlreadyNormalized("child2"), secondOwner);
+ TreeFileArtifact expectedExpansionOutput1 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child1", secondOwner);
+ TreeFileArtifact expectedExpansionOutput2 =
+ TreeFileArtifact.createTemplateExpansionOutput(artifact2, "child2", secondOwner);
Action throwingAction =
new ThrowingDummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactA),
- ImmutableSet.of(expectedOutputTreeFileArtifact1));
+ TreeFileArtifact.createTreeOutput(artifact1, "child1"), expectedExpansionOutput1);
Action anotherThrowingAction =
new ThrowingDummyAction(
- NestedSetBuilder.create(Order.STABLE_ORDER, treeFileArtifactB),
- ImmutableSet.of(expectedOutputTreeFileArtifact2));
+ TreeFileArtifact.createTreeOutput(artifact1, "child2"), expectedExpansionOutput2);
actionTemplateExpansionFunction =
new DummyActionTemplateExpansionFunction(
@@ -919,15 +778,13 @@
}
@Test
- public void testInputTreeArtifactCreationFailedInActionTemplate() throws Throwable {
+ public void inputTreeArtifactCreationFailedInActionTemplate() {
// expect errors
reporter.removeHandler(failFastHandler);
// artifact1 is created by a action that throws.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
- registerAction(
- new ThrowingDummyAction(
- NestedSetBuilder.emptySet(Order.STABLE_ORDER), ImmutableSet.of(artifact1)));
+ registerAction(new ThrowingDummyAction(artifact1));
// artifact2 is a tree artifact generated by an action template.
SpecialArtifact artifact2 = createTreeArtifact("treeArtifact2");
@@ -941,12 +798,10 @@
}
@Test
- public void testEmptyInputAndOutputTreeArtifactInActionTemplate() throws Throwable {
+ public void emptyInputAndOutputTreeArtifactInActionTemplate() throws Exception {
// artifact1 is an empty tree artifact which is generated by a single no-op dummy action.
SpecialArtifact artifact1 = createTreeArtifact("treeArtifact1");
- registerAction(
- new NoOpDummyAction(
- NestedSetBuilder.emptySet(Order.STABLE_ORDER), ImmutableSet.of(artifact1)));
+ registerAction(new NoOpDummyAction(artifact1));
// artifact2 is a tree artifact generated by an action template that takes artifact1 as input.
SpecialArtifact artifact2 = createTreeArtifact("treeArtifact2");
@@ -962,248 +817,98 @@
assertThat(artifact2.getPath().getDirectoryEntries()).isEmpty();
}
- private static boolean isErrorEvent(Event event) {
- return event.getKind().equals(EventKind.ERROR);
- }
+ private abstract static class SimpleTestAction extends TestAction {
+ private final Button button;
- /**
- * A generic test action that takes at most one input TreeArtifact, exactly one output
- * TreeArtifact, and some path fragment inputs/outputs.
- */
- private abstract static class TreeArtifactTestAction extends TestAction {
- final Iterable<TreeFileArtifact> inputFiles;
- final Iterable<TreeFileArtifact> outputFiles;
-
- TreeArtifactTestAction(final SpecialArtifact output, final String... subOutputs) {
- this(
- Runnables.doNothing(),
- null,
- ImmutableList.<TreeFileArtifact>of(),
- output,
- Collections2.transform(
- Arrays.asList(subOutputs),
- new Function<String, TreeFileArtifact>() {
- @Nullable
- @Override
- public TreeFileArtifact apply(String s) {
- return ActionInputHelper.treeFileArtifact(output, s);
- }
- }));
+ SimpleTestAction(Artifact output) {
+ this(/*inputs=*/ ImmutableList.of(), output);
}
- TreeArtifactTestAction(Runnable effect, TreeFileArtifact... outputFiles) {
- this(effect, Arrays.asList(outputFiles));
+ SimpleTestAction(Iterable<Artifact> inputs, Artifact output) {
+ this(new Button(), inputs, output);
}
- TreeArtifactTestAction(Runnable effect, Collection<TreeFileArtifact> outputFiles) {
- this(
- effect,
- null,
- ImmutableList.<TreeFileArtifact>of(),
- outputFiles.iterator().next().getParent(),
- outputFiles);
- }
-
- TreeArtifactTestAction(
- Runnable effect, Artifact inputFile, Collection<TreeFileArtifact> outputFiles) {
- this(
- effect,
- inputFile,
- ImmutableList.<TreeFileArtifact>of(),
- outputFiles.iterator().next().getParent(),
- outputFiles);
- }
-
- TreeArtifactTestAction(
- Runnable effect,
- Collection<TreeFileArtifact> inputFiles,
- Collection<TreeFileArtifact> outputFiles) {
- this(
- effect,
- inputFiles.iterator().next().getParent(),
- inputFiles,
- outputFiles.iterator().next().getParent(),
- outputFiles);
- }
-
- TreeArtifactTestAction(
- Runnable effect,
- @Nullable Artifact input,
- Collection<TreeFileArtifact> inputFiles,
- Artifact output,
- Collection<TreeFileArtifact> outputFiles) {
- super(
- effect,
- input == null
- ? NestedSetBuilder.emptySet(Order.STABLE_ORDER)
- : NestedSetBuilder.create(Order.STABLE_ORDER, input),
- ImmutableSet.of(output));
- Preconditions.checkArgument(
- inputFiles.isEmpty() || (input != null && input.isTreeArtifact()));
- Preconditions.checkArgument(output == null || output.isTreeArtifact());
- this.inputFiles = ImmutableList.copyOf(inputFiles);
- this.outputFiles = ImmutableList.copyOf(outputFiles);
- for (TreeFileArtifact inputFile : inputFiles) {
- Preconditions.checkState(inputFile.getParent().equals(input));
- }
- for (TreeFileArtifact outputFile : outputFiles) {
- Preconditions.checkState(outputFile.getParent().equals(output));
- }
+ SimpleTestAction(Button button, Iterable<Artifact> inputs, Artifact output) {
+ super(NO_EFFECT, NestedSetBuilder.wrap(Order.STABLE_ORDER, inputs), ImmutableSet.of(output));
+ this.button = button;
}
@Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext)
+ public final ActionResult execute(ActionExecutionContext context)
throws ActionExecutionException {
- if (!getInputs().isEmpty()) {
- // Sanity check--verify all inputs exist.
- Artifact input = getInputs().getSingleton();
- if (!input.getPath().exists()) {
- throw new IllegalStateException(
- "action's input Artifact does not exist: " + input.getPath());
- }
- for (Artifact inputFile : inputFiles) {
- if (!inputFile.getPath().exists()) {
- throw new IllegalStateException("action's input does not exist: " + inputFile);
- }
- }
- }
-
- Artifact output = getSoleOutput();
- assertThat(output.getPath().exists()).isTrue();
+ button.pressed = true;
try {
- effect.call();
- executeTestBehavior(actionExecutionContext);
- } catch (RuntimeException e) {
- throw new RuntimeException(e);
- } catch (Exception e) {
- throw new ActionExecutionException(
- "TestAction failed due to exception: " + e.getMessage(), e, this, false);
+ run(context);
+ } catch (IOException e) {
+ throw new ActionExecutionException(e, this, /*catastrophe=*/ false);
}
return ActionResult.EMPTY;
}
- void executeTestBehavior(ActionExecutionContext c) throws ActionExecutionException {
- // Default: do nothing
- }
-
- /** Checks there's exactly one output, and returns it. */
- SpecialArtifact getSoleOutput() {
- Iterator<Artifact> it = getOutputs().iterator();
- SpecialArtifact r = (SpecialArtifact) it.next();
- Preconditions.checkNotNull(r);
- Preconditions.checkState(!it.hasNext());
- Preconditions.checkState(r.equals(getPrimaryOutput()));
- return r;
- }
-
- static List<TreeFileArtifact> asTreeFileArtifacts(
- final SpecialArtifact parent, String... files) {
- return Lists.transform(
- Arrays.asList(files),
- new Function<String, TreeFileArtifact>() {
- @Nullable
- @Override
- public TreeFileArtifact apply(String s) {
- return ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
- parent, PathFragment.create(s), parent.getArtifactOwner());
- }
- });
- }
+ abstract void run(ActionExecutionContext context) throws IOException;
}
/** An action that touches some output TreeFileArtifacts. Takes no inputs. */
- private static class TouchingTestAction extends TreeArtifactTestAction {
- TouchingTestAction(TreeFileArtifact... outputPaths) {
- super(Runnables.doNothing(), outputPaths);
+ private static final class TouchingTestAction extends SimpleTestAction {
+ private final ImmutableList<String> outputFiles;
+
+ TouchingTestAction(SpecialArtifact output, String... outputFiles) {
+ this(new Button(), output, outputFiles);
}
- TouchingTestAction(Runnable effect, SpecialArtifact output, String... outputPaths) {
- super(effect, asTreeFileArtifacts(output, outputPaths));
+ TouchingTestAction(Button button, SpecialArtifact output, String... outputFiles) {
+ super(button, /*inputs=*/ ImmutableList.of(), output);
+ this.outputFiles = ImmutableList.copyOf(outputFiles);
}
@Override
- public void executeTestBehavior(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
- try {
- for (Artifact file : outputFiles) {
- touchFile(file);
- }
- } catch (IOException e) {
- throw new RuntimeException(e);
+ void run(ActionExecutionContext context) throws IOException {
+ for (String file : outputFiles) {
+ touchFile(getPrimaryOutput().getPath().getRelative(file));
}
}
}
/** Takes an input file and populates several copies inside a TreeArtifact. */
- private static class WriteInputToFilesAction extends TreeArtifactTestAction {
- WriteInputToFilesAction(Artifact input, TreeFileArtifact... outputs) {
- this(Runnables.doNothing(), input, outputs);
- }
+ private static final class WriteInputToFilesAction extends SimpleTestAction {
+ private final ImmutableList<String> outputFiles;
- WriteInputToFilesAction(Runnable effect, Artifact input, TreeFileArtifact... outputs) {
- super(effect, input, Arrays.asList(outputs));
- Preconditions.checkArgument(!input.isTreeArtifact());
+ WriteInputToFilesAction(
+ Button button, Artifact input, SpecialArtifact output, String... outputFiles) {
+ super(button, ImmutableList.of(input), output);
+ this.outputFiles = ImmutableList.copyOf(outputFiles);
}
@Override
- public void executeTestBehavior(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
- try {
- for (Artifact file : outputFiles) {
- FileSystemUtils.createDirectoryAndParents(file.getPath().getParentDirectory());
- FileSystemUtils.copyFile(getInputs().getSingleton().getPath(), file.getPath());
- }
- } catch (IOException e) {
- throw new RuntimeException(e);
+ void run(ActionExecutionContext actionExecutionContext) throws IOException {
+ for (String file : outputFiles) {
+ Path newOutput = getPrimaryOutput().getPath().getRelative(file);
+ newOutput.createDirectoryAndParents();
+ FileSystemUtils.copyFile(getPrimaryInput().getPath(), newOutput);
}
}
}
/** Copies the given TreeFileArtifact inputs to the given outputs, in respective order. */
- private static class CopyTreeAction extends TreeArtifactTestAction {
+ private static final class CopyTreeAction extends SimpleTestAction {
- CopyTreeAction(
- Runnable effect, SpecialArtifact input, SpecialArtifact output, String... sourcesAndDests) {
- super(
- effect,
- input,
- asTreeFileArtifacts(input, sourcesAndDests),
- output,
- asTreeFileArtifacts(output, sourcesAndDests));
+ CopyTreeAction(SpecialArtifact input, SpecialArtifact output) {
+ this(new Button(), input, output);
}
- CopyTreeAction(
- Collection<TreeFileArtifact> inputPaths, Collection<TreeFileArtifact> outputPaths) {
- super(Runnables.doNothing(), inputPaths, outputPaths);
- }
-
- CopyTreeAction(
- Runnable effect,
- Collection<TreeFileArtifact> inputPaths,
- Collection<TreeFileArtifact> outputPaths) {
- super(effect, inputPaths, outputPaths);
+ CopyTreeAction(Button button, SpecialArtifact input, SpecialArtifact output) {
+ super(button, ImmutableList.of(input), output);
}
@Override
- public void executeTestBehavior(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
- Iterator<TreeFileArtifact> inputIterator = inputFiles.iterator();
- Iterator<TreeFileArtifact> outputIterator = outputFiles.iterator();
-
- try {
- while (inputIterator.hasNext() || outputIterator.hasNext()) {
- Artifact input = inputIterator.next();
- Artifact output = outputIterator.next();
- FileSystemUtils.createDirectoryAndParents(output.getPath().getParentDirectory());
- FileSystemUtils.copyFile(input.getPath(), output.getPath());
- }
- } catch (IOException e) {
- throw new RuntimeException(e);
+ void run(ActionExecutionContext context) throws IOException {
+ List<Artifact> children = new ArrayList<>();
+ context.getArtifactExpander().expand(getPrimaryInput(), children);
+ for (Artifact child : children) {
+ Path newOutput = getPrimaryOutput().getPath().getRelative(child.getParentRelativePath());
+ newOutput.createDirectoryAndParents();
+ FileSystemUtils.copyFile(child.getPath(), newOutput);
}
-
- // both iterators must be of the same size
- assertThat(inputIterator.hasNext()).isFalse();
- assertThat(inputIterator.hasNext()).isFalse();
}
}
@@ -1219,12 +924,32 @@
SpecialArtifactType.TREE);
}
- private void buildArtifact(Artifact artifact) throws Exception {
- buildArtifacts(cachingBuilder(), artifact);
+ private TreeArtifactValue buildArtifact(SpecialArtifact treeArtifact) throws Exception {
+ Preconditions.checkArgument(treeArtifact.isTreeArtifact(), treeArtifact);
+ BuilderWithResult builder = cachingBuilder();
+ buildArtifacts(builder, treeArtifact);
+ return (TreeArtifactValue) builder.getLatestResult().get(treeArtifact);
+ }
+
+ private void buildArtifact(Artifact normalArtifact) throws Exception {
+ buildArtifacts(cachingBuilder(), normalArtifact);
+ }
+
+ private static void verifyOutputTree(
+ TreeArtifactValue result, SpecialArtifact parent, String... expectedChildPaths) {
+ Preconditions.checkArgument(parent.isTreeArtifact(), parent);
+ Set<TreeFileArtifact> expectedChildren =
+ Arrays.stream(expectedChildPaths)
+ .map(path -> TreeFileArtifact.createTreeOutput(parent, path))
+ .collect(toImmutableSet());
+ for (TreeFileArtifact child : expectedChildren) {
+ assertWithMessage(child + " does not exist").that(child.getPath().exists()).isTrue();
+ }
+ assertThat(result.getChildren()).isEqualTo(expectedChildren);
}
private static void writeFile(Path path, String contents) throws IOException {
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
// sometimes we write read-only files
if (path.exists()) {
path.setWritable(true);
@@ -1237,7 +962,7 @@
}
private static void touchFile(Path path) throws IOException {
- FileSystemUtils.createDirectoryAndParents(path.getParentDirectory());
+ path.getParentDirectory().createDirectoryAndParents();
path.getParentDirectory().setWritable(true);
FileSystemUtils.touchFile(path);
}
@@ -1249,16 +974,14 @@
private static void deleteFile(Artifact file) throws IOException {
Path path = file.getPath();
// sometimes we write read-only files
- if (path.exists()) {
- path.setWritable(true);
- // work around the sticky bit (this might depend on the behavior of the OS?)
- path.getParentDirectory().setWritable(true);
- path.delete();
- }
+ path.setWritable(true);
+ // work around the sticky bit (this might depend on the behavior of the OS?)
+ path.getParentDirectory().setWritable(true);
+ path.delete();
}
- /** A dummy action template expansion function that just returns the injected actions */
- private static class DummyActionTemplateExpansionFunction implements SkyFunction {
+ /** A dummy action template expansion function that just returns the injected actions. */
+ private static final class DummyActionTemplateExpansionFunction implements SkyFunction {
private final ActionKeyContext actionKeyContext;
private final ImmutableList<ActionAnalysisMetadata> actions;
@@ -1273,10 +996,7 @@
try {
return new ActionTemplateExpansionValue(
Actions.assignOwnersAndFilterSharedActionsAndThrowActionConflict(
- actionKeyContext,
- actions,
- (ActionLookupValue.ActionLookupKey) skyKey,
- /*outputFiles=*/ null));
+ actionKeyContext, actions, (ActionLookupKey) skyKey, /*outputFiles=*/ null));
} catch (ActionConflictException e) {
throw new IllegalStateException(e);
}
@@ -1289,30 +1009,37 @@
}
/** No-op action that does not generate the action outputs. */
- private static class NoOpDummyAction extends TestAction {
- public NoOpDummyAction(NestedSet<Artifact> inputs, ImmutableSet<Artifact> outputs) {
- super(NO_EFFECT, inputs, outputs);
+ private static final class NoOpDummyAction extends SimpleTestAction {
+
+ NoOpDummyAction(Artifact output) {
+ super(/*inputs=*/ ImmutableList.of(), output);
}
- /** Do nothing */
- @Override
- public ActionResult execute(ActionExecutionContext actionExecutionContext)
- throws ActionExecutionException {
- return ActionResult.EMPTY;
+ NoOpDummyAction(Artifact input, Artifact output) {
+ super(ImmutableList.of(input), output);
}
+
+ /** Does nothing. */
+ @Override
+ void run(ActionExecutionContext actionExecutionContext) {}
}
- /** No-op action that throws when executed */
- private static class ThrowingDummyAction extends TestAction {
- public ThrowingDummyAction(NestedSet<Artifact> inputs, ImmutableSet<Artifact> outputs) {
- super(NO_EFFECT, inputs, outputs);
+ /** No-op action that throws when executed. */
+ private static final class ThrowingDummyAction extends TestAction {
+
+ ThrowingDummyAction(Artifact output) {
+ super(NO_EFFECT, NestedSetBuilder.emptySet(Order.STABLE_ORDER), ImmutableSet.of(output));
}
- /** Throws */
+ ThrowingDummyAction(Artifact input, Artifact output) {
+ super(NO_EFFECT, NestedSetBuilder.create(Order.STABLE_ORDER, input), ImmutableSet.of(output));
+ }
+
+ /** Unconditionally throws. */
@Override
public ActionResult execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException {
- throw new ActionExecutionException("Throwing dummy action", this, true);
+ throw new ActionExecutionException("Throwing dummy action", this, /*catastrophe=*/ true);
}
}
}
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 60d89bf..ef80a1e 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
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.skyframe;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.devtools.build.lib.actions.ActionInputHelper.asTreeFileArtifacts;
import static org.junit.Assert.assertThrows;
import com.google.common.base.Predicate;
@@ -24,7 +23,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Actions;
@@ -82,7 +80,7 @@
private List<PathFragment> testTreeArtifactContents;
@Before
- public final void setUp() throws Exception {
+ public final void setUp() {
delegateActionExecutionFunction = new TreeArtifactExecutionFunction();
}
@@ -105,8 +103,9 @@
SpecialArtifact tree, Iterable<PathFragment> children) throws Exception {
TreeArtifactValue value = evaluateTreeArtifact(tree, children);
assertThat(value.getChildPaths()).containsExactlyElementsIn(ImmutableSet.copyOf(children));
- assertThat(value.getChildren()).containsExactlyElementsIn(
- asTreeFileArtifacts(tree, children));
+ assertThat(value.getChildren())
+ .containsExactlyElementsIn(
+ Iterables.transform(children, child -> TreeFileArtifact.createTreeOutput(tree, child)));
// Assertions about digest. As of this writing this logic is essentially the same
// as that in TreeArtifact, but it's good practice to unit test anyway to guard against
@@ -286,7 +285,7 @@
SpecialArtifact output = (SpecialArtifact) Iterables.getOnlyElement(action.getOutputs());
for (PathFragment subpath : testTreeArtifactContents) {
try {
- TreeFileArtifact suboutput = ActionInputHelper.treeFileArtifact(output, subpath);
+ TreeFileArtifact suboutput = TreeFileArtifact.createTreeOutput(output, subpath);
Path path = suboutput.getPath();
FileArtifactValue noDigest =
ActionMetadataHandler.fileArtifactValueFromArtifact(
