Use the cc_toolchain rule instead of an event bus-based hack to signal an error when the LIPO context is not a cc_binary.

RELNOTES[NEW]: LIPO context (--lipo_context) can now also be a cc_test (in addition to cc_binary)

--
MOS_MIGRATED_REVID=124555465

7 files changed, 75 insertions, 54 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
index 3564564dfa..6fc4476782 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
@@ -30,11 +30,11 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations;
-import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses.CppTransition;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
 import com.google.devtools.build.lib.packages.Attribute.Transition;
+import com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LipoTransition;
 
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -192,8 +192,9 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
     // implementing a way for the C++ rules to contribute this transition to the configuration
     // collection.
     for (BuildConfiguration config : allConfigurations) {
-      transitionBuilder.put(config, CppTransition.LIPO_COLLECTOR, new ConfigurationHolder(config));
-      transitionBuilder.put(config, CppTransition.TARGET_CONFIG_FOR_LIPO,
+      transitionBuilder.put(config, LipoTransition.LIPO_COLLECTOR,
+          new ConfigurationHolder(config));
+      transitionBuilder.put(config, LipoTransition.TARGET_CONFIG_FOR_LIPO,
           new ConfigurationHolder(config.isHostConfiguration() ? null : config));
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
index 800b95ad88..3208dadc57 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCppRuleClasses.java
@@ -47,7 +47,6 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
-import com.google.devtools.build.lib.packages.Attribute.Transition;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
@@ -63,8 +62,8 @@ import com.google.devtools.build.lib.rules.cpp.CcIncLibraryRule;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
 import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
+import com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LipoTransition;
 import com.google.devtools.build.lib.util.FileTypeSet;
-import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
 
 /**
  * Rule class definitions for C++ rules.
@@ -98,28 +97,6 @@ public class BazelCppRuleClasses {
         "objc_library",
       };
 
-  /**
-   * Miscellaneous configuration transitions. It would be better not to have this - please don't add
-   * to it.
-   */
-  public static enum CppTransition implements Transition {
-    /**
-     * The configuration for LIPO information collection. Requesting this from a configuration that
-     * does not have lipo optimization enabled may result in an exception.
-     */
-    LIPO_COLLECTOR,
-
-    /**
-     * The corresponding (target) configuration.
-     */
-    TARGET_CONFIG_FOR_LIPO;
-
-    @Override
-    public boolean defaultsToSelf() {
-      return false;
-    }
-  }
-
   private static final RuleClass.Configurator<BuildConfiguration, Rule> LIPO_ON_DEMAND =
       new RuleClass.Configurator<BuildConfiguration, Rule>() {
     @Override
@@ -132,7 +109,7 @@ public class BazelCppRuleClasses {
         return configuration;
       }
       BuildConfiguration toplevelConfig =
-          configuration.getConfiguration(CppTransition.TARGET_CONFIG_FOR_LIPO);
+          configuration.getConfiguration(LipoTransition.TARGET_CONFIG_FOR_LIPO);
       // If LIPO is enabled, override the default configuration.
       if (toplevelConfig != null
           && toplevelConfig.getFragment(CppConfiguration.class).isLipoOptimization()
@@ -187,23 +164,6 @@ public class BazelCppRuleClasses {
       };
 
   /**
-   * Implementation for the :lipo_context_collector attribute.
-   */
-  public static final LateBoundLabel<BuildConfiguration> LIPO_CONTEXT_COLLECTOR =
-      new LateBoundLabel<BuildConfiguration>() {
-    @Override
-    public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
-      // This attribute connects a target to the LIPO context target configured with the
-      // lipo input collector configuration.
-      CppConfiguration cppConfiguration = configuration.getFragment(CppConfiguration.class);
-      return !cppConfiguration.isLipoContextCollector()
-          && (cppConfiguration.getLipoMode() == LipoMode.BINARY)
-          ? cppConfiguration.getLipoContextLabel()
-          : null;
-    }
-  };
-
-  /**
    * Returns the STL prerequisite of the rule.
    *
    * <p>If rule has an implicit $stl_default attribute returns STL version set on the
@@ -316,8 +276,8 @@ public class BazelCppRuleClasses {
           <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
           .add(attr("includes", STRING_LIST))
           .add(attr(":lipo_context_collector", LABEL)
-              .cfg(CppTransition.LIPO_COLLECTOR)
-              .value(LIPO_CONTEXT_COLLECTOR)
+              .cfg(LipoTransition.LIPO_COLLECTOR)
+              .value(CppRuleClasses.LIPO_CONTEXT_COLLECTOR)
               .skipPrereqValidatorCheck())
           .build();
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 541363d7e0..3df4a4ea3f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -36,7 +36,6 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
-import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.apple.Platform;
@@ -152,11 +151,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext context)
       throws InterruptedException, RuleErrorException {
-    return CcBinary.init(semantics, context, /*fake =*/ false, /*useTestOnlyFlags =*/ false);
+    return CcBinary.init(semantics, context, /*fake =*/ false, /*isTest =*/ false);
   }
 
   public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake,
-      boolean useTestOnlyFlags) throws InterruptedException {
+      boolean isTest) throws InterruptedException {
     ruleContext.checkSrcsSamePackage(true);
     FeatureConfiguration featureConfiguration = CcCommon.configureFeatures(ruleContext);
     CcCommon common = new CcCommon(ruleContext);
@@ -200,7 +199,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
             binary,
             linkStaticness,
             linkopts);
-    linkActionBuilder.setUseTestOnlyFlags(useTestOnlyFlags);
+    linkActionBuilder.setUseTestOnlyFlags(isTest);
     linkActionBuilder.addNonLibraryInputs(ccCompilationOutputs.getHeaderTokenFiles());
 
     CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext);
@@ -355,7 +354,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
           ExecutionInfoProvider.class,
           new ExecutionInfoProvider(ImmutableMap.of("requires-darwin", "")));
     }
-    
+
     return ruleBuilder
         .add(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
         .add(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java
index e4d312f833..acfb5c33ed 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcTest.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 
 /**
@@ -33,6 +32,6 @@ public abstract class CcTest implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext context)
       throws InterruptedException, RuleErrorException {
-    return CcBinary.init(semantics, context, /*fake =*/ false, /*useTestOnlyFlags =*/ true);
+    return CcBinary.init(semantics, context, /*fake =*/ false, /*isTest =*/ true);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index a50b71a33d..843711b39c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -63,6 +63,14 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
 
   @Override
   public ConfiguredTarget create(RuleContext ruleContext) throws RuleErrorException {
+    TransitiveInfoCollection lipoContextCollector =
+        ruleContext.getPrerequisite(":lipo_context_collector", Mode.DONT_CHECK);
+    if (lipoContextCollector != null
+        && lipoContextCollector.getProvider(LipoContextProvider.class) == null) {
+      ruleContext.ruleError("--lipo_context must point to a cc_binary or a cc_test rule");
+      return null;
+    }
+
     CppConfiguration cppConfiguration =
         Preconditions.checkNotNull(ruleContext.getFragment(CppConfiguration.class));
     Path fdoZip = ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
index a795d18838..9ee448165f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.rules.cpp.CppRuleClasses.LipoTransition;
 
 /**
  * Rule definition for compiler definition.
@@ -76,6 +77,10 @@ public final class CcToolchainRule implements RuleDefinition {
         .add(attr("supports_header_parsing", BOOLEAN).value(false))
         // TODO(bazel-team): Should be using the TARGET configuration.
         .add(attr(":libc_top", LABEL).cfg(HOST).value(LIBC_TOP))
+        .add(attr(":lipo_context_collector", LABEL)
+            .cfg(LipoTransition.LIPO_COLLECTOR)
+            .value(CppRuleClasses.LIPO_CONTEXT_COLLECTOR)
+            .skipPrereqValidatorCheck())
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index d8987bb8ec..7c9aa05675 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -30,14 +30,63 @@ import static com.google.devtools.build.lib.rules.cpp.CppFileTypes.SHARED_LIBRAR
 import static com.google.devtools.build.lib.rules.cpp.CppFileTypes.VERSIONED_SHARED_LIBRARY;
 
 import com.google.devtools.build.lib.analysis.LanguageDependentFragment.LibraryLanguage;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
+import com.google.devtools.build.lib.packages.Attribute.Transition;
+import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
+import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.InstrumentationSpec;
 import com.google.devtools.build.lib.util.FileTypeSet;
+import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode;
 
 /**
  * Rule class definitions for C++ rules.
  */
 public class CppRuleClasses {
+
+  /**
+   * Implementation for the :lipo_context_collector attribute.
+   */
+  public static final LateBoundLabel<BuildConfiguration> LIPO_CONTEXT_COLLECTOR =
+      new LateBoundLabel<BuildConfiguration>() {
+    @Override
+    public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
+      // This attribute connects a target to the LIPO context target configured with the
+      // lipo input collector configuration.
+      CppConfiguration cppConfiguration = configuration.getFragment(CppConfiguration.class);
+      return !cppConfiguration.isLipoContextCollector()
+          && (cppConfiguration.getLipoMode() == LipoMode.BINARY)
+          ? cppConfiguration.getLipoContextLabel()
+          : null;
+    }
+  };
+
+  /**
+   * Configuration transitions required by LIPO.
+   */
+  public enum LipoTransition implements Transition {
+    /**
+     * LIPO context collector.
+     *
+     * <p>This configuration transition leads into a configuration that is used for collecting
+     * C++ compilation contexts for LIPO builds so that e.g. an include path entry required by an
+     * inlined function is there when the place is compiled where it is inlined at.
+     */
+    LIPO_COLLECTOR,
+
+    /**
+     * Transition used for switching back to the LIPO-optimized configuration.
+     */
+    TARGET_CONFIG_FOR_LIPO;
+
+    @Override
+    public boolean defaultsToSelf() {
+      return true;
+    }
+  }
+
   // Artifacts of these types are discarded from the 'hdrs' attribute in cc rules
   static final FileTypeSet DISALLOWED_HDRS_FILES = FileTypeSet.of(
       ARCHIVE,