C++: Give error when grep-includes missing
This was crashing during execution of compilation actions.
RELNOTES:none
PiperOrigin-RevId: 318268035
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
index 4515374..c81e25a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppSemantics.java
@@ -21,6 +21,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.Provider;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.StarlarkProvider;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.cpp.AspectLegalCppSemantics;
@@ -61,7 +62,8 @@
public void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder) {
+ CppCompileActionBuilder actionBuilder,
+ RuleErrorConsumer ruleErrorConsumer) {
CcToolchainProvider toolchain = actionBuilder.getToolchain();
actionBuilder
.addTransitiveMandatoryInputs(
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
index 9c75b1c..b39f970 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
@@ -853,7 +853,7 @@
Artifact file,
ActionExecutionMetadata actionExecutionMetadata,
ActionExecutionContext actionExecutionContext,
- @Nullable Artifact grepIncludes,
+ Artifact grepIncludes,
@Nullable SpawnIncludeScanner remoteIncludeScanner,
boolean isOutputFile)
throws IOException, ExecException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
index c58abe6..5bd0c8e 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
@@ -58,7 +58,6 @@
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
-import javax.annotation.Nullable;
/**
* C include scanner. Quickly scans C/C++ source files to determine the bounding set of transitively
@@ -749,7 +748,7 @@
private class LegacyIncludeVisitor extends AbstractQueueVisitor implements IncludeVisitor {
private final ActionExecutionMetadata actionExecutionMetadata;
private final ActionExecutionContext actionExecutionContext;
- @Nullable private final Artifact grepIncludes;
+ private final Artifact grepIncludes;
private final Map<PathFragment, Artifact> pathToLegalOutputArtifact;
/** The set of headers known to be part of a C++ module. Scanning can stop here. */
private final Set<Artifact> modularHeaders;
@@ -760,7 +759,7 @@
LegacyIncludeVisitor(
ActionExecutionMetadata actionExecutionMetadata,
ActionExecutionContext actionExecutionContext,
- @Nullable Artifact grepIncludes,
+ Artifact grepIncludes,
final Map<PathFragment, Artifact> pathToLegalOutputArtifact,
Set<Artifact> modularHeaders) {
super(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 0fb9224..5e93ee2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1444,7 +1444,8 @@
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* additionalBuildVariables= */ ImmutableMap.of()));
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, builder, ruleErrorConsumer);
// Make sure this builder doesn't reference ruleContext outside of analysis phase.
SpecialArtifact dotdTreeArtifact = null;
// The MSVC compiler won't generate .d file, instead we parse the output of /showIncludes flag.
@@ -1686,7 +1687,8 @@
builder.setGcnoFile(gcnoFile);
builder.setDwoFile(dwoFile);
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
Artifact objectFile = compileAction.getOutputFile();
@@ -1736,7 +1738,8 @@
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* additionalBuildVariables= */ ImmutableMap.of()));
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
Artifact tokenFile = compileAction.getOutputFile();
@@ -1845,7 +1848,8 @@
picBuilder.setDwoFile(dwoFile);
picBuilder.setLtoIndexingFile(ltoIndexingFile);
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, picBuilder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, picBuilder, ruleErrorConsumer);
CppCompileAction picAction = picBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(picAction);
directOutputs.add(picAction.getOutputFile());
@@ -1920,7 +1924,8 @@
builder.setDwoFile(noPicDwoFile);
builder.setLtoIndexingFile(ltoIndexingFile);
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
Artifact objectFile = compileAction.getOutputFile();
@@ -2018,7 +2023,8 @@
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* additionalBuildVariables= */ ImmutableMap.of()));
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction action = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(action);
if (addObject) {
@@ -2140,7 +2146,8 @@
ImmutableMap.of(
CompileBuildVariables.OUTPUT_PREPROCESS_FILE.getVariableName(),
dBuilder.getRealOutputFilePath().getSafePathString())));
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, dBuilder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, dBuilder, ruleErrorConsumer);
CppCompileAction dAction = dBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(dAction);
@@ -2166,7 +2173,8 @@
ImmutableMap.of(
CompileBuildVariables.OUTPUT_ASSEMBLY_FILE.getVariableName(),
sdBuilder.getRealOutputFilePath().getSafePathString())));
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, sdBuilder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, sdBuilder, ruleErrorConsumer);
CppCompileAction sdAction = sdBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(sdAction);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index 230789e..79dcef1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -634,4 +634,9 @@
Preconditions.checkNotNull(featureConfiguration);
return ccToolchain.shouldProcessHeaders(featureConfiguration, cppConfiguration);
}
+
+ @Nullable
+ public Artifact getGrepIncludes() {
+ return grepIncludes;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
index eab64d8..3d31bac 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
@@ -90,7 +90,8 @@
.setShareable(true)
.setShouldScanIncludes(false)
.setActionName(CppActionNames.LINKSTAMP_COMPILE);
- semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
+ semantics.finalizeCompileActionBuilder(
+ configuration, featureConfiguration, builder, ruleErrorConsumer);
return builder.buildOrThrowIllegalStateException();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
index f411173..96e1b1e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
@@ -34,7 +35,8 @@
void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder);
+ CppCompileActionBuilder actionBuilder,
+ RuleErrorConsumer ruleErrorConsumer);
/** Determines the applicable mode of headers checking for the passed in ruleContext. */
HeadersCheckingMode determineHeadersCheckingMode(RuleContext ruleContext);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
index fad0815..39a571b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
@@ -21,6 +21,7 @@
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.cpp.CcCompilationContext;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
@@ -74,7 +75,8 @@
public void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder) {
+ CppCompileActionBuilder actionBuilder,
+ RuleErrorConsumer ruleErrorConsumer) {
actionBuilder
// Without include scanning, we need the entire crosstool filegroup, including header files,
// as opposed to just the "compile" filegroup. Even with include scanning, we need the
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 9e20694..4c5b7d7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -420,7 +420,6 @@
// TODO(dmarting): Add --stamp option only to test that requires it.
allArgs.add("--stamp"); // Stamp is now defaulted to false.
allArgs.add("--experimental_extended_sanity_checks");
- allArgs.add("--features=cc_include_scanning");
// Always default to k8, even on mac and windows. Tests that need different cpu should set it
// using {@link useConfiguration()} explicitly.
allArgs.add("--cpu=k8");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java
index 1931d54..eca9843 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/MockCppSemantics.java
@@ -20,6 +20,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.AspectDescriptor;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
@@ -39,7 +40,8 @@
public void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder) {}
+ CppCompileActionBuilder actionBuilder,
+ RuleErrorConsumer ruleErrorConsumer) {}
@Override
public IncludeProcessing getIncludeProcessing() {
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleContextTest.java
index c9385d2..32d63bf 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleContextTest.java
@@ -888,7 +888,7 @@
public void testFeatures() throws Exception {
setRuleContext(createRuleContext("//foo:cc_with_features"));
Object result = ev.eval("ruleContext.features");
- assertThat((Sequence) result).containsExactly("cc_include_scanning", "f1", "f2");
+ assertThat((Sequence) result).containsExactly("f1", "f2");
}
@Test
