Added new RuleClass flag to turn off platform support, to stop depend…

…ency cycles when loading platforms.

Part of bazelbuild#4128.

Change-Id: Ie55a91aaaec15d8eb537f59131fc2e69a8f9c251
PiperOrigin-RevId: 176509311

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

@@ -228,9 +228,6 @@ public final ConfiguredTarget createConfiguredTarget(
       @Nullable ToolchainContext toolchainContext)
       throws InterruptedException {
     if (target instanceof Rule) {
-      Preconditions.checkArgument(
-          toolchainContext != null,
-          "ToolchainContext should never be null when creating a ConfiguredTarget for a Rule");
       try {
         CurrentRuleTracker.beginConfiguredTarget(((Rule) target).getRuleClassObject());
         return createRule(
@@ -302,11 +299,13 @@ private ConfiguredTarget createRule(
       BuildConfiguration hostConfiguration,
       OrderedSetMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       ImmutableMap<Label, ConfigMatchingProvider> configConditions,
-      ToolchainContext toolchainContext)
+      @Nullable ToolchainContext toolchainContext)
       throws InterruptedException {
 
     // Load the requested toolchains into the ToolchainContext.
-    toolchainContext.resolveToolchains(prerequisiteMap);
+    if (toolchainContext != null) {
+      toolchainContext.resolveToolchains(prerequisiteMap);
+    }
 
     // Visibility computation and checking is done for every rule.
     RuleContext ruleContext =
@@ -409,13 +408,15 @@ public ConfiguredAspect createAspect(
       Aspect aspect,
       OrderedSetMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
       ImmutableMap<Label, ConfigMatchingProvider> configConditions,
-      ToolchainContext toolchainContext,
+      @Nullable ToolchainContext toolchainContext,
       BuildConfiguration aspectConfiguration,
       BuildConfiguration hostConfiguration)
       throws InterruptedException {
 
     // Load the requested toolchains into the ToolchainContext.
-    toolchainContext.resolveToolchains(prerequisiteMap);
+    if (toolchainContext != null) {
+      toolchainContext.resolveToolchains(prerequisiteMap);
+    }
 
     RuleContext.Builder builder = new RuleContext.Builder(
         env,

src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java

@@ -84,6 +84,7 @@ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
             attr("path", STRING)
                 .undocumented(
                     "only used to expose FilegroupPathProvider, which is not currently used"))
+        .supportsPlatforms(false)
         .build();
   }
 

src/main/java/com/google/devtools/build/lib/packages/RuleClass.java

@@ -486,6 +486,7 @@ public Transition buildTransitionFor(Rule rule) {
 
     private final Map<String, Attribute> attributes = new LinkedHashMap<>();
     private final Set<Label> requiredToolchains = new HashSet<>();
+    private boolean supportsPlatforms = true;
 
     /**
      * Constructs a new {@code RuleClassBuilder} using all attributes from all
@@ -517,6 +518,7 @@ public Builder(String name, RuleClassType type, boolean skylark, RuleClass... pa
         supportsConstraintChecking = parent.supportsConstraintChecking;
 
         addRequiredToolchains(parent.getRequiredToolchains());
+        supportsPlatforms = parent.supportsPlatforms;
 
         for (Attribute attribute : parent.getAttributes()) {
           String attrName = attribute.getName();
@@ -599,6 +601,7 @@ public RuleClass build(String name, String key) {
           configurationFragmentPolicy.build(),
           supportsConstraintChecking,
           requiredToolchains,
+          supportsPlatforms,
           attributes.values().toArray(new Attribute[0]));
     }
 
@@ -1010,6 +1013,11 @@ public Builder addRequiredToolchains(Label... toolchainLabels) {
       return this;
     }
 
+    public Builder supportsPlatforms(boolean flag) {
+      this.supportsPlatforms = flag;
+      return this;
+    }
+
     /**
      * Returns an Attribute.Builder object which contains a replica of the
      * same attribute in the parent rule if exists.
@@ -1132,6 +1140,7 @@ public Attribute.Builder<?> copy(String name) {
   private final boolean supportsConstraintChecking;
 
   private final ImmutableSet<Label> requiredToolchains;
+  private final boolean supportsPlatforms;
 
   /**
    * Constructs an instance of RuleClass whose name is 'name', attributes are 'attributes'. The
@@ -1181,6 +1190,7 @@ public Attribute.Builder<?> copy(String name) {
       ConfigurationFragmentPolicy configurationFragmentPolicy,
       boolean supportsConstraintChecking,
       Set<Label> requiredToolchains,
+      boolean supportsPlatforms,
       Attribute... attributes) {
     this.name = name;
     this.key = key;
@@ -1211,6 +1221,7 @@ public Attribute.Builder<?> copy(String name) {
     this.configurationFragmentPolicy = configurationFragmentPolicy;
     this.supportsConstraintChecking = supportsConstraintChecking;
     this.requiredToolchains = ImmutableSet.copyOf(requiredToolchains);
+    this.supportsPlatforms = supportsPlatforms;
 
     // Create the index and collect non-configurable attributes.
     int index = 0;
@@ -2031,6 +2042,10 @@ public ImmutableSet<Label> getRequiredToolchains() {
     return requiredToolchains;
   }
 
+  public boolean supportsPlatforms() {
+    return supportsPlatforms;
+  }
+
   public static boolean isThirdPartyPackage(PackageIdentifier packageIdentifier) {
     if (!packageIdentifier.getRepository().isMain()) {
       return false;

src/main/java/com/google/devtools/build/lib/rules/platform/PlatformBaseRule.java

@@ -39,6 +39,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
                 .value(ImmutableList.of("manual"))
                 .nonconfigurable("low-level attribute, used in platform configuration"))
         .exemptFromConstraintChecking("this rule helps *define* a constraint")
+        .supportsPlatforms(false)
         .removeAttribute("deps")
         .removeAttribute("data")
         .build();

src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java

@@ -208,12 +208,15 @@ public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunc
       // Determine what toolchains are needed by this target.
       if (target instanceof Rule) {
         Rule rule = ((Rule) target);
-        ImmutableSet<Label> requiredToolchains = rule.getRuleClassObject().getRequiredToolchains();
-        toolchainContext =
-            ToolchainUtil.createToolchainContext(
-                env, rule.toString(), requiredToolchains, configuration);
-        if (env.valuesMissing()) {
-          return null;
+        if (rule.getRuleClassObject().supportsPlatforms()) {
+          ImmutableSet<Label> requiredToolchains =
+              rule.getRuleClassObject().getRequiredToolchains();
+          toolchainContext =
+              ToolchainUtil.createToolchainContext(
+                  env, rule.toString(), requiredToolchains, configuration);
+          if (env.valuesMissing()) {
+            return null;
+          }
         }
       }
 

src/main/java/com/google/devtools/build/lib/skyframe/ToolchainUtil.java

@@ -114,6 +114,9 @@ private static PlatformDescriptors loadPlatformDescriptors(
       throws InterruptedException, ToolchainContextException {
     PlatformConfiguration platformConfiguration =
         configuration.getFragment(PlatformConfiguration.class);
+    if (platformConfiguration == null) {
+      return null;
+    }
     Label executionPlatformLabel = platformConfiguration.getExecutionPlatform();
     Label targetPlatformLabel = platformConfiguration.getTargetPlatforms().get(0);
 

src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java

@@ -890,6 +890,7 @@ private static RuleClass newRuleClass(
             .build(),
         supportsConstraintChecking,
         /*requiredToolchains=*/ ImmutableSet.<Label>of(),
+        /*supportsPlatforms=*/ true,
         attributes);
   }