Automated rollback of commit bd7999eed71fc14c68890553c68fa5a8f608922c.
*** Reason for rollback ***
Breaks targets in nightly:
[]
which are not passing the grep includes binary.
*** Original change description ***
C++: Give error when grep-includes missing
This was crashing during execution of compilation actions.
RELNOTES:none
PiperOrigin-RevId: 318803927
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 c81e25a..4515374 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,7 +21,6 @@
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;
@@ -62,8 +61,7 @@
public void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder,
- RuleErrorConsumer ruleErrorConsumer) {
+ CppCompileActionBuilder actionBuilder) {
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 b39f970..9c75b1c 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,
- Artifact grepIncludes,
+ @Nullable 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 5bd0c8e..c58abe6 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,6 +58,7 @@
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
@@ -748,7 +749,7 @@
private class LegacyIncludeVisitor extends AbstractQueueVisitor implements IncludeVisitor {
private final ActionExecutionMetadata actionExecutionMetadata;
private final ActionExecutionContext actionExecutionContext;
- private final Artifact grepIncludes;
+ @Nullable 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;
@@ -759,7 +760,7 @@
LegacyIncludeVisitor(
ActionExecutionMetadata actionExecutionMetadata,
ActionExecutionContext actionExecutionContext,
- Artifact grepIncludes,
+ @Nullable 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 5e93ee2..0fb9224 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,8 +1444,7 @@
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* additionalBuildVariables= */ ImmutableMap.of()));
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, builder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
// 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.
@@ -1687,8 +1686,7 @@
builder.setGcnoFile(gcnoFile);
builder.setDwoFile(dwoFile);
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, builder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
Artifact objectFile = compileAction.getOutputFile();
@@ -1738,8 +1736,7 @@
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* additionalBuildVariables= */ ImmutableMap.of()));
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, builder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
Artifact tokenFile = compileAction.getOutputFile();
@@ -1848,8 +1845,7 @@
picBuilder.setDwoFile(dwoFile);
picBuilder.setLtoIndexingFile(ltoIndexingFile);
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, picBuilder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, picBuilder);
CppCompileAction picAction = picBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(picAction);
directOutputs.add(picAction.getOutputFile());
@@ -1924,8 +1920,7 @@
builder.setDwoFile(noPicDwoFile);
builder.setLtoIndexingFile(ltoIndexingFile);
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, builder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
Artifact objectFile = compileAction.getOutputFile();
@@ -2023,8 +2018,7 @@
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
/* additionalBuildVariables= */ ImmutableMap.of()));
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, builder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
CppCompileAction action = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(action);
if (addObject) {
@@ -2146,8 +2140,7 @@
ImmutableMap.of(
CompileBuildVariables.OUTPUT_PREPROCESS_FILE.getVariableName(),
dBuilder.getRealOutputFilePath().getSafePathString())));
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, dBuilder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, dBuilder);
CppCompileAction dAction = dBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(dAction);
@@ -2173,8 +2166,7 @@
ImmutableMap.of(
CompileBuildVariables.OUTPUT_ASSEMBLY_FILE.getVariableName(),
sdBuilder.getRealOutputFilePath().getSafePathString())));
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, sdBuilder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, sdBuilder);
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 79dcef1..230789e 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,9 +634,4 @@
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 3d31bac..eab64d8 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,8 +90,7 @@
.setShareable(true)
.setShouldScanIncludes(false)
.setActionName(CppActionNames.LINKSTAMP_COMPILE);
- semantics.finalizeCompileActionBuilder(
- configuration, featureConfiguration, builder, ruleErrorConsumer);
+ semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
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 96e1b1e..f411173 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,7 +19,6 @@
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;
@@ -35,8 +34,7 @@
void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder,
- RuleErrorConsumer ruleErrorConsumer);
+ CppCompileActionBuilder actionBuilder);
/** 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 39a571b..fad0815 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,7 +21,6 @@
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;
@@ -75,8 +74,7 @@
public void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder,
- RuleErrorConsumer ruleErrorConsumer) {
+ CppCompileActionBuilder actionBuilder) {
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 4c5b7d7..9e20694 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,6 +420,7 @@
// 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 eca9843..1931d54 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,7 +20,6 @@
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;
@@ -40,8 +39,7 @@
public void finalizeCompileActionBuilder(
BuildConfiguration configuration,
FeatureConfiguration featureConfiguration,
- CppCompileActionBuilder actionBuilder,
- RuleErrorConsumer ruleErrorConsumer) {}
+ CppCompileActionBuilder actionBuilder) {}
@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 c1d2bcc..32d0f55 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
@@ -889,7 +889,7 @@
public void testFeatures() throws Exception {
setRuleContext(createRuleContext("//foo:cc_with_features"));
Object result = ev.eval("ruleContext.features");
- assertThat((Sequence) result).containsExactly("f1", "f2");
+ assertThat((Sequence) result).containsExactly("cc_include_scanning", "f1", "f2");
}
@Test
