Skip to content

Commit

Permalink
Bypass execroot symlink forest creation when using virtual roots.
Browse files Browse the repository at this point in the history
To support Skymeld, there is somewhat duplicated logic in `SkyframeExecutor` (non-Skymeld) and `ExecutionTool` (Skymeld).

PiperOrigin-RevId: 642334521
Change-Id: I2d81e81cb214e6605bbf716874f7ce6f1a138995
  • Loading branch information
justinhorvitz authored and copybara-github committed Jun 11, 2024
1 parent affa59e commit 0c50709
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.buildtool;

import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -256,33 +257,34 @@ TestActionContext getTestActionContext() {
*
* <p>TODO(b/213040766): Write tests for these setup steps.
*/
public void prepareForExecution(Stopwatch executionTimer)
void prepareForExecution(Stopwatch executionTimer)
throws AbruptExitException,
BuildFailedException,
InterruptedException,
InvalidConfigurationException {
init();
BuildRequestOptions buildRequestOptions = request.getBuildOptions();

SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
boolean localActionsSupported =
env.getOutputService().actionFileSystemType().supportsLocalActions();

// TODO: b/290617036 - Reconsider this for local action support with virtual roots.
checkState(
!localActionsSupported || env.getDirectories().getVirtualSourceRoot() == null,
"Local actions are incompatible with virtual roots");

try (SilentCloseable c = Profiler.instance().profile("preparingExecroot")) {
boolean shouldSymlinksBePlanted =
skyframeExecutor.getForcedSingleSourceRootIfNoExecrootSymlinkCreation() == null;
Root singleSourceRoot =
shouldSymlinksBePlanted
? Iterables.getOnlyElement(env.getPackageLocator().getPathEntries())
: skyframeExecutor.getForcedSingleSourceRootIfNoExecrootSymlinkCreation();
IncrementalPackageRoots incrementalPackageRoots =
IncrementalPackageRoots.createAndRegisterToEventBus(
getExecRoot(),
singleSourceRoot,
// Single package path is a Skymeld prerequisite.
Iterables.getOnlyElement(env.getPackageLocator().getPathEntries()),
env.getEventBus(),
env.getDirectories().getProductName() + "-",
skyframeExecutor.getIgnoredPaths(),
request.getOptions(BuildLanguageOptions.class).experimentalSiblingRepositoryLayout,
runtime.getWorkspace().doesAllowExternalRepositories());
if (shouldSymlinksBePlanted) {
if (localActionsSupported) {
incrementalPackageRoots.eagerlyPlantSymlinksToSingleSourceRoot();
}

Expand All @@ -294,7 +296,7 @@ public void prepareForExecution(Stopwatch executionTimer)
OutputService outputService = env.getOutputService();
ModifiedFileSet modifiedOutputFiles =
startBuildAndDetermineModifiedOutputFiles(request.getId(), outputService);
if (outputService.actionFileSystemType().supportsLocalActions()) {
if (localActionsSupported) {
// Must be created after the output path is created above.
try (SilentCloseable c = Profiler.instance().profile("createActionLogDirectory")) {
createActionLogDirectory();
Expand Down Expand Up @@ -374,8 +376,7 @@ public void prepareForExecution(Stopwatch executionTimer)
* @param analysisResult the analysis phase output
* @param buildResult the mutable build result
* @param packageRoots package roots collected from loading phase and {@link
* BuildConfigurationValue} creation. May be empty if {@link
* SkyframeExecutor#getForcedSingleSourceRootIfNoExecrootSymlinkCreation} is false.
* BuildConfigurationValue} creation. May be empty if using virtual roots.
*/
void executeBuild(
UUID buildId,
Expand Down Expand Up @@ -738,7 +739,7 @@ private static BuildConfigurationValue getConfiguration(
*
* @return map of convenience symlink name to target
*/
public ImmutableMap<PathFragment, PathFragment> handleConvenienceSymlinks(
ImmutableMap<PathFragment, PathFragment> handleConvenienceSymlinks(
ImmutableSet<ConfiguredTarget> targetsToBuild, BuildConfigurationValue configuration) {
try (SilentCloseable c =
Profiler.instance().profile("ExecutionTool.handleConvenienceSymlinks")) {
Expand Down Expand Up @@ -902,7 +903,7 @@ public void handle(Event event) {
*
* @param configuredTargets The configured targets whose artifacts are to be built.
*/
static ImmutableSet<ConfiguredTarget> determineSuccessfulTargets(
private static ImmutableSet<ConfiguredTarget> determineSuccessfulTargets(
Collection<ConfiguredTarget> configuredTargets, Set<ConfiguredTargetKey> builtTargets) {
// Maintain the ordering by copying builtTargets into an ImmutableSet.Builder in the same
// iteration order as configuredTargets.
Expand All @@ -915,14 +916,14 @@ static ImmutableSet<ConfiguredTarget> determineSuccessfulTargets(
return successfulTargets.build();
}

static ImmutableSet<AspectKey> determineSuccessfulAspects(
private static ImmutableSet<AspectKey> determineSuccessfulAspects(
ImmutableSet<AspectKey> aspects, Set<AspectKey> builtAspects) {
// Maintain the ordering.
return aspects.stream().filter(builtAspects::contains).collect(ImmutableSet.toImmutableSet());
}

/** Get action cache if present or reload it from the on-disk cache. */
ActionCache getOrLoadActionCache() throws AbruptExitException {
private ActionCache getOrLoadActionCache() throws AbruptExitException {
try {
return env.getBlazeWorkspace().getOrLoadPersistentActionCache(getReporter());
} catch (IOException e) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ java_library(
":package_lookup_function",
":package_lookup_value",
":package_progress_receiver",
":package_roots_no_symlink_creation",
":package_value",
":pattern_expanding_error",
":precomputed_function",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1077,20 +1077,6 @@ protected GlobbingStrategy getGlobbingStrategy() {
return GlobbingStrategy.NON_SKYFRAME;
}

/**
* If not null, this is the only source root in the build, corresponding to the single element in
* a single-element package path. Such a single-source-root build need not plant the execroot
* symlink forest, and can trivially resolve source artifacts from exec paths. As a consequence,
* builds where this is not null do not need to track a package -> source root map. In addition,
* such builds can only occur in a monorepo, and thus do not need to produce repo mapping
* manifests for runfiles.
*/
// TODO(wyv): To be safe, fail early if we're in a multi-repo setup but this is not being tracked.
@Nullable
public Root getForcedSingleSourceRootIfNoExecrootSymlinkCreation() {
return null;
}

private boolean shouldStoreTransitivePackagesInLoadingAndAnalysis() {
// Transitive packages may be needed for either RepoMappingManifestAction or Skymeld with
// external repository support. They are never needed if external repositories are disabled. To
Expand Down Expand Up @@ -2117,9 +2103,18 @@ protected ConfigureTargetsResult configureTargets(
getPackageRoots());
}

@ForOverride
protected PackageRoots getPackageRoots() {
return new MapAsPackageRoots(collectPackageRoots());
private PackageRoots getPackageRoots() {
Root virtualSourceRoot = directories.getVirtualSourceRoot();
if (virtualSourceRoot == null) {
return new MapAsPackageRoots(collectPackageRoots());
}

// No need to plant symlinks when using virtual roots.
// TODO: b/290617036 - Reconsider this for local action support with virtual roots.
checkState(
!outputService.actionFileSystemType().supportsLocalActions(),
"Local actions are incompatible with virtual roots");
return new PackageRootsNoSymlinkCreation(virtualSourceRoot);
}

@Nullable
Expand Down

0 comments on commit 0c50709

Please sign in to comment.