A partial, manual rollback of commit 7af14dfdbd6addb779226c0a103b2a8dc72c16b1.
This became necessary because extra actions for C++ compile actions require .h files, but the compiler only returns the .pcm files in the .d file for headers that it reads from the .pcm file. This is not a problem for correctness because the .pcm files depend on the headers, but that doesn't help the extra actions that would then only get the .pcm files.
--
PiperOrigin-RevId: 150052839
MOS_MIGRATED_REVID=150052839
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 78a1d5e..101cc39 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
@@ -522,6 +522,25 @@
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 for actions
+ * that return true for {link #discoversInputs()}.
+ *
+ * @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 ImmutableList.of();
+ }
+
@SkylarkCallable(
name = "inputs",
doc = "A set of the input files of this action.",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
index 4b45965..c2e2d3d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
@@ -88,6 +88,22 @@
ImmutableSet<Artifact> getOutputs();
/**
+ * 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 for actions
+ * that return true for {link #discoversInputs()}.
+ *
+ * @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
+ */
+ Iterable<Artifact> getInputFilesForExtraAction(ActionExecutionContext actionExecutionContext)
+ throws ActionExecutionException, InterruptedException;
+
+ /**
* Returns the set of output Artifacts that are required to be saved. This is
* used to identify items that would otherwise be potentially identified as
* orphaned (not consumed by any downstream {@link Action}s and potentially
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 6394c0c..6a64709 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
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
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.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
@@ -183,6 +184,12 @@
}
@Override
+ public Iterable<Artifact> getInputFilesForExtraAction(
+ ActionExecutionContext actionExecutionContext) {
+ return ImmutableList.of();
+ }
+
+ @Override
public ImmutableSet<Artifact> getMandatoryOutputs() {
return ImmutableSet.of();
}
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 d5b6c85..268dbff 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
@@ -1231,6 +1231,31 @@
}
}
+ /**
+ * When compiling with modules, the C++ compile action only has the {@code .pcm} files on its
+ * inputs, which is not enough for extra actions that parse header files. Thus, re-run include
+ * scanning and add headers to the inputs of the extra action, too.
+ */
+ @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();
+ }
+
+ return Sets.<Artifact>difference(
+ ImmutableSet.<Artifact>copyOf(scannedIncludes), ImmutableSet.<Artifact>copyOf(getInputs()));
+ }
+
@Override
public String getMnemonic() {
if (CppFileTypes.OBJC_SOURCE.matches(sourceFile.getExecPath())
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 d132c6a..8b80179 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
@@ -16,7 +16,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-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.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
@@ -145,6 +146,13 @@
}
@Override
+ public Iterable<Artifact> getInputFilesForExtraAction(
+ ActionExecutionContext actionExecutionContext)
+ throws ActionExecutionException, InterruptedException {
+ return ImmutableList.of();
+ }
+
+ @Override
public ImmutableSet<Artifact> getMandatoryOutputs() {
return ImmutableSet.<Artifact>of();
}
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 b2f7fdb..930cc93 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
@@ -80,7 +80,9 @@
super(
shadowedAction.getOwner(),
ImmutableList.<Artifact>of(),
- createInputs(shadowedAction.getInputs(), extraActionInputs, runfilesSupplier),
+ createInputs(
+ shadowedAction.getInputs(), ImmutableList.<Artifact>of(), extraActionInputs,
+ runfilesSupplier),
outputs,
AbstractAction.DEFAULT_RESOURCE_SET,
argv,
@@ -121,20 +123,29 @@
// We need to update our inputs to take account of any additional
// inputs the shadowed action may need to do its work.
- updateInputs(createInputs(shadowedAction.getInputs(), extraActionInputs, runfilesSupplier));
- return Sets.difference(ImmutableSet.copyOf(shadowedAction.getInputs()),
- ImmutableSet.copyOf(originalShadowedActionInputs));
+ Iterable<Artifact> oldInputs = getInputs();
+ updateInputs(createInputs(
+ shadowedAction.getInputs(),
+ shadowedAction.getInputFilesForExtraAction(actionExecutionContext),
+ extraActionInputs,
+ runfilesSupplier));
+ return Sets.<Artifact>difference(
+ ImmutableSet.<Artifact>copyOf(getInputs()), ImmutableSet.<Artifact>copyOf(oldInputs));
}
private static NestedSet<Artifact> createInputs(
Iterable<Artifact> shadowedActionInputs,
+ Iterable<Artifact> inputFilesForExtraAction,
ImmutableSet<Artifact> extraActionInputs,
RunfilesSupplier extraActionRunfilesSupplier) {
NestedSetBuilder<Artifact> result = new NestedSetBuilder<>(Order.STABLE_ORDER);
- if (shadowedActionInputs instanceof NestedSet) {
- result.addTransitive((NestedSet<Artifact>) shadowedActionInputs);
- } else {
- result.addAll(shadowedActionInputs);
+ for (Iterable<Artifact> inputSet : ImmutableList.of(
+ shadowedActionInputs, inputFilesForExtraAction)) {
+ if (inputSet instanceof NestedSet) {
+ result.addTransitive((NestedSet<Artifact>) inputSet);
+ } else {
+ result.addAll(inputSet);
+ }
}
result.addAll(extraActionRunfilesSupplier.getArtifacts());
return result.addAll(extraActionInputs).build();
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 32d7115..a6eff2e 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
@@ -20,6 +20,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
import com.google.devtools.build.lib.testutil.TestThread;
@@ -540,6 +541,12 @@
}
@Override
+ public Iterable<Artifact> getInputFilesForExtraAction(
+ ActionExecutionContext actionExecutionContext) {
+ return ImmutableList.of();
+ }
+
+ @Override
public String getKey() {
throw new IllegalStateException();
}
