From 10082f375913c98479ac53bd18de860bdc2ae6e0 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 16 Jan 2024 14:15:56 -0800 Subject: [PATCH] Handle platform mapping errors in BuildConfigurationKeyProducer. Work towards platform-based flags: #19409. PiperOrigin-RevId: 598954569 Change-Id: I29056b09ebdb898fee4a1ec1621fb01b8d8d8f09 --- .../build/lib/analysis/producers/BUILD | 3 +++ .../BuildConfigurationKeyProducer.java | 27 +++++++++++++------ .../analysis/producers/DependencyError.java | 11 ++++++++ .../producers/DependencyProducer.java | 6 +++++ .../TargetAndConfigurationProducer.java | 25 ++++++++++++----- .../google/devtools/build/lib/skyframe/BUILD | 1 + .../lib/skyframe/DependencyResolver.java | 4 +++ .../config/BaselineOptionsFunction.java | 13 ++++++++- 8 files changed, 74 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD index ad85f12845bfc7..47a931c34dabb3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD @@ -74,6 +74,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:package_value", "//src/main/java/com/google/devtools/build/lib/skyframe:rule_configured_target_value", "//src/main/java/com/google/devtools/build/lib/skyframe/config", + "//src/main/java/com/google/devtools/build/lib/skyframe/config:exceptions", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:no_matching_platform_exception", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util", "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_key", @@ -100,10 +101,12 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options", "//src/main/java/com/google/devtools/build/lib/analysis:platform_options", "//src/main/java/com/google/devtools/build/lib/skyframe/config", + "//src/main/java/com/google/devtools/build/lib/skyframe/config:exceptions", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/com/google/devtools/common/options", "//third_party:guava", + "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java index 189e038095649e..035f9bf540fc24 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java @@ -17,14 +17,16 @@ import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; +import com.google.devtools.build.lib.skyframe.config.PlatformMappingException; import com.google.devtools.build.lib.skyframe.config.PlatformMappingValue; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.state.StateMachine; +import com.google.devtools.build.skyframe.state.StateMachine.ValueOrExceptionSink; import com.google.devtools.common.options.OptionsParsingException; import java.util.Map; import java.util.Optional; -import java.util.function.Consumer; +import javax.annotation.Nullable; /** * Creates the needed {@link BuildConfigurationKey} instances for the given options. @@ -34,13 +36,16 @@ *

The output preserves the iteration order of the input. */ // Logic here must be kept in sync with SkyframeExecutor.createBuildConfigurationKey. -public class BuildConfigurationKeyProducer implements StateMachine, Consumer { +public class BuildConfigurationKeyProducer + implements StateMachine, ValueOrExceptionSink { /** Interface for clients to accept results of this computation. */ public interface ResultSink { void acceptTransitionError(OptionsParsingException e); + void acceptPlatformMappingError(PlatformMappingException e); + void acceptTransitionedConfigurations( ImmutableMap transitionedOptions); } @@ -73,17 +78,23 @@ public StateMachine step(Tasks tasks) { .findAny(); PlatformMappingValue.Key platformMappingValueKey = PlatformMappingValue.Key.create(platformMappingsPath.orElse(null)); - // No need to consider error handling: this flag cannot be changed and so errors were - // detected and handled in SkyframeExecutor.createBuildConfigurationKey. - tasks.lookUp(platformMappingValueKey, this); + tasks.lookUp(platformMappingValueKey, PlatformMappingException.class, this); return this::applyMappings; } @Override - public void accept(SkyValue skyValue) { - if (skyValue instanceof PlatformMappingValue) { - this.platformMappingValue = (PlatformMappingValue) skyValue; + public void acceptValueOrException( + @Nullable SkyValue value, @Nullable PlatformMappingException exception) { + if (exception != null) { + sink.acceptPlatformMappingError(exception); + return; + } + if (value instanceof PlatformMappingValue) { + this.platformMappingValue = (PlatformMappingValue) value; + return; } + + throw new IllegalStateException("No value or exception was provided"); } private StateMachine applyMappings(Tasks tasks) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyError.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyError.java index b6a9fdc1d0acb2..cccf5396b0baee 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyError.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyError.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException; import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition.TransitionException; import com.google.devtools.build.lib.skyframe.AspectCreationException; +import com.google.devtools.build.lib.skyframe.config.PlatformMappingException; import com.google.devtools.common.options.OptionsParsingException; /** Tagged union of exceptions thrown by {@link DependencyProducer}. */ @@ -37,6 +38,8 @@ public enum Kind { ASPECT_EVALUATION, /** An error occurred during evaluation of the aspect using Skyframe. */ ASPECT_CREATION, + /** An error occurred during evaluation of platform mappings. */ + PLATFORM_MAPPING, } public abstract Kind kind(); @@ -51,6 +54,8 @@ public enum Kind { public abstract AspectCreationException aspectCreation(); + public abstract PlatformMappingException platformMapping(); + public static boolean isSecondErrorMoreImportant(DependencyError first, DependencyError second) { // There isn't a good way to prioritize when the type matches, so we just keep the first. return first.kind().compareTo(second.kind()) > 0; @@ -68,6 +73,8 @@ public Exception getException() { return aspectEvaluation(); case ASPECT_CREATION: return aspectCreation(); + case PLATFORM_MAPPING: + return platformMapping(); } throw new IllegalStateException("unreachable"); } @@ -91,4 +98,8 @@ static DependencyError of(DependencyEvaluationException e) { static DependencyError of(AspectCreationException e) { return AutoOneOf_DependencyError.aspectCreation(e); } + + static DependencyError of(PlatformMappingException e) { + return AutoOneOf_DependencyError.platformMapping(e); + } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java index 449d2c647ddf36..ffd44d71b20b71 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.skyframe.ConfiguredValueCreationException; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; +import com.google.devtools.build.lib.skyframe.config.PlatformMappingException; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.state.StateMachine; import com.google.devtools.common.options.OptionsParsingException; @@ -201,6 +202,11 @@ public void acceptTransitionError(OptionsParsingException e) { getMessageWithEdgeTransitionInfo(e), e.getInvalidArgument(), e))); } + @Override + public void acceptPlatformMappingError(PlatformMappingException e) { + sink.acceptDependencyError(DependencyError.of(e)); + } + private String getMessageWithEdgeTransitionInfo(Throwable e) { return String.format( "On dependency edge %s (%s) -|%s|-> %s: %s", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java index c587c6253dbfb1..63c3e8c197a2db 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java @@ -67,6 +67,7 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.ConfiguredValueCreationException; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; +import com.google.devtools.build.lib.skyframe.config.PlatformMappingException; import com.google.devtools.build.lib.skyframe.toolchains.PlatformLookupUtil.InvalidPlatformException; import com.google.devtools.build.lib.skyframe.toolchains.ToolchainContextKey; import com.google.devtools.build.lib.util.DetailedExitCode; @@ -451,7 +452,7 @@ public void acceptConfigConditions(ConfigConditions configConditions) { @Override public void acceptConfigConditionsError(ConfiguredValueCreationException e) { - emitTransitionErrorMessage(e.getMessage()); + emitErrorMessage(e.getMessage()); } public StateMachine computeTransition(Tasks tasks) { @@ -512,12 +513,17 @@ public void acceptTransitionedConfigurations( @Override public void acceptTransitionError(TransitionException e) { - emitTransitionErrorMessage(e.getMessage()); + emitErrorMessage(e.getMessage()); } @Override public void acceptTransitionError(OptionsParsingException e) { - emitTransitionErrorMessage(e.getMessage()); + emitErrorMessage(e.getMessage()); + } + + @Override + public void acceptPlatformMappingError(PlatformMappingException e) { + emitErrorMessage(e.getMessage()); } @Override @@ -527,7 +533,7 @@ public void acceptPlatformInfo(PlatformInfo info) { @Override public void acceptPlatformInfoError(InvalidPlatformException error) { - emitTransitionErrorMessage(error.getMessage()); + emitErrorMessage(error.getMessage()); } private StateMachine processTransitionedKey(Tasks tasks) { @@ -605,12 +611,17 @@ public void acceptTransitionedConfigurations( @Override public void acceptTransitionError(TransitionException e) { - emitTransitionErrorMessage(e.getMessage()); + emitErrorMessage(e.getMessage()); } @Override public void acceptTransitionError(OptionsParsingException e) { - emitTransitionErrorMessage(e.getMessage()); + emitErrorMessage(e.getMessage()); + } + + @Override + public void acceptPlatformMappingError(PlatformMappingException e) { + emitErrorMessage(e.getMessage()); } private StateMachine checkIdempotencyAndDelegate(Tasks tasks) { @@ -634,7 +645,7 @@ private StateMachine checkIdempotencyAndDelegate(Tasks tasks) { } } - private void emitTransitionErrorMessage(String message) { + private void emitErrorMessage(String message) { emitError(message, TargetUtils.getLocationMaybe(target), /* exitCode= */ null); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 1f6de31b36bd27..33e40a6ee9bc64 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -318,6 +318,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/rules/genquery:genquery_package_providers", "//src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2:actiongraph_v2", "//src/main/java/com/google/devtools/build/lib/skyframe/config", + "//src/main/java/com/google/devtools/build/lib/skyframe/config:exceptions", "//src/main/java/com/google/devtools/build/lib/skyframe/config:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe/rewinding", "//src/main/java/com/google/devtools/build/lib/skyframe/rewinding:action_rewound_event", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/DependencyResolver.java index a4c0da94349aec..d8cdc47a69d638 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DependencyResolver.java @@ -79,6 +79,7 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetEvaluationExceptions.ReportedException; import com.google.devtools.build.lib.skyframe.ConfiguredTargetEvaluationExceptions.UnreportedException; import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey; +import com.google.devtools.build.lib.skyframe.config.PlatformMappingException; import com.google.devtools.build.lib.skyframe.toolchains.ToolchainContextKey; import com.google.devtools.build.lib.skyframe.toolchains.ToolchainException; import com.google.devtools.build.lib.skyframe.toolchains.UnloadedToolchainContext; @@ -703,6 +704,9 @@ public static OrderedSetMultimap comput throw error.aspectEvaluation(); case ASPECT_CREATION: throw error.aspectCreation(); + case PLATFORM_MAPPING: + PlatformMappingException e = error.platformMapping(); + throw new ConfiguredValueCreationException(ctgValue.getTarget(), e.getMessage()); } } if (!state.transitiveState.hasRootCause() && state.dependencyMap == 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 42f546ab8794e6..d6577bf762b38c 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 @@ -133,6 +133,8 @@ private static BuildOptions mapBuildOptions(Environment env, BuildOptions rawBas return null; } return key.getOptions(); + } catch (PlatformMappingException e) { + throw new BaselineOptionsFunctionException(e); } catch (OptionsParsingException e) { throw new BaselineOptionsFunctionException(e); } @@ -145,6 +147,7 @@ private static class BuildOptionsMapper implements BuildConfigurationKeyProducer private final Driver driver; private ImmutableMap transitionedOptions; private OptionsParsingException transitionError; + private PlatformMappingException platformMappingException; private BuildOptionsMapper(BuildOptions options) { this.driver = @@ -158,6 +161,11 @@ public void acceptTransitionError(OptionsParsingException e) { this.transitionError = e; } + @Override + public void acceptPlatformMappingError(PlatformMappingException e) { + this.platformMappingException = e; + } + @Override public void acceptTransitionedConfigurations( ImmutableMap transitionedOptions) { @@ -166,7 +174,7 @@ public void acceptTransitionedConfigurations( @Nullable private BuildConfigurationKey drive(LookupEnvironment env) - throws OptionsParsingException, InterruptedException { + throws OptionsParsingException, InterruptedException, PlatformMappingException { if (!this.driver.drive(env)) { return null; } @@ -174,6 +182,9 @@ private BuildConfigurationKey drive(LookupEnvironment env) if (this.transitionError != null) { throw this.transitionError; } + if (this.platformMappingException != null) { + throw this.platformMappingException; + } return this.transitionedOptions.get(TRANSITION_KEY); }