Guard include scanner against null grepIncludes
The grepIncludes artifact may be null if the owning rule doesn't have a
$grep_includes attribute (see CppHelper.getGrepIncludes). This seems to
be intentional. However, the remote include scanner requires that
grepIncludes is *not* null, so we need to safe-guard against it.
Unfortunately, this is brittle, because accidentally misspelling or
removing the attributes makes all grep operations fall back to the
built-in implementation.
PiperOrigin-RevId: 277499382
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 0857f29..acde8ac 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
@@ -843,13 +843,16 @@
Artifact file,
ActionExecutionMetadata actionExecutionMetadata,
ActionExecutionContext actionExecutionContext,
- Artifact grepIncludes,
+ @Nullable Artifact grepIncludes,
@Nullable SpawnIncludeScanner remoteIncludeScanner,
boolean isOutputFile)
throws IOException, ExecException, InterruptedException {
Collection<Inclusion> inclusions;
+ // TODO(ulfjack): grepIncludes may be null if the corresponding attribute on the rule is missing
+ // (see CppHelper.getGrepIncludes) or misspelled. It would be better to disallow this case.
if (remoteIncludeScanner != null
+ && grepIncludes != null
&& remoteIncludeScanner.shouldParseRemotely(file, actionExecutionContext)) {
inclusions =
remoteIncludeScanner.extractInclusions(
@@ -866,7 +869,7 @@
extractInclusions(
FileSystemUtils.readContent(actionExecutionContext.getInputPath(file)));
} catch (IOException e) {
- if (remoteIncludeScanner != null) {
+ if (remoteIncludeScanner != null && grepIncludes != null) {
logger.log(
Level.WARNING,
"Falling back on remote parsing of " + actionExecutionContext.getInputPath(file),
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 0c16c97..440fef0 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
@@ -650,7 +651,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;
@@ -661,7 +662,7 @@
public LegacyIncludeVisitor(
ActionExecutionMetadata actionExecutionMetadata,
ActionExecutionContext actionExecutionContext,
- Artifact grepIncludes,
+ @Nullable Artifact grepIncludes,
final Map<PathFragment, Artifact> pathToLegalOutputArtifact,
Set<Artifact> modularHeaders) {
super(
