diff options
5 files changed, 13 insertions, 66 deletions
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 7919529da0..80f6d7df6b 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 @@ -495,27 +495,6 @@ public abstract class AbstractAction implements Action, SkylarkValue { return ImmutableSet.of(); } - /** - * Returns input files that need to be present to allow extra_action rules to shadow this action - * correctly when run remotely. This is at least the normal inputs of the action, but may include - * other files as well. For example C(++) compilation may perform include file header scanning. - * This needs to be mirrored by the extra_action rule. Called by - * {@link com.google.devtools.build.lib.rules.extra.ExtraAction} at execution time. - * - * <p>As this method is called from the ExtraAction, make sure it is ok to call - * this method from a different thread than the one this action is executed on. - * - * @param actionExecutionContext Services in the scope of the action, like the Out/Err streams. - * @throws ActionExecutionException only when code called from this method - * throws that exception. - * @throws InterruptedException if interrupted - */ - public Iterable<Artifact> getInputFilesForExtraAction( - ActionExecutionContext actionExecutionContext) - throws ActionExecutionException, InterruptedException { - return getInputs(); - } - @SkylarkCallable( name = "inputs", doc = "A set of the input files of this action.", 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 1814e2516c..afc77d23c3 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 @@ -1265,32 +1265,6 @@ public class CppCompileAction extends AbstractAction } } - /** - * Provides list of include files needed for performing extra actions on this action when run - * remotely. The list of include files is created by performing a header scan on the known input - * files. - */ - @Override - public Iterable<Artifact> getInputFilesForExtraAction( - ActionExecutionContext actionExecutionContext) - throws ActionExecutionException, InterruptedException { - Iterable<Artifact> scannedIncludes; - try { - scannedIncludes = actionExecutionContext.getExecutor().getContext(actionContext) - .findAdditionalInputs(this, actionExecutionContext, cppSemantics.getIncludeProcessing()); - } catch (ExecException e) { - throw e.toActionExecutionException(this); - } - - if (scannedIncludes == null) { - return ImmutableList.of(); - } - - // Use a set to eliminate duplicates. - ImmutableSet.Builder<Artifact> result = ImmutableSet.builder(); - return result.addAll(getInputs()).addAll(scannedIncludes).build(); - } - @Override public String getMnemonic() { return "CppCompile"; } 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 866e45e20a..a293b00497 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 @@ -18,6 +18,7 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +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; @@ -51,6 +52,8 @@ public final class ExtraAction extends SpawnAction { private final boolean createDummyOutput; private final RunfilesSupplier runfilesSupplier; 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; @@ -95,6 +98,7 @@ public final class ExtraAction extends SpawnAction { mnemonic, false, null); + this.originalShadowedActionInputs = shadowedAction.getInputs(); this.shadowedAction = shadowedAction; this.runfilesSupplier = runfilesSupplier; this.createDummyOutput = createDummyOutput; @@ -117,15 +121,15 @@ public final class ExtraAction extends SpawnAction { public Iterable<Artifact> discoverInputs(ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException { 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()); + // We need to update our inputs to take account of any additional // inputs the shadowed action may need to do its work. - if (shadowedAction.discoversInputs() && shadowedAction instanceof AbstractAction) { - Iterable<Artifact> additionalInputs = - ((AbstractAction) shadowedAction).getInputFilesForExtraAction(actionExecutionContext); - updateInputs(createInputs(additionalInputs, extraActionInputs, runfilesSupplier)); - return ImmutableSet.copyOf(additionalInputs); - } - return null; + updateInputs(createInputs(shadowedAction.getInputs(), extraActionInputs, runfilesSupplier)); + return Sets.difference(ImmutableSet.copyOf(shadowedAction.getInputs()), + ImmutableSet.copyOf(originalShadowedActionInputs)); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java index 81f1a8683d..db3569a370 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java @@ -85,9 +85,10 @@ public final class ExtraActionSpec implements TransitiveInfoProvider { NestedSetBuilder<Artifact> extraActionInputs = NestedSetBuilder.stableOrder(); Label ownerLabel = owningRule.getLabel(); - if (requiresActionOutput) { + if (requiresActionOutput || actionToShadow.discoversInputs()) { extraActionInputs.addAll(actionToShadow.getOutputs()); } + extraActionInputs.addAll(resolvedTools); extraActionInputs.addAll(resolvedData); 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 f3d185cc01..8164cc3c85 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 @@ -387,17 +387,6 @@ public class ObjcCompileAction extends SpawnAction { } @Override - public Iterable<Artifact> getInputFilesForExtraAction( - ActionExecutionContext actionExecutionContext) - throws ActionExecutionException, InterruptedException { - // Use a set to eliminate duplicates. - return ImmutableSet.<Artifact>builder() - .addAll(getInputs()) - .addAll(discoverInputs(actionExecutionContext)) - .build(); - } - - @Override protected SpawnInfo getExtraActionSpawnInfo() { SpawnInfo.Builder info = SpawnInfo.newBuilder(super.getExtraActionSpawnInfo()); if (!inputsKnown()) { |