Refactor include scanner API to use futures
Note that the implementation is still mostly synchronous.
Progress on #6394.
PiperOrigin-RevId: 245374534
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/CppIncludeScanningContextImpl.java b/src/main/java/com/google/devtools/build/lib/includescanning/CppIncludeScanningContextImpl.java
index 6652324..b611522 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/CppIncludeScanningContextImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/CppIncludeScanningContextImpl.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.includescanning;
import com.google.common.base.Supplier;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecException;
@@ -38,7 +39,7 @@
}
@Override
- public Iterable<Artifact> findAdditionalInputs(
+ public ListenableFuture<Iterable<Artifact>> findAdditionalInputs(
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
IncludeProcessing includeProcessing,
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
index 305d06c..cd4ff72 100644
--- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
+++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java
@@ -15,6 +15,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Supplier;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -47,6 +48,7 @@
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.WorkspaceBuilder;
import com.google.devtools.build.lib.skyframe.MutableSupplier;
+import com.google.devtools.build.lib.vfs.IORuntimeException;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -56,6 +58,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.logging.Logger;
@@ -180,18 +183,30 @@
for (Artifact path : legalOutputPaths) {
pathToLegalOutputArtifact.put(path.getExecPath(), path);
}
- scanner.process(
- source,
- ImmutableList.of(source),
- // For Swig include scanning just point to the output file in the map.
- new IncludeScanningHeaderData.Builder(
- pathToLegalOutputArtifact.build(), /*modularHeaders=*/ ImmutableSet.of())
- .build(),
- ImmutableList.of(),
- includes,
- actionExecutionMetadata,
- actionExecContext,
- grepIncludes);
+ try {
+ scanner
+ .processAsync(
+ source,
+ ImmutableList.of(source),
+ // For Swig include scanning just point to the output file in the map.
+ new IncludeScanningHeaderData.Builder(
+ pathToLegalOutputArtifact.build(), /*modularHeaders=*/ ImmutableSet.of())
+ .build(),
+ ImmutableList.of(),
+ includes,
+ actionExecutionMetadata,
+ actionExecContext,
+ grepIncludes)
+ .get();
+ } catch (ExecutionException e) {
+ Throwables.throwIfInstanceOf(e.getCause(), ExecException.class);
+ Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
+ if (e.getCause() instanceof IORuntimeException) {
+ throw ((IORuntimeException) e.getCause()).getCauseIOException();
+ }
+ Throwables.throwIfUnchecked(e.getCause());
+ throw new IllegalStateException(e.getCause());
+ }
}
}
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 b37676d..d14bbb2 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
@@ -18,6 +18,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.Artifact;
@@ -417,7 +419,7 @@
}
@Override
- public void process(
+ public ListenableFuture<?> processAsync(
Artifact mainSource,
Collection<Artifact> sources,
IncludeScanningHeaderData includeScanningHeaderData,
@@ -427,140 +429,34 @@
ActionExecutionContext actionExecutionContext,
Artifact grepIncludes)
throws IOException, ExecException, InterruptedException {
- try {
- // Because our IncludeVisitor can dynamically switch to in-thread execution, we could get
- // unlucky and run one of the async calls on the main thread. Catch and rethrow the runtime
- // exceptions as the appropriate corresponding checked exception.
- processInternal(
- mainSource,
- sources,
- includeScanningHeaderData,
- cmdlineIncludes,
- includes,
- actionExecutionMetadata,
- actionExecutionContext,
- grepIncludes);
- } catch (IORuntimeException e) {
- throw e.getCauseIOException();
- } catch (ExecRuntimeException e) {
- throw e.getRealCause();
- } catch (InterruptedRuntimeException e) {
- throw e.getRealCause();
- }
+ ImmutableSet<Artifact> pathHints =
+ prepare(actionExecutionContext.getEnvironmentForDiscoveringInputs());
+ IncludeVisitor visitor = new IncludeVisitor(includeScanningHeaderData.getModularHeaders());
+ return visitor.processInternal(
+ mainSource,
+ sources,
+ includeScanningHeaderData,
+ cmdlineIncludes,
+ includes,
+ actionExecutionMetadata,
+ actionExecutionContext,
+ grepIncludes,
+ pathHints);
}
- private void processInternal(
- Artifact mainSource,
- Collection<Artifact> sources,
- IncludeScanningHeaderData includeScanningHeaderData,
- List<String> cmdlineIncludes,
- Set<Artifact> includes,
- ActionExecutionMetadata actionExecutionMetadata,
- ActionExecutionContext actionExecutionContext,
- Artifact grepIncludes)
- throws IOException, ExecException, InterruptedException {
- Preconditions.checkArgument(mainSource == null || sources.contains(mainSource),
- "The main source '%s' is not part of '%s'", mainSource, sources);
- ImmutableSet.Builder<Artifact> pathHints = null;
- SkyFunction.Environment env = actionExecutionContext.getEnvironmentForDiscoveringInputs();
+ private ImmutableSet<Artifact> prepare(SkyFunction.Environment env) throws InterruptedException {
if (parser.getHints() != null) {
- pathHints = ImmutableSet.builderWithExpectedSize(quoteIncludePaths.size());
Collection<Artifact> artifacts =
parser.getHints().getPathLevelHintedInclusions(quoteIncludePaths, env);
- if (!env.valuesMissing()) {
- pathHints.addAll(Preconditions.checkNotNull(artifacts, quoteIncludePaths));
+ if (env.valuesMissing()) {
+ throw new MissingDepException();
}
+ ImmutableSet.Builder<Artifact> pathHints;
+ pathHints = ImmutableSet.builderWithExpectedSize(quoteIncludePaths.size());
+ pathHints.addAll(Preconditions.checkNotNull(artifacts, quoteIncludePaths));
+ return pathHints.build();
}
- if (env.valuesMissing()) {
- throw new MissingDepException();
- }
-
- Set<ArtifactWithInclusionContext> visitedInclusions = Sets.newConcurrentHashSet();
-
- IncludeVisitor visitor = new IncludeVisitor(includeScanningHeaderData.getModularHeaders());
-
- try {
- // Process cmd line includes, if specified.
- if (mainSource != null && !cmdlineIncludes.isEmpty()) {
- visitor.processCmdlineIncludes(
- mainSource,
- cmdlineIncludes,
- grepIncludes,
- includes,
- includeScanningHeaderData.getPathToLegalOutputArtifact(),
- actionExecutionMetadata,
- actionExecutionContext,
- visitedInclusions);
- visitor.sync();
- }
-
- visitor.processBulkAsync(
- sources,
- includes,
- includeScanningHeaderData.getPathToLegalOutputArtifact(),
- actionExecutionMetadata,
- actionExecutionContext,
- visitedInclusions,
- grepIncludes);
- visitor.sync();
-
- // Process include hints
- // TODO(ulfjack): Make this code go away. Use the new hinted inclusions instead.
- Hints hints = parser.getHints();
- if (hints != null) {
- // Follow "path" hints.
- visitor.processBulkAsync(
- pathHints.build(),
- includes,
- includeScanningHeaderData.getPathToLegalOutputArtifact(),
- actionExecutionMetadata,
- actionExecutionContext,
- visitedInclusions,
- grepIncludes);
- // Follow "file" hints for the primary sources.
- for (Artifact source : sources) {
- visitor.processFileLevelHintsAsync(
- hints,
- source,
- includes,
- includeScanningHeaderData.getPathToLegalOutputArtifact(),
- actionExecutionMetadata,
- actionExecutionContext,
- visitedInclusions,
- grepIncludes);
- }
- visitor.sync();
-
- // Follow "file" hints for all included headers, transitively.
- Set<Artifact> frontier = includes;
- while (!frontier.isEmpty()) {
- Set<Artifact> adjacent = Sets.newConcurrentHashSet();
- for (Artifact include : frontier) {
- visitor.processFileLevelHintsAsync(
- hints,
- include,
- adjacent,
- includeScanningHeaderData.getPathToLegalOutputArtifact(),
- actionExecutionMetadata,
- actionExecutionContext,
- visitedInclusions,
- grepIncludes);
- }
- visitor.sync();
- // Keep novel nodes as the next frontier.
- for (Iterator<Artifact> iter = adjacent.iterator(); iter.hasNext(); ) {
- if (!includes.add(iter.next())) {
- iter.remove();
- }
- }
- frontier = adjacent;
- }
- }
- } catch (IOException | InterruptedException | ExecException | MissingDepException e) {
- // Careful: Do not leak visitation threads if we have an exception in the initial thread.
- visitor.sync();
- throw e;
- }
+ return ImmutableSet.of();
}
private static void checkForInterrupt(String operation, Object source)
@@ -590,8 +486,8 @@
}
/**
- * Implements a potentially parallel traversal over source files using a
- * thread pool shared across different IncludeScanner instances.
+ * Implements a potentially parallel traversal over source files using a thread pool shared across
+ * different IncludeScanner instances.
*/
private class IncludeVisitor extends AbstractQueueVisitor {
/** The set of headers known to be part of a C++ module. Scanning can stop here. */
@@ -606,10 +502,106 @@
this.modularHeaders = modularHeaders;
}
- /**
- * Block for the completion of all outstanding visitations.
- */
- public void sync() throws IOException, ExecException, InterruptedException {
+ ListenableFuture<?> processInternal(
+ Artifact mainSource,
+ Collection<Artifact> sources,
+ IncludeScanningHeaderData includeScanningHeaderData,
+ List<String> cmdlineIncludes,
+ Set<Artifact> includes,
+ ActionExecutionMetadata actionExecutionMetadata,
+ ActionExecutionContext actionExecutionContext,
+ Artifact grepIncludes,
+ ImmutableSet<Artifact> pathHints)
+ throws InterruptedException, IOException, ExecException {
+ Set<ArtifactWithInclusionContext> visitedInclusions = Sets.newConcurrentHashSet();
+
+ try {
+ // Process cmd line includes, if specified.
+ if (mainSource != null && !cmdlineIncludes.isEmpty()) {
+ processCmdlineIncludes(
+ mainSource,
+ cmdlineIncludes,
+ grepIncludes,
+ includes,
+ includeScanningHeaderData.getPathToLegalOutputArtifact(),
+ actionExecutionMetadata,
+ actionExecutionContext,
+ visitedInclusions);
+ sync();
+ }
+
+ processBulkAsync(
+ sources,
+ includes,
+ includeScanningHeaderData.getPathToLegalOutputArtifact(),
+ actionExecutionMetadata,
+ actionExecutionContext,
+ visitedInclusions,
+ grepIncludes);
+ sync();
+
+ // Process include hints
+ // TODO(ulfjack): Make this code go away. Use the new hinted inclusions instead.
+ Hints hints = parser.getHints();
+ if (hints != null) {
+ // Follow "path" hints.
+ processBulkAsync(
+ pathHints,
+ includes,
+ includeScanningHeaderData.getPathToLegalOutputArtifact(),
+ actionExecutionMetadata,
+ actionExecutionContext,
+ visitedInclusions,
+ grepIncludes);
+ // Follow "file" hints for the primary sources.
+ for (Artifact source : sources) {
+ processFileLevelHintsAsync(
+ hints,
+ source,
+ includes,
+ includeScanningHeaderData.getPathToLegalOutputArtifact(),
+ actionExecutionMetadata,
+ actionExecutionContext,
+ visitedInclusions,
+ grepIncludes);
+ }
+ sync();
+
+ // Follow "file" hints for all included headers, transitively.
+ Set<Artifact> frontier = includes;
+ while (!frontier.isEmpty()) {
+ Set<Artifact> adjacent = Sets.newConcurrentHashSet();
+ for (Artifact include : frontier) {
+ processFileLevelHintsAsync(
+ hints,
+ include,
+ adjacent,
+ includeScanningHeaderData.getPathToLegalOutputArtifact(),
+ actionExecutionMetadata,
+ actionExecutionContext,
+ visitedInclusions,
+ grepIncludes);
+ }
+ sync();
+ // Keep novel nodes as the next frontier.
+ for (Iterator<Artifact> iter = adjacent.iterator(); iter.hasNext(); ) {
+ if (!includes.add(iter.next())) {
+ iter.remove();
+ }
+ }
+ frontier = adjacent;
+ }
+ }
+ } catch (IOException | InterruptedException | ExecException | MissingDepException e) {
+ // Careful: Do not leak visitation threads if we have an exception in the initial thread.
+ sync();
+ throw e;
+ }
+ return Futures.immediateFuture(null);
+ }
+
+ /** Block for the completion of all outstanding visitations. */
+ private void sync() throws IOException, ExecException, InterruptedException {
try {
super.awaitQuiescence(true);
} catch (InterruptedException e) {
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 047587d..73e73d4 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
@@ -77,6 +77,7 @@
import com.google.devtools.build.lib.util.ShellEscaper;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.IORuntimeException;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
@@ -97,6 +98,7 @@
import java.util.Map;
import java.util.Set;
import java.util.UUID;
+import java.util.concurrent.ExecutionException;
import javax.annotation.Nullable;
/** Action that represents some kind of C++ compilation step. */
@@ -401,28 +403,37 @@
private Iterable<Artifact> findUsedHeaders(
ActionExecutionContext actionExecutionContext, IncludeScanningHeaderData headerData)
throws ActionExecutionException, InterruptedException {
- Iterable<Artifact> includeScanningResult;
+ if (!shouldScanIncludes()) {
+ return NestedSetBuilder.fromNestedSet(ccCompilationContext.getDeclaredIncludeSrcs())
+ .addTransitive(additionalPrunableHeaders)
+ .build();
+ }
try {
- includeScanningResult =
- actionExecutionContext
- .getContext(CppIncludeScanningContext.class)
- .findAdditionalInputs(
- this,
- actionExecutionContext,
- includeProcessing,
- headerData);
+ try {
+ return actionExecutionContext
+ .getContext(CppIncludeScanningContext.class)
+ .findAdditionalInputs(this, actionExecutionContext, includeProcessing, headerData)
+ .get();
+ } catch (ExecutionException e) {
+ Throwables.throwIfInstanceOf(e.getCause(), ExecException.class);
+ Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
+ if (e.getCause() instanceof IORuntimeException) {
+ throw new EnvironmentalExecException(
+ ((IORuntimeException) e.getCause()).getCauseIOException().getMessage(),
+ ((IORuntimeException) e.getCause()).getCauseIOException());
+ }
+ if (e.getCause() instanceof IOException) {
+ throw new EnvironmentalExecException(e.getMessage(), e);
+ }
+ Throwables.throwIfUnchecked(e.getCause());
+ throw new IllegalStateException(e.getCause());
+ }
} catch (ExecException e) {
throw e.toActionExecutionException(
"Include scanning of rule '" + getOwner().getLabel() + "'",
actionExecutionContext.getVerboseFailures(),
this);
}
- if (includeScanningResult != null) {
- return includeScanningResult;
- }
- return NestedSetBuilder.fromNestedSet(ccCompilationContext.getDeclaredIncludeSrcs())
- .addTransitive(additionalPrunableHeaders)
- .build();
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppIncludeScanningContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppIncludeScanningContext.java
index 72f842e..21e5d38 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppIncludeScanningContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppIncludeScanningContext.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionContextMarker;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -33,7 +34,7 @@
* <p>Returns null if additional inputs will only be found during action execution, not before.
*/
@Nullable
- Iterable<Artifact> findAdditionalInputs(
+ ListenableFuture<Iterable<Artifact>> findAdditionalInputs(
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
IncludeProcessing includeProcessing,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java
index 0a85187..0843be9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecException;
@@ -24,7 +25,7 @@
/** Used as an interface to thin header inputs to compile actions for C++-like compiles. */
public interface IncludeProcessing {
/** Performs include processing actions and returns the processed set of resulting headers. */
- Iterable<Artifact> determineAdditionalInputs(
+ ListenableFuture<Iterable<Artifact>> determineAdditionalInputs(
@Nullable IncludeScannerSupplier includeScannerSupplier,
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java
index e5d83d5..d81b6d2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanner.java
@@ -15,22 +15,14 @@
package com.google.devtools.build.lib.rules.cpp;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.UserExecException;
-import com.google.devtools.build.lib.profiler.Profiler;
-import com.google.devtools.build.lib.profiler.ProfilerTask;
-import com.google.devtools.build.lib.profiler.SilentCloseable;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.Collection;
-import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -49,23 +41,24 @@
* impact in the case that we are scanning a single source file, since it is already known to be
* an input. However, this is necessary when we have more than one source to scan from, for
* example when building C++ modules. In that case we have one of two possibilities:
+ *
* <ol>
- * <li>We compile a header module - there, the .cppmap file is the main source file (which we do
- * not include-scan, as that would require an extra parser), and thus already in the input;
- * all headers in the .cppmap file are our entry points for include scanning, but are not yet
- * in the inputs - they get added here.</li>
- * <li>We compile an object file that uses a header module; currently using a header module
- * requires all headers it can reference to be available for the compilation. The header
- * module can reference headers that are not in the transitive include closure of the current
- * translation unit. Therefore, {@link CppCompileAction} adds all headers specified
- * transitively for compiled header modules as include scanning entry points, and we need to
- * add the entry points to the inputs here.</li></ol>
- * </p>
+ * <li>We compile a header module - there, the .cppmap file is the main source file (which we do
+ * not include-scan, as that would require an extra parser), and thus already in the input;
+ * all headers in the .cppmap file are our entry points for include scanning, but are not
+ * yet in the inputs - they get added here.
+ * <li>We compile an object file that uses a header module; currently using a header module
+ * requires all headers it can reference to be available for the compilation. The header
+ * module can reference headers that are not in the transitive include closure of the
+ * current translation unit. Therefore, {@link CppCompileAction} adds all headers specified
+ * transitively for compiled header modules as include scanning entry points, and we need to
+ * add the entry points to the inputs here.
+ * </ol>
*
* <p>{@code mainSource} is the source file relative to which the {@code cmdlineIncludes} are
- * interpreted.</p>
+ * interpreted.
*/
- void process(
+ ListenableFuture<?> processAsync(
Artifact mainSource,
Collection<Artifact> sources,
IncludeScanningHeaderData includeScanningHeaderData,
@@ -87,104 +80,13 @@
}
/**
- * Helper class that exists just to provide a static method that prepares the arguments with which
- * to call an IncludeScanner.
- */
- class IncludeScanningPreparer {
- private IncludeScanningPreparer() {}
-
- /**
- * Returns the files transitively included by the source files of the given IncludeScannable.
- *
- * @param action IncludeScannable whose sources' transitive includes will be returned.
- * @param includeScannerSupplier supplies IncludeScanners to actually do the transitive
- * scanning (and caching results) for a given source file.
- * @param actionExecutionContext the context for {@code action}.
- * @param profilerTaskName what the {@link Profiler} should record this call for.
- */
- public static Collection<Artifact> scanForIncludedInputs(
- IncludeScannable action,
- IncludeScannerSupplier includeScannerSupplier,
- IncludeScanningHeaderData includeScanningHeaderData,
- ActionExecutionMetadata actionExecutionMetadata,
- ActionExecutionContext actionExecutionContext,
- String profilerTaskName)
- throws ExecException, InterruptedException {
-
- Set<Artifact> includes = Sets.newConcurrentHashSet();
-
- final List<PathFragment> absoluteBuiltInIncludeDirs = new ArrayList<>();
- includes.addAll(action.getBuiltInIncludeFiles());
-
- Profiler profiler = Profiler.instance();
- try (SilentCloseable c = profiler.profile(ProfilerTask.SCANNER, profilerTaskName)) {
- // Deduplicate include directories. This can occur especially with "built-in" and "system"
- // include directories because of the way we retrieve them. Duplicate include directories
- // really mess up #include_next directives.
- Set<PathFragment> includeDirs = new LinkedHashSet<>(action.getIncludeDirs());
- List<PathFragment> quoteIncludeDirs = action.getQuoteIncludeDirs();
- List<String> cmdlineIncludes = includeScanningHeaderData.getCmdlineIncludes();
-
- includeDirs.addAll(includeScanningHeaderData.getSystemIncludeDirs());
-
- // Add the system include paths to the list of include paths.
- for (PathFragment pathFragment : action.getBuiltInIncludeDirectories()) {
- if (pathFragment.isAbsolute()) {
- absoluteBuiltInIncludeDirs.add(pathFragment);
- }
- includeDirs.add(pathFragment);
- }
-
- List<PathFragment> includeDirList = ImmutableList.copyOf(includeDirs);
- IncludeScanner scanner =
- includeScannerSupplier.scannerFor(quoteIncludeDirs, includeDirList);
-
- Artifact mainSource = action.getMainIncludeScannerSource();
- Collection<Artifact> sources = action.getIncludeScannerSources();
- scanner.process(
- mainSource,
- sources,
- includeScanningHeaderData,
- cmdlineIncludes,
- includes,
- actionExecutionMetadata,
- actionExecutionContext,
- action.getGrepIncludes());
-
- } catch (IOException e) {
- throw new EnvironmentalExecException(e.getMessage());
- }
-
- // Collect inputs and output
- List<Artifact> inputs = new ArrayList<>();
- for (Artifact included : includes) {
- // Check for absolute includes -- we assign the file system root as
- // the root path for such includes
- if (included.getRoot().getRoot().isAbsolute()) {
- if (FileSystemUtils.startsWithAny(
- actionExecutionContext.getInputPath(included).asFragment(),
- absoluteBuiltInIncludeDirs)) {
- // Skip include files found in absolute include directories.
- continue;
- }
- throw new UserExecException(
- "illegal absolute path to include file: "
- + actionExecutionContext.getInputPath(included));
- }
- inputs.add(included);
- }
- return inputs;
- }
- }
-
- /**
* Holds pre-aggregated information that the {@link IncludeScanner} needs from the compilation
* action.
*/
class IncludeScanningHeaderData {
/**
* Lookup table to find the {@link Artifact}s of generated files based on their {@link
- * Artifact#execPath}.
+ * Artifact#getExecPath}.
*/
private final Map<PathFragment, Artifact> pathToLegalOutputArtifact;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java
index 9e68607..61dc2ee 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeScanning.java
@@ -15,21 +15,41 @@
package com.google.devtools.build.lib.rules.cpp;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.AsyncFunction;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.actions.UserExecException;
+import com.google.devtools.build.lib.profiler.Profiler;
+import com.google.devtools.build.lib.profiler.ProfilerTask;
+import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScannerSupplier;
import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScanningHeaderData;
-import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScanningPreparer;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
import javax.annotation.Nullable;
/** Runs include scanning on actions, if include scanning is enabled. */
public class IncludeScanning implements IncludeProcessing {
+
@AutoCodec public static final IncludeScanning INSTANCE = new IncludeScanning();
+ @Nullable
@Override
- public @Nullable Iterable<Artifact> determineAdditionalInputs(
+ public ListenableFuture<Iterable<Artifact>> determineAdditionalInputs(
@Nullable IncludeScannerSupplier includeScannerSupplier,
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
@@ -41,12 +61,87 @@
return null;
}
- return IncludeScanningPreparer.scanForIncludedInputs(
- action,
- Preconditions.checkNotNull(includeScannerSupplier, action),
- includeScanningHeaderData,
- action,
- actionExecutionContext,
- action.getSourceFile().getExecPathString());
+ Preconditions.checkNotNull(includeScannerSupplier, action);
+
+ Set<Artifact> includes = Sets.newConcurrentHashSet();
+
+ final List<PathFragment> absoluteBuiltInIncludeDirs = new ArrayList<>();
+ includes.addAll(action.getBuiltInIncludeFiles());
+
+ // Deduplicate include directories. This can occur especially with "built-in" and "system"
+ // include directories because of the way we retrieve them. Duplicate include directories
+ // really mess up #include_next directives.
+ Set<PathFragment> includeDirs = new LinkedHashSet<>(action.getIncludeDirs());
+ List<PathFragment> quoteIncludeDirs = action.getQuoteIncludeDirs();
+ List<String> cmdlineIncludes = includeScanningHeaderData.getCmdlineIncludes();
+
+ includeDirs.addAll(includeScanningHeaderData.getSystemIncludeDirs());
+
+ // Add the system include paths to the list of include paths.
+ for (PathFragment pathFragment : action.getBuiltInIncludeDirectories()) {
+ if (pathFragment.isAbsolute()) {
+ absoluteBuiltInIncludeDirs.add(pathFragment);
+ }
+ includeDirs.add(pathFragment);
+ }
+
+ List<PathFragment> includeDirList = ImmutableList.copyOf(includeDirs);
+ IncludeScanner scanner = includeScannerSupplier.scannerFor(quoteIncludeDirs, includeDirList);
+
+ Artifact mainSource = action.getMainIncludeScannerSource();
+ Collection<Artifact> sources = action.getIncludeScannerSources();
+
+ Profiler profiler = Profiler.instance();
+ try (SilentCloseable c =
+ profiler.profile(ProfilerTask.SCANNER, action.getSourceFile().getExecPathString())) {
+ ListenableFuture<?> future =
+ scanner.processAsync(
+ mainSource,
+ sources,
+ includeScanningHeaderData,
+ cmdlineIncludes,
+ includes,
+ action,
+ actionExecutionContext,
+ action.getGrepIncludes());
+ return Futures.transformAsync(
+ future,
+ new AsyncFunction<Object, Iterable<Artifact>>() {
+ @Override
+ public ListenableFuture<Iterable<Artifact>> apply(Object input) throws Exception {
+ return Futures.immediateFuture(
+ collect(actionExecutionContext, includes, absoluteBuiltInIncludeDirs));
+ }
+ },
+ MoreExecutors.directExecutor());
+ } catch (IOException e) {
+ throw new EnvironmentalExecException("I/O failure during include scanning", e);
+ }
+ }
+
+ private static List<Artifact> collect(
+ ActionExecutionContext actionExecutionContext,
+ Set<Artifact> includes,
+ List<PathFragment> absoluteBuiltInIncludeDirs)
+ throws ExecException {
+ // Collect inputs and output
+ List<Artifact> inputs = new ArrayList<>();
+ for (Artifact included : includes) {
+ // Check for absolute includes -- we assign the file system root as
+ // the root path for such includes
+ if (included.getRoot().getRoot().isAbsolute()) {
+ if (FileSystemUtils.startsWithAny(
+ actionExecutionContext.getInputPath(included).asFragment(),
+ absoluteBuiltInIncludeDirs)) {
+ // Skip include files found in absolute include directories.
+ continue;
+ }
+ throw new UserExecException(
+ "illegal absolute path to include file: "
+ + actionExecutionContext.getInputPath(included));
+ }
+ inputs.add(included);
+ }
+ return inputs;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java
index 9946c52..07e08e3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java
@@ -14,9 +14,9 @@
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScannerSupplier;
import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScanningHeaderData;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -27,12 +27,11 @@
@AutoCodec public static final NoProcessing INSTANCE = new NoProcessing();
@Override
- public Iterable<Artifact> determineAdditionalInputs(
+ public ListenableFuture<Iterable<Artifact>> determineAdditionalInputs(
@Nullable IncludeScannerSupplier includeScannerSupplier,
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
- IncludeScanningHeaderData includeScanningHeaderData)
- throws ExecException, InterruptedException {
+ IncludeScanningHeaderData includeScanningHeaderData) {
return null;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/HeaderThinning.java b/src/main/java/com/google/devtools/build/lib/rules/objc/HeaderThinning.java
index c5d3ecb..95c32ea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/HeaderThinning.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/HeaderThinning.java
@@ -16,6 +16,8 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
@@ -74,7 +76,7 @@
}
@Override
- public Iterable<Artifact> determineAdditionalInputs(
+ public ListenableFuture<Iterable<Artifact>> determineAdditionalInputs(
@Nullable IncludeScannerSupplier includeScannerSupplier,
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
@@ -84,10 +86,14 @@
if (headersListFile == null) {
return null;
}
- return findRequiredHeaderInputs(action.getSourceFile(), headersListFile, getAllowedInputsMap(),
- actionExecutionContext == null
- ? ArtifactPathResolver.IDENTITY
- : actionExecutionContext.getPathResolver());
+ return Futures.immediateFuture(
+ findRequiredHeaderInputs(
+ action.getSourceFile(),
+ headersListFile,
+ getAllowedInputsMap(),
+ actionExecutionContext == null
+ ? ArtifactPathResolver.IDENTITY
+ : actionExecutionContext.getPathResolver()));
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/DummyCppIncludeScanningContext.java b/src/main/java/com/google/devtools/build/lib/standalone/DummyCppIncludeScanningContext.java
index d574027..656f826 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/DummyCppIncludeScanningContext.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/DummyCppIncludeScanningContext.java
@@ -13,10 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.standalone;
+import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
@@ -29,12 +28,11 @@
class DummyCppIncludeScanningContext implements CppIncludeScanningContext {
@Override
@Nullable
- public Iterable<Artifact> findAdditionalInputs(
+ public ListenableFuture<Iterable<Artifact>> findAdditionalInputs(
CppCompileAction action,
ActionExecutionContext actionExecutionContext,
IncludeProcessing includeProcessing,
- IncludeScanningHeaderData includeScanningHeaderData)
- throws ExecException, InterruptedException, ActionExecutionException {
+ IncludeScanningHeaderData includeScanningHeaderData) {
return null;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
index 63a4c13..861990c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/HeaderThinningTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
@@ -34,10 +35,13 @@
import com.google.devtools.build.lib.packages.util.MockObjcSupport;
import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.IORuntimeException;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ExecutionException;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -61,6 +65,22 @@
"--ios_sdk_version=" + MockObjcSupport.DEFAULT_IOS_SDK_VERSION);
}
+ private Iterable<Artifact> determineAdditionalInputs(
+ HeaderThinning headerThinning, CppCompileAction action)
+ throws ExecException, InterruptedException, IOException {
+ try {
+ return headerThinning.determineAdditionalInputs(null, action, null, null).get();
+ } catch (ExecutionException e) {
+ Throwables.throwIfInstanceOf(e.getCause(), ExecException.class);
+ Throwables.throwIfInstanceOf(e.getCause(), InterruptedException.class);
+ if (e.getCause() instanceof IORuntimeException) {
+ throw ((IORuntimeException) e.getCause()).getCauseIOException();
+ }
+ Throwables.throwIfUnchecked(e.getCause());
+ throw new IllegalStateException(e.getCause());
+ }
+ }
+
@Test
public void testCppCompileActionHeaderThinningCanDetermineAdditionalInputs() throws Exception {
Artifact sourceFile = getSourceArtifact("objc/a.m");
@@ -74,8 +94,7 @@
HeaderThinning headerThinning = new HeaderThinning(getPotentialHeaders(expectedHeaders));
writeToHeadersListFile(action, "objc/a.pch", "objc/b.h", "objc/c", "objc/d.hpp");
- Iterable<Artifact> headersFound =
- headerThinning.determineAdditionalInputs(null, action, null, null);
+ Iterable<Artifact> headersFound = determineAdditionalInputs(headerThinning, action);
assertThat(headersFound).containsExactlyElementsIn(expectedHeaders);
}
@@ -89,7 +108,7 @@
writeToHeadersListFile(action, "objc/a.h", "objc/b.h", "objc/c.h");
try {
- headerThinning.determineAdditionalInputs(null, action, null, null);
+ determineAdditionalInputs(headerThinning, action);
fail("Exception was not thrown");
} catch (ExecException e) {
assertThat(e).hasMessageThat().containsMatch("(objc/c.h)");
@@ -106,8 +125,7 @@
HeaderThinning headerThinning = new HeaderThinning(getPotentialHeaders(expectedHeaders));
writeToHeadersListFile(action, "objc/a.h", "tree/dir/c.h");
- Iterable<Artifact> headersFound =
- headerThinning.determineAdditionalInputs(null, action, null, null);
+ Iterable<Artifact> headersFound = determineAdditionalInputs(headerThinning, action);
assertThat(headersFound).containsExactlyElementsIn(expectedHeaders);
}
