Handle platform mapping errors in BuildConfigurationKeyProducer.

Work towards platform-based flags: #19409. PiperOrigin-RevId: 598954569 Change-Id: I29056b09ebdb898fee4a1ec1621fb01b8d8d8f09
bazelbuild · Jan 16, 2024 · 10082f3 · 10082f3
1 parent 4a6f896
commit 10082f3
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 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
@@ -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/.../java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java b/.../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 @@
  * <p>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<SkyValue> {
+public class BuildConfigurationKeyProducer
+    implements StateMachine, ValueOrExceptionSink<PlatformMappingException> {
 
   /** Interface for clients to accept results of this computation. */
   public interface ResultSink {
 
     void acceptTransitionError(OptionsParsingException e);
 
+    void acceptPlatformMappingError(PlatformMappingException e);
+
     void acceptTransitionedConfigurations(
         ImmutableMap<String, BuildConfigurationKey> 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
@@ -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
@@ -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/...java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java b/...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
@@ -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
@@ -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<DependencyKind, ConfiguredTargetAndData> 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
@@ -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<String, BuildConfigurationKey> 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<String, BuildConfigurationKey> transitionedOptions) {
@@ -166,14 +174,17 @@ public void acceptTransitionedConfigurations(
 
     @Nullable
     private BuildConfigurationKey drive(LookupEnvironment env)
-        throws OptionsParsingException, InterruptedException {
+        throws OptionsParsingException, InterruptedException, PlatformMappingException {
       if (!this.driver.drive(env)) {
         return null;
       }
 
       if (this.transitionError != null) {
         throw this.transitionError;
       }
+      if (this.platformMappingException != null) {
+        throw this.platformMappingException;
+      }
 
       return this.transitionedOptions.get(TRANSITION_KEY);
     }