Clean up the semantics of input discovering actions a bit by making updateInputs() and inputsKnown() non-overridable and removing setInputs().
This comes at the cost of adding a flag to every action instance that's not used for non-input-discovering actions, but I think that's a deal. Simpler APIs are good, mmmmkay?
Also fixed a few pre-existing issues in TestAction and ObjcCompileAction.
--
PiperOrigin-RevId: 148749734
MOS_MIGRATED_REVID=148749734
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 80f6d7d..78a1d5e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -45,6 +45,7 @@
import java.util.Collection;
import java.util.Map;
import java.util.Map.Entry;
+import javax.annotation.concurrent.GuardedBy;
/**
* Abstract implementation of Action which implements basic functionality: the inputs, outputs, and
@@ -94,8 +95,13 @@
*/
private final Iterable<Artifact> tools;
+ @GuardedBy("this")
+ private boolean inputsDiscovered = false; // Only used when discoversInputs() returns true
+
// The variable inputs is non-final only so that actions that discover their inputs can modify it.
+ @GuardedBy("this")
private Iterable<Artifact> inputs;
+
private final Iterable<String> clientEnvironmentVariables;
private final RunfilesSupplier runfilesSupplier;
private final ImmutableSet<Artifact> outputs;
@@ -164,15 +170,36 @@
}
@Override
- public boolean inputsKnown() {
- return true;
+ public final synchronized boolean inputsDiscovered() {
+ return discoversInputs() ? inputsDiscovered : true;
}
+ /**
+ * Should be overridden by actions that do input discovery.
+ *
+ * <p>The value returned by each instance should be constant over the lifetime of that instance.
+ *
+ * <p>If this returns true, {@link #discoverInputs(ActionExecutionContext)} must also be
+ * implemented.
+ */
@Override
public boolean discoversInputs() {
return false;
}
+ /**
+ * Run input discovery on the action.
+ *
+ * <p>Called by Blaze if {@link #discoversInputs()} returns true. It must return the set of
+ * input artifacts that were not known at analysis time. May also call
+ * {@link #updateInputs(Iterable<Artifact>)}; if it doesn't, the action itself must arrange for
+ * the newly discovered artifacts to be available during action execution, probably by keeping
+ * state in the action instance and using a custom action execution context and for
+ * {@code #updateInputs()} to be called during the execution of the action.
+ *
+ * <p>Since keeping state within an action bad, don't do that unless there is a very good reason
+ * to do so.
+ */
@Override
public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
@@ -192,10 +219,21 @@
"Method must be overridden for actions that may have unknown inputs.");
}
+ /**
+ * Should be called when the inputs of the action become known, that is, either during
+ * {@link #discoverInputs(ActionExecutionContext)} or during
+ * {@link #execute(ActionExecutionContext)}.
+ *
+ * <p>When an action discovers inputs, it must have been called by the time {@code #execute()}
+ * returns. It can be called both during {@code discoverInputs} and during {@code execute()}.
+ *
+ * <p>In addition to being called from action implementations, it will also be called by Bazel
+ * itself when an action is loaded from the on-disk action cache.
+ */
@Override
- public void updateInputs(Iterable<Artifact> inputs) {
- throw new IllegalStateException(
- "Method must be overridden for actions that may have unknown inputs.");
+ public final synchronized void updateInputs(Iterable<Artifact> inputs) {
+ this.inputs = CollectionUtils.makeImmutable(inputs);
+ inputsDiscovered = true;
}
@Override
@@ -204,12 +242,10 @@
}
/**
- * Should only be overridden by actions that need to optionally insert inputs. Actions that
- * discover their inputs should use {@link #setInputs} to set the new iterable of inputs when they
- * know it.
+ * Should not be overridden (it's non-final only for tests)
*/
@Override
- public Iterable<Artifact> getInputs() {
+ public synchronized Iterable<Artifact> getInputs() {
return inputs;
}
@@ -223,15 +259,6 @@
return runfilesSupplier;
}
- /**
- * Set the inputs of the action. May only be used by an action that {@link #discoversInputs()}.
- * The iterable passed in is automatically made immutable.
- */
- protected void setInputs(Iterable<Artifact> inputs) {
- Preconditions.checkState(discoversInputs(), this);
- this.inputs = CollectionUtils.makeImmutable(inputs);
- }
-
@Override
public ImmutableSet<Artifact> getOutputs() {
return outputs;
@@ -259,7 +286,7 @@
@Override
public String toString() {
return prettyPrint() + " (" + getMnemonic() + "[" + ImmutableList.copyOf(getInputs())
- + (inputsKnown() ? " -> " : ", unknown inputs -> ")
+ + (inputsDiscovered() ? " -> " : ", unknown inputs -> ")
+ getOutputs() + "]" + ")";
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java
index bee4774..ee193ba 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Action.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java
@@ -176,7 +176,7 @@
/**
* Informs the action that its inputs are {@code inputs}, and that its inputs are now known. Can
* only be called for actions that discover inputs. After this method is called,
- * {@link ActionExecutionMetadata#inputsKnown} should return true.
+ * {@link ActionExecutionMetadata#inputsDiscovered} should return true.
*/
void updateInputs(Iterable<Artifact> inputs);
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
index 7170836..eac03d9 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -196,8 +196,8 @@
}
Iterable<Artifact> actionInputs = action.getInputs();
// Resolve action inputs from cache, if necessary.
- boolean inputsKnown = action.inputsKnown();
- if (!inputsKnown && resolvedCacheArtifacts != null) {
+ boolean inputsDiscovered = action.inputsDiscovered();
+ if (!inputsDiscovered && resolvedCacheArtifacts != null) {
// The action doesn't know its inputs, but the caller has a good idea of what they are.
Preconditions.checkState(action.discoversInputs(),
"Actions that don't know their inputs must discover them: %s", action);
@@ -211,7 +211,7 @@
return new Token(getKeyString(action));
}
- if (!inputsKnown) {
+ if (!inputsDiscovered) {
action.updateInputs(actionInputs);
}
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
index 63748af..6184954 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
@@ -106,7 +106,7 @@
* built first, as usual.
*/
@ThreadSafe
- boolean inputsKnown();
+ boolean inputsDiscovered();
/**
* Returns true iff inputsKnown() may ever return false.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index afc77d2..d1f0e4b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -78,7 +78,6 @@
import java.util.Set;
import java.util.UUID;
import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
/** Action that represents some kind of C++ compilation step. */
@ThreadCompatible
@@ -215,9 +214,8 @@
*/
private final UUID actionClassId;
- // This can be read/written from multiple threads, and so accesses should be synchronized.
- @GuardedBy("this")
- private boolean inputsKnown = false;
+ /** Whether this action needs to discover inputs. */
+ private final boolean discoversInputs;
/**
* Set when the action prepares for execution. Used to preserve state between preparation and
@@ -267,8 +265,6 @@
* @param lipoScannables List of artifacts to include-scan when this action is a lipo action
* @param additionalIncludeScannables list of additional artifacts to include-scan
* @param actionClassId TODO(bazel-team): Add parameter description
- * @param executionRequirements out-of-band hints to be passed to the execution backend to signal
- * platform requirements
* @param environment TODO(bazel-team): Add parameter description
* @param builtinIncludeFiles List of include files that may be included even if they are not
* mentioned in the source file or any of the headers included by it
@@ -338,7 +334,7 @@
Preconditions.checkArgument(!shouldPruneModules || shouldScanIncludes, this);
this.usePic = usePic;
this.useHeaderModules = useHeaderModules;
- this.inputsKnown = !shouldScanIncludes && !cppSemantics.needsDotdInputPruning();
+ this.discoversInputs = shouldScanIncludes || cppSemantics.needsDotdInputPruning();
this.cppCompileCommandLine =
new CppCompileCommandLine(
sourceFile, dotdFile, copts, coptsFilter, features, variables, actionName);
@@ -405,11 +401,6 @@
return super.getMandatoryOutputs();
}
- @Override
- public synchronized boolean inputsKnown() {
- return inputsKnown;
- }
-
/**
* Returns the list of additional inputs found by dependency discovery, during action preparation,
* and clears the stored list. {@link #prepare} must be called before this method is called, on
@@ -428,7 +419,7 @@
@Override
public boolean discoversInputs() {
- return true;
+ return discoversInputs;
}
@VisibleForTesting // productionVisibility = Visibility.PRIVATE
@@ -761,7 +752,7 @@
}
info.setOutputFile(outputFile.getExecPathString());
info.setSourceFile(getSourceFile().getExecPathString());
- if (inputsKnown()) {
+ if (inputsDiscovered()) {
info.addAllSourcesAndHeaders(Artifact.toExecPaths(getInputs()));
} else {
info.addSourcesAndHeaders(getSourceFile().getExecPathString());
@@ -958,11 +949,10 @@
*
* @throws ActionExecutionException iff any errors happen during update.
*/
- @VisibleForTesting
+ @VisibleForTesting // productionVisibility = Visibility.PRIVATE
@ThreadCompatible
- public final synchronized void updateActionInputs(NestedSet<Artifact> discoveredInputs)
+ public final void updateActionInputs(NestedSet<Artifact> discoveredInputs)
throws ActionExecutionException {
- inputsKnown = false;
NestedSetBuilder<Artifact> inputs = NestedSetBuilder.stableOrder();
Profiler.instance().startTask(ProfilerTask.ACTION_UPDATE, this);
try {
@@ -972,12 +962,9 @@
}
inputs.addAll(context.getTransitiveCompilationPrerequisites());
inputs.addTransitive(discoveredInputs);
- inputsKnown = true;
+ updateInputs(inputs.build());
} finally {
Profiler.instance().completeTask(ProfilerTask.ACTION_UPDATE);
- synchronized (this) {
- setInputs(inputs.build());
- }
}
}
@@ -1013,18 +1000,6 @@
return variableBuilder.build();
}
- @Override protected void setInputs(Iterable<Artifact> inputs) {
- super.setInputs(inputs);
- }
-
- @Override
- public synchronized void updateInputs(Iterable<Artifact> inputs) {
- inputsKnown = true;
- synchronized (this) {
- setInputs(inputs);
- }
- }
-
@Override
public Iterable<Artifact> getAllowedDerivedInputs() {
return getAllowedDerivedInputsMap().values();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java
index 0b71041..9af100f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LTOBackendAction.java
@@ -37,7 +37,6 @@
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
/**
* Action used by LTOBackendArtifacts to create an LTOBackendAction. Similar to {@link SpawnAction},
@@ -54,10 +53,6 @@
* http://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html.
*/
public final class LTOBackendAction extends SpawnAction {
- // This can be read/written from multiple threads, and so accesses should be synchronized.
- @GuardedBy("this")
- private boolean inputsKnown;
-
private Collection<Artifact> mandatoryInputs;
private Map<PathFragment, Artifact> bitcodeFiles;
private Artifact imports;
@@ -92,8 +87,6 @@
mnemonic,
false,
null);
-
- inputsKnown = false;
mandatoryInputs = inputs;
bitcodeFiles = allBitcodeFiles;
imports = importsFile;
@@ -150,11 +143,6 @@
}
@Override
- public synchronized boolean inputsKnown() {
- return inputsKnown;
- }
-
- @Override
public Collection<Artifact> getMandatoryInputs() {
return mandatoryInputs;
}
@@ -167,12 +155,6 @@
}
@Override
- public synchronized void updateInputs(Iterable<Artifact> discoveredInputs) {
- setInputs(discoveredInputs);
- inputsKnown = true;
- }
-
- @Override
public Iterable<Artifact> getAllowedDerivedInputs() {
return bitcodeFiles.values();
}
@@ -181,10 +163,6 @@
public void execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
super.execute(actionExecutionContext);
-
- synchronized (this) {
- inputsKnown = true;
- }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
index a293b00..b2f7fdb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
@@ -41,7 +41,6 @@
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
/**
* Action used by extra_action rules to create an action that shadows an existing action. Runs a
@@ -54,10 +53,6 @@
private final ImmutableSet<Artifact> extraActionInputs;
private final Iterable<Artifact> originalShadowedActionInputs;
- // This can be read/written from multiple threads, and so accesses should be synchronized.
- @GuardedBy("this")
- private boolean inputsKnown;
-
/**
* A long way to say (ExtraAction xa) -> xa.getShadowedAction().
*/
@@ -104,7 +99,6 @@
this.createDummyOutput = createDummyOutput;
this.extraActionInputs = extraActionInputs;
- inputsKnown = shadowedAction.inputsKnown();
if (createDummyOutput) {
// Expecting just a single dummy file in the outputs.
Preconditions.checkArgument(outputs.size() == 1, outputs);
@@ -123,7 +117,7 @@
Preconditions.checkState(discoversInputs(), this);
// We depend on the outputs of actions doing input discovery and they should know their inputs
// after having been executed
- Preconditions.checkState(shadowedAction.inputsKnown());
+ Preconditions.checkState(shadowedAction.inputsDiscovered());
// We need to update our inputs to take account of any additional
// inputs the shadowed action may need to do its work.
@@ -132,11 +126,6 @@
ImmutableSet.copyOf(originalShadowedActionInputs));
}
- @Override
- public synchronized boolean inputsKnown() {
- return inputsKnown;
- }
-
private static NestedSet<Artifact> createInputs(
Iterable<Artifact> shadowedActionInputs,
ImmutableSet<Artifact> extraActionInputs,
@@ -152,12 +141,6 @@
}
@Override
- public synchronized void updateInputs(Iterable<Artifact> discoveredInputs) {
- setInputs(discoveredInputs);
- inputsKnown = true;
- }
-
- @Override
public Iterable<Artifact> getAllowedDerivedInputs() {
return shadowedAction.getAllowedDerivedInputs();
}
@@ -189,9 +172,6 @@
}
}
}
- synchronized (this) {
- inputsKnown = true;
- }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
index 8164cc3..8087928 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
@@ -46,7 +46,6 @@
import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.rules.cpp.HeaderDiscovery;
-import com.google.devtools.build.lib.rules.cpp.HeaderDiscovery.DotdPruningMode;
import com.google.devtools.build.lib.rules.cpp.IncludeScanningContext;
import com.google.devtools.build.lib.util.DependencySet;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -56,13 +55,11 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import javax.annotation.Nullable;
-import javax.annotation.concurrent.GuardedBy;
/**
* An action that compiles objc or objc++ source.
@@ -111,10 +108,6 @@
private Iterable<Artifact> discoveredInputs;
- // This can be read/written from multiple threads, so accesses must be synchronized.
- @GuardedBy("this")
- private boolean inputsKnown = false;
-
private static final String GUID = "a00d5bac-a72c-4f0f-99a7-d5fdc6072137";
private ObjcCompileAction(
@@ -157,7 +150,6 @@
this.sourceFile = sourceFile;
this.mandatoryInputs = mandatoryInputs;
this.dotdPruningPlan = dotdPruningPlan;
- this.inputsKnown = (dotdPruningPlan == DotdPruningMode.DO_NOT_USE && headersListFile == null);
this.headers = headers;
this.headersListFile = headersListFile;
}
@@ -182,11 +174,6 @@
}
@Override
- public synchronized boolean inputsKnown() {
- return inputsKnown;
- }
-
- @Override
public final Spawn getSpawn(Map<String, String> clientEnv) {
return new ObjcCompileActionSpawn(clientEnv);
}
@@ -287,14 +274,6 @@
}
@Override
- public synchronized void updateInputs(Iterable<Artifact> inputs) {
- inputsKnown = true;
- synchronized (this) {
- setInputs(inputs);
- }
- }
-
- @Override
public ImmutableSet<Artifact> getMandatoryOutputs() {
return ImmutableSet.of(dotdFile.artifact());
}
@@ -312,6 +291,12 @@
executor.getExecRoot(), scanningContext.getArtifactResolver());
updateActionInputs(discoveredInputs);
+ } else {
+ // TODO(lberki): This is awkward, but necessary since updateInputs() must be called when
+ // input discovery is in effect. I *think* it's possible to avoid setting discoversInputs()
+ // to true if the header list file is null and then we'd not need to have this here, but I
+ // haven't quite managed to get that right yet.
+ updateActionInputs(getInputs());
}
}
@@ -370,26 +355,22 @@
*
* @throws ActionExecutionException iff any errors happen during update.
*/
- @VisibleForTesting
+ @VisibleForTesting // productionVisibility = Visibility.PRIVATE
@ThreadCompatible
- public final synchronized void updateActionInputs(Iterable<Artifact> discoveredInputs)
+ final void updateActionInputs(Iterable<Artifact> discoveredInputs)
throws ActionExecutionException {
- inputsKnown = false;
- Iterable<Artifact> inputs = Collections.emptyList();
Profiler.instance().startTask(ProfilerTask.ACTION_UPDATE, this);
try {
- inputs = Iterables.concat(mandatoryInputs, discoveredInputs);
- inputsKnown = true;
+ updateInputs(Iterables.concat(mandatoryInputs, discoveredInputs));
} finally {
Profiler.instance().completeTask(ProfilerTask.ACTION_UPDATE);
- setInputs(inputs);
}
}
@Override
protected SpawnInfo getExtraActionSpawnInfo() {
SpawnInfo.Builder info = SpawnInfo.newBuilder(super.getExtraActionSpawnInfo());
- if (!inputsKnown()) {
+ if (!inputsDiscovered()) {
for (Artifact headerArtifact : filterHeaderFiles()) {
// As in SpawnAction#getExtraActionSpawnInfo explicitly ignore middleman artifacts here.
if (!headerArtifact.isMiddlemanArtifact()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 538ef90..905c3a9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -227,7 +227,7 @@
throws ActionExecutionFunctionException, InterruptedException {
Iterable<Artifact> allKnownInputs = Iterables.concat(
action.getInputs(), action.getRunfilesSupplier().getArtifacts());
- if (action.inputsKnown()) {
+ if (action.inputsDiscovered()) {
return new AllInputs(allKnownInputs);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index c122f3b..1d23376 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -798,7 +798,7 @@
private void completeAction(Action action, MetadataHandler metadataHandler, FileOutErr fileOutErr,
boolean outputAlreadyDumped) throws ActionExecutionException {
try {
- Preconditions.checkState(action.inputsKnown(),
+ Preconditions.checkState(action.inputsDiscovered(),
"Action %s successfully executed, but inputs still not known", action);
profiler.startTask(ProfilerTask.ACTION_COMPLETE, action);
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
index d05123e..9605c31 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java
@@ -492,7 +492,7 @@
}
@Override
- public boolean inputsKnown() {
+ public boolean inputsDiscovered() {
throw new IllegalStateException();
}
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 c0a3230..2d74dd8 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
@@ -81,7 +81,7 @@
@Override
public boolean discoversInputs() {
for (Artifact input : getInputs()) {
- if (!input.getExecPath().getBaseName().endsWith(".optional")) {
+ if (input.getExecPath().getBaseName().endsWith(".optional")) {
return true;
}
}
@@ -91,6 +91,7 @@
@Override
public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext) {
Preconditions.checkState(discoversInputs(), this);
+ updateInputs(getInputs());
return ImmutableList.of();
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java
index bef984f..eb196ec 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LTOBackendActionTest.java
@@ -98,11 +98,11 @@
assertEquals(AbstractAction.DEFAULT_RESOURCE_SET, action.getSpawn().getLocalResources());
assertThat(action.getArguments()).containsExactly("/bin/clang");
assertEquals("Test", action.getProgressMessage());
- assertThat(action.inputsKnown()).isFalse();
+ assertThat(action.inputsDiscovered()).isFalse();
// Discover inputs, which should not add any inputs since bitcode1.imports is empty.
action.discoverInputs(context);
- assertThat(action.inputsKnown()).isTrue();
+ assertThat(action.inputsDiscovered()).isTrue();
assertThat(action.getInputs()).containsExactly(bitcode1Artifact, index1Artifact);
}
@@ -125,11 +125,11 @@
assertEquals(AbstractAction.DEFAULT_RESOURCE_SET, action.getSpawn().getLocalResources());
assertThat(action.getArguments()).containsExactly("/bin/clang");
assertEquals("Test", action.getProgressMessage());
- assertThat(action.inputsKnown()).isFalse();
+ assertThat(action.inputsDiscovered()).isFalse();
// Discover inputs, which should add bitcode1.o which is listed in bitcode2.imports.
action.discoverInputs(context);
- assertThat(action.inputsKnown()).isTrue();
+ assertThat(action.inputsDiscovered()).isTrue();
assertThat(action.getInputs())
.containsExactly(bitcode1Artifact, bitcode2Artifact, index2Artifact);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionDataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionDataTest.java
index 342784c..05442e6 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionDataTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionDataTest.java
@@ -14,30 +14,24 @@
package com.google.devtools.build.lib.skyframe;
import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.AbstractAction;
-import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
-import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.ResourceSet;
-import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.actions.util.DummyExecutor;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
-
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
import java.io.IOException;
import java.util.Collection;
import java.util.Set;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/**
* Tests that the data passed from the application to the Builder is passed
@@ -104,72 +98,4 @@
null);
assertSame(executor, action.executor);
}
-
- private static class InputDiscoveringAction extends AbstractAction {
- private final Collection<Artifact> discoveredInputs;
-
- public InputDiscoveringAction(Artifact output, Collection<Artifact> discoveredInputs) {
- super(
- ActionsTestUtil.NULL_ACTION_OWNER,
- ImmutableList.<Artifact>of(),
- ImmutableList.of(output));
- this.discoveredInputs = discoveredInputs;
- }
-
- @Override
- public boolean discoversInputs() {
- return true;
- }
-
- @Override
- public boolean inputsKnown() {
- return true;
- }
-
- @Override
- public Iterable<Artifact> getMandatoryInputs() {
- return ImmutableList.of();
- }
-
- @Override
- public Iterable<Artifact> getInputs() {
- return discoveredInputs;
- }
-
- @Override
- public void execute(ActionExecutionContext actionExecutionContext) {
- throw new IllegalStateException();
- }
-
- @Override
- public String getMnemonic() {
- return "InputDiscovering";
- }
-
- @Override
- protected String computeKey() {
- return "";
- }
-
- @Override
- public ResourceSet estimateResourceConsumption(Executor executor) {
- return ResourceSet.ZERO;
- }
- }
-
- @Test
- public void testActionSharabilityAndDiscoveredInputs() throws Exception {
- Artifact output =
- new Artifact(
- scratch.file("/out/output"), Root.asDerivedRoot(scratch.dir("/"), scratch.dir("/out")));
- Artifact discovered =
- new Artifact(
- scratch.file("/bin/discovered"),
- Root.asDerivedRoot(scratch.dir("/"), scratch.dir("/bin")));
-
- Action a = new InputDiscoveringAction(output, ImmutableList.of(discovered));
- Action b = new InputDiscoveringAction(output, ImmutableList.<Artifact>of());
-
- assertTrue(Actions.canBeShared(a, b));
- }
}
