Skip to content

Commit

Permalink
Expand tree artifact headers for include scanning.
Browse files Browse the repository at this point in the history
When compiling a header module with tree artifact headers, we run includes scanning on the tree itself. This doesn't work as the include scanner is not intended to work on directories. Expand tree artifacts before we pass them to the include scanner.

Base the tree artifact expansion on Skyframe directly since the `ArtifactExpander` only covers direct action inputs.

PiperOrigin-RevId: 482627266
Change-Id: I72ebbdcd47deea6ceb508be6b012353208997b01
  • Loading branch information
alexjski authored and copybara-github committed Oct 20, 2022
1 parent 08fd627 commit 2b67116
Show file tree
Hide file tree
Showing 4 changed files with 365 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:glob_descriptor",
"//src/main/java/com/google/devtools/build/lib/skyframe:glob_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:mutable_supplier",
"//src/main/java/com/google/devtools/build/lib/skyframe:per_build_syscall_cache",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.includescanning;

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
Expand All @@ -33,11 +35,13 @@
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.IncludeScanning.Code;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -83,14 +87,20 @@ public List<Artifact> findAdditionalInputs(
includeDirs.add(pathFragment);
}

List<PathFragment> includeDirList = ImmutableList.copyOf(includeDirs);
ImmutableList<PathFragment> includeDirList = ImmutableList.copyOf(includeDirs);
IncludeScanner scanner =
includeScannerSupplier
.get()
.scannerFor(quoteIncludeDirs, includeDirList, frameworkIncludeDirs);

Artifact mainSource = action.getMainIncludeScannerSource();
Collection<Artifact> sources = action.getIncludeScannerSources();
ImmutableList<Artifact> sources =
expandTreeArtifacts(
action.getIncludeScannerSources(),
actionExecutionContext.getEnvironmentForDiscoveringInputs());
if (sources == null) {
return null;
}

try (SilentCloseable c =
Profiler.instance()
Expand Down Expand Up @@ -120,6 +130,45 @@ public List<Artifact> findAdditionalInputs(
}
}

/**
* Returns a list of artifacts with all tree artifacts replaced by their expansion or null if we
* are missing a Skyframe dependency.
*
* <p>We take an ad-hoc approach, which consults Skyframe to retrieve the tree expansions. This is
* necessary because include scanning may include tree artifacts which are not inputs of the
* original action.
*
* <p>Normally, we expand tree artifacts using a tree artifact expander from the {@link
* ActionExecutionContext}, however the expander available before include scanning only captures
* tree artifacts from the action inputs, which is insufficient. In fact, the action execution
* context for include scanning has a null expander in it.
*/
@Nullable
private static ImmutableList<Artifact> expandTreeArtifacts(
ImmutableList<Artifact> artifacts, Environment env) throws InterruptedException {
ImmutableList<Artifact> trees =
artifacts.stream().filter(Artifact::isTreeArtifact).collect(toImmutableList());
if (trees.isEmpty()) {
return artifacts;
}

SkyframeLookupResult expansions = env.getValuesAndExceptions(trees);
ImmutableList.Builder<Artifact> expanded = ImmutableList.builder();
for (var artifact : artifacts) {
if (!artifact.isTreeArtifact()) {
expanded.add(artifact);
continue;
}

TreeArtifactValue treeArtifactValue = (TreeArtifactValue) expansions.get(artifact);
if (treeArtifactValue == null) {
return null;
}
expanded.addAll(treeArtifactValue.getChildren());
}
return expanded.build();
}

private static List<Artifact> collect(
ActionExecutionContext actionExecutionContext,
Set<Artifact> includes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.MemoizingEvaluator;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
Expand Down Expand Up @@ -220,6 +221,26 @@ public static ActionExecutionContext createContextForInputDiscovery(
MetadataHandler metadataHandler,
MemoizingEvaluator evaluator,
DiscoveredModulesPruner discoveredModulesPruner) {
return createContextForInputDiscovery(
executor,
eventHandler,
actionKeyContext,
fileOutErr,
execRoot,
metadataHandler,
new BlockingSkyFunctionEnvironment(evaluator, eventHandler),
discoveredModulesPruner);
}

public static ActionExecutionContext createContextForInputDiscovery(
Executor executor,
ExtendedEventHandler eventHandler,
ActionKeyContext actionKeyContext,
FileOutErr fileOutErr,
Path execRoot,
MetadataHandler metadataHandler,
Environment environment,
DiscoveredModulesPruner discoveredModulesPruner) {
return ActionExecutionContext.forInputDiscovery(
executor,
new SingleBuildFileCache(
Expand All @@ -232,7 +253,7 @@ public static ActionExecutionContext createContextForInputDiscovery(
fileOutErr,
eventHandler,
ImmutableMap.of(),
new BlockingSkyFunctionEnvironment(evaluator, eventHandler),
environment,
/*actionFileSystem=*/ null,
discoveredModulesPruner,
SyscallCache.NO_CACHE,
Expand Down
Loading

0 comments on commit 2b67116

Please sign in to comment.