diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 208a9d95588aa3..bb6401fe133e34 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -242,7 +242,7 @@ public AnalysisResult update( pollInterruptedStatus(); skyframeBuildView.resetProgressReceiver(); - skyframeExecutor.setBaselineConfiguration(targetOptions); + skyframeExecutor.setBaselineConfiguration(targetOptions, eventHandler); ImmutableMap.Builder labelToTargetsMapBuilder = ImmutableMap.builderWithExpectedSize(loadingResult.getTargetLabels().size()); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java index 54d804cb592ab8..159a85c617494b 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java @@ -153,7 +153,7 @@ public BlazeCommandResult exec( ensureSyncPackageLoading(env, optionsParsingResult); // TODO(bazel-team): What if there are multiple configurations? [multi-config] BuildOptions buildOptions = runtime.createBuildOptions(optionsParsingResult); - env.getSkyframeExecutor().setBaselineConfiguration(buildOptions); + env.getSkyframeExecutor().setBaselineConfiguration(buildOptions, env.getReporter()); return env.getSkyframeExecutor() .getConfiguration(env.getReporter(), buildOptions, /* keepGoing= */ true); } catch (InvalidConfigurationException e) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java index 21082d2472b142..6ffc3e53ce0328 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java @@ -101,6 +101,10 @@ public static Injected injected(Precomputed precomputed, T value) { public static final Precomputed BASELINE_CONFIGURATION = new Precomputed<>("baseline_configuration", /*shareable=*/ false); + // Unsharable because of complications in deserializing BuildOptions on startup due to caching. + public static final Precomputed BASELINE_EXEC_CONFIGURATION = + new Precomputed<>("baseline_exec_configuration", /* shareable= */ false); + private final Object value; @VisibleForTesting diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index c0720630a8ff2b..d0e0565c26b8ac 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -1399,8 +1399,22 @@ private void setAutoloadsConfiguration(AutoloadSymbols autoloadSymbols) { AutoloadSymbols.AUTOLOAD_SYMBOLS.set(injectable(), autoloadSymbols); } - public void setBaselineConfiguration(BuildOptions buildOptions) { + public void setBaselineConfiguration(BuildOptions buildOptions, ExtendedEventHandler eventHandler) + throws InvalidConfigurationException, InterruptedException { PrecomputedValue.BASELINE_CONFIGURATION.set(injectable(), buildOptions); + PrecomputedValue.BASELINE_EXEC_CONFIGURATION.set( + injectable(), adjustForExec(buildOptions, eventHandler)); + } + + public BuildOptions adjustForExec(BuildOptions buildOptions, ExtendedEventHandler eventHandler) + throws InvalidConfigurationException, InterruptedException { + StarlarkAttributeTransitionProvider execTransition; + try { + execTransition = getStarlarkExecTransition(buildOptions, eventHandler); + } catch (StarlarkExecTransitionLoadingException e) { + throw new InvalidConfigurationException(e); + } + return BaselineOptionsFunction.adjustForExec(buildOptions, execTransition, null, eventHandler); } public void injectExtraPrecomputedValues(List extraPrecomputedValues) { @@ -2008,20 +2022,25 @@ public Collection getTransitiveConfigurationKeys() { *

Production code handles this in Bazel's analysis phase skyfunctions. */ @Nullable - @VisibleForTesting - public StarlarkAttributeTransitionProvider getStarlarkExecTransitionForTesting( + public StarlarkAttributeTransitionProvider getStarlarkExecTransition( BuildOptions options, ExtendedEventHandler eventHandler) throws StarlarkExecTransitionLoadingException, InterruptedException { return StarlarkExecTransitionLoader.loadStarlarkExecTransition( options, - (bzlKey) -> - (BzlLoadValue) - evaluate( - ImmutableList.of(bzlKey), - /* keepGoing= */ false, - /* numThreads= */ DEFAULT_THREAD_COUNT, - eventHandler) - .get(bzlKey)) + (bzlKey) -> { + EvaluationResult result = + evaluate( + ImmutableList.of(bzlKey), + /* keepGoing= */ false, + /* numThreads= */ DEFAULT_THREAD_COUNT, + eventHandler); + if (result.hasError()) { + if (result.getError(bzlKey).getException() instanceof BzlLoadFailedException e) { + throw e; + } + } + return (BzlLoadValue) result.get(bzlKey); + }) .orElse(null); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/config/BaselineOptionsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/config/BaselineOptionsFunction.java index c7f255298dba1d..31984041755f67 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/config/BaselineOptionsFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/config/BaselineOptionsFunction.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.config.transitions.TransitionUtil; import com.google.devtools.build.lib.analysis.starlark.StarlarkAttributeTransitionProvider; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.AttributeTransitionData; import com.google.devtools.build.lib.skyframe.BzlLoadFailedException; import com.google.devtools.build.lib.skyframe.BzlLoadValue; @@ -71,41 +72,28 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (mappedBaselineOptions == null) { return null; } - - Optional starlarkExecTransition; - try { - starlarkExecTransition = - StarlarkExecTransitionLoader.loadStarlarkExecTransition( - mappedBaselineOptions, - (bzlKey) -> (BzlLoadValue) env.getValueOrThrow(bzlKey, BzlLoadFailedException.class)); - } catch (StarlarkExecTransitionLoadingException e) { - throw new BaselineOptionsFunctionException(e); - } - if (starlarkExecTransition == null) { - return null; - } - - // Next, apply elements of BaselineOptionsKey: apply exec transition and/or adjust platform BuildOptions adjustedBaselineOptions = mappedBaselineOptions; + if (key.afterExecTransition()) { - // A null executionPlatform actually skips transition application so need some value here when - // not overriding the platform. It is safe to supply some fake value here (as long as it is - // constant) since the baseline should never be used to actually construct an action or do - // toolchain resolution. - PatchTransition execTransition = - ExecutionTransitionFactory.createFactory() - .create( - AttributeTransitionData.builder() - .executionPlatform( - key.newPlatform() != null - ? key.newPlatform() - : Label.parseCanonicalUnchecked( - "//this_is_a_faked_exec_platform_for_blaze_internals")) - .analysisData(starlarkExecTransition.orElse(null)) - .build()); + Optional starlarkExecTransition; + try { + starlarkExecTransition = + StarlarkExecTransitionLoader.loadStarlarkExecTransition( + adjustedBaselineOptions, + (bzlKey) -> + (BzlLoadValue) env.getValueOrThrow(bzlKey, BzlLoadFailedException.class)); + } catch (StarlarkExecTransitionLoadingException e) { + throw new BaselineOptionsFunctionException(e); + } + if (starlarkExecTransition == null) { + return null; + } adjustedBaselineOptions = - execTransition.patch( - TransitionUtil.restrict(execTransition, adjustedBaselineOptions), env.getListener()); + adjustForExec( + mappedBaselineOptions, + starlarkExecTransition.orElse(null), + key.newPlatform(), + env.getListener()); } else if (key.newPlatform() != null) { // Clone for safety as-is the standard for all transitions. adjustedBaselineOptions = adjustedBaselineOptions.clone(); @@ -131,6 +119,36 @@ public SkyValue compute(SkyKey skyKey, Environment env) return BaselineOptionsValue.create(remappedAdjustedBaselineOptions); } + /** Adjusts the baseline options for the exec transition. */ + public static BuildOptions adjustForExec( + BuildOptions baselineOptions, + StarlarkAttributeTransitionProvider starlarkExecTransition, + Label newPlatform, + ExtendedEventHandler eventHandler) + throws InterruptedException { + + // A null executionPlatform actually skips transition application so need some value here when + // not overriding the platform. It is safe to supply some fake value here (as long as it is + // constant) since the baseline should never be used to actually construct an action or do + // toolchain resolution. + PatchTransition execTransition = + ExecutionTransitionFactory.createFactory() + .create( + AttributeTransitionData.builder() + .executionPlatform( + newPlatform != null + ? newPlatform + : Label.parseCanonicalUnchecked( + "//this_is_a_faked_exec_platform_for_blaze_internals")) + .analysisData(starlarkExecTransition) + .build()); + baselineOptions = + execTransition.patch( + TransitionUtil.restrict(execTransition, baselineOptions), eventHandler); + + return baselineOptions; + } + @Nullable private static BuildOptions mapBuildOptions(Environment env, BuildOptions rawBaselineOptions) throws InterruptedException, BaselineOptionsFunctionException { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java index 5f6973ce38350a..12bc20a9319d4e 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactoryTest.java @@ -47,7 +47,7 @@ private PatchTransition getExecTransition(Label execPlatform) throws Exception { .attributes(FakeAttributeMapper.empty()) .analysisData( getSkyframeExecutor() - .getStarlarkExecTransitionForTesting(targetConfig.getOptions(), reporter)) + .getStarlarkExecTransition(targetConfig.getOptions(), reporter)) .executionPlatform(execPlatform) .build()); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java index cd7708aad37c6a..a98a8db75e6d42 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionTest.java @@ -107,8 +107,7 @@ public void composeCommutativelyWithExecutionTransition() throws Exception { .attributes(FakeAttributeMapper.empty()) .analysisData( getSkyframeExecutor() - .getStarlarkExecTransitionForTesting( - targetConfig.getOptions(), reporter)) + .getStarlarkExecTransition(targetConfig.getOptions(), reporter)) .executionPlatform(executionPlatform) .build()); assertThat(execTransition).isNotNull(); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index f69100fc0a8214..d933f61bcb0a26 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -545,8 +545,7 @@ public static BuildOptions execOptions( .attributes(FakeAttributeMapper.empty()) .executionPlatform(targetOptions.get(PlatformOptions.class).hostPlatform) .analysisData( - skyframeExecutor.getStarlarkExecTransitionForTesting( - targetOptions, handler)) + skyframeExecutor.getStarlarkExecTransition(targetOptions, handler)) .build()) .apply(new BuildOptionsView(targetOptions, targetOptions.getFragmentClasses()), handler) .values()); 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 16c6b7e4a63c1f..421b1e0509cd74 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 @@ -457,7 +457,7 @@ protected final BuildConfigurationValue createConfiguration(String... args) thro // This is being done outside of BuildView, potentially even before the BuildView was // constructed and thus cannot rely on BuildView having injected this for us. - skyframeExecutor.setBaselineConfiguration(buildOptions); + skyframeExecutor.setBaselineConfiguration(buildOptions, reporter); return skyframeExecutor.createConfiguration(reporter, buildOptions, false); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java index a95d080ac19e89..779af8552f64d5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java @@ -189,7 +189,7 @@ protected BuildConfigurationValue createConfiguration( BuildOptions targetOptions = parseBuildOptions(starlarkOptions, args); skyframeExecutor.handleDiffsForTesting(reporter); - skyframeExecutor.setBaselineConfiguration(targetOptions); + skyframeExecutor.setBaselineConfiguration(targetOptions, reporter); return skyframeExecutor.createConfiguration(reporter, targetOptions, false); }