C++: Introduce flag for forcing rules through macro

The flag `--incompatible_load_cc_rules_from_bzl` will be flipped for Bazel 1.0. GitHub Issue: #8743 RELNOTES:none PiperOrigin-RevId: 259285337
bazelbuild · Jul 22, 2019 · 8daa514 · 8daa514
1 parent 3b10c59
commit 8daa514
Show file tree

Hide file tree

Showing 27 changed files with 398 additions and 10 deletions.
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
@@ -268,7 +268,7 @@ public ConfiguredTarget create(RuleContext context)
 
   public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake)
       throws InterruptedException, RuleErrorException, ActionConflictException {
-
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     semantics.validateDeps(ruleContext);
     if (ruleContext.hasErrors()) {
       return null;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -1061,4 +1061,31 @@ public static Map<String, NestedSet<Artifact>> createSaveFeatureStateArtifacts(
     }
     return outputGroupsBuilder.build();
   }
+
+  public static void checkRuleLoadedThroughMacro(RuleContext ruleContext)
+      throws RuleErrorException {
+    if (!ruleContext.getFragment(CppConfiguration.class).loadCcRulesFromBzl()) {
+      return;
+    }
+
+    if (!hasValidTag(ruleContext) || !ruleContext.getRule().wasCreatedByMacro()) {
+      registerMigrationRuleError(ruleContext);
+    }
+  }
+
+  private static boolean hasValidTag(RuleContext ruleContext) {
+    return ruleContext
+        .attributes()
+        .get("tags", Type.STRING_LIST)
+        .contains("__CC_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__");
+  }
+
+  private static void registerMigrationRuleError(RuleContext ruleContext)
+      throws RuleErrorException {
+    ruleContext.ruleError(
+        "The native C++/Objc rules are deprecated. Please load "
+            + ruleContext.getRule().getRuleClass()
+            + " from the rules_cc repository."
+            + " See http://github.com/bazelbuild/rules_cc.");
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImport.java
@@ -71,6 +71,7 @@ private static NoPicAndPicStaticLibrary create(@Nullable Artifact staticLibrary)
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
 
     boolean systemProvided = ruleContext.attributes().get("system_provided", Type.BOOLEAN);
     CcToolchainProvider ccToolchain =

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -111,7 +111,7 @@ public static void init(
       boolean linkStatic,
       boolean addDynamicRuntimeInputArtifactsToRunfiles)
       throws RuleErrorException, InterruptedException {
-
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     semantics.validateDeps(ruleContext);
     if (ruleContext.hasErrors()) {
       return;

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
@@ -45,6 +45,9 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
+    if (!isAppleToolchain()) {
+      CcCommon.checkRuleLoadedThroughMacro(ruleContext);
+    }
     validateToolchain(ruleContext);
     CcToolchainAttributesProvider attributes =
         new CcToolchainAttributesProvider(

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuite.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuite.java
@@ -42,6 +42,7 @@ public class CcToolchainSuite implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
 
     String transformedCpu = cppConfiguration.getTransformedCpuFromOptions();

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -694,4 +694,8 @@ public boolean useSpecificToolFiles() {
   public boolean disableNoCopts() {
     return cppOptions.disableNoCopts;
   }
+
+  public boolean loadCcRulesFromBzl() {
+    return cppOptions.loadCcRulesFromBzl;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -904,6 +904,20 @@ public Label getFdoPrefetchHintsLabel() {
               + " https://github.com/bazelbuild/bazel/issues/8706 for details.")
   public boolean disableNoCopts;
 
+  @Option(
+      name = "incompatible_load_cc_rules_from_bzl",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "If enabled, direct usage of the native C++ and some Objc rules is disabled. Please use "
+              + "the Starlark rules instead https://github.com/bazelbuild/rules_cc")
+  public boolean loadCcRulesFromBzl;
+
   @Override
   public FragmentOptions getHost() {
     CppOptions host = (CppOptions) getDefault();
@@ -961,6 +975,7 @@ public FragmentOptions getHost() {
     host.useSpecificToolFiles = useSpecificToolFiles;
     host.disableStaticCcToolchains = disableStaticCcToolchains;
     host.disableNoCopts = disableNoCopts;
+    host.loadCcRulesFromBzl = loadCcRulesFromBzl;
 
     // Save host options for further use.
     host.hostCoptList = hostCoptList;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoPrefetchHints.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoPrefetchHints.java
@@ -28,6 +28,7 @@ public final class FdoPrefetchHints implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws RuleErrorException, ActionConflictException {
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
 
     FdoInputFile inputFile = FdoInputFile.fromProfileRule(ruleContext);
     if (ruleContext.hasErrors()) {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoProfile.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoProfile.java
@@ -30,7 +30,7 @@ public final class FdoProfile implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws RuleErrorException, ActionConflictException {
-
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     FdoInputFile inputFile = FdoInputFile.fromProfileRule(ruleContext);
     if (ruleContext.hasErrors()) {
       return null;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibrary.java
@@ -24,14 +24,15 @@
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.rules.cpp.CcCommon;
 import com.google.devtools.build.lib.rules.cpp.CcSkylarkApiProvider;
 
 /** Part of the implementation of cc_proto_library. */
 public class CcProtoLibrary implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
-
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     if (ruleContext.getPrerequisites("deps", TARGET).size() != 1) {
       ruleContext.throwWithAttributeError(
           "deps",

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java
@@ -21,6 +21,7 @@
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.rules.cpp.CcCommon;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.syntax.Type;
 
@@ -31,6 +32,7 @@ public class ObjcImport implements RuleConfiguredTargetFactory {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     ObjcCommon common =
         new ObjcCommon.Builder(ruleContext)
             .setCompilationAttributes(

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImportRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImportRule.java
@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
 import com.google.devtools.build.lib.util.FileType;
 
@@ -32,16 +33,17 @@ public class ObjcImportRule implements RuleDefinition {
   @Override
   public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
     return builder
-        .requiresConfigurationFragments(ObjcConfiguration.class, AppleConfiguration.class,
-            AppleConfiguration.class)
+        .requiresConfigurationFragments(
+            ObjcConfiguration.class,
+            AppleConfiguration.class,
+            AppleConfiguration.class,
+            CppConfiguration.class)
         /* <!-- #BLAZE_RULE(objc_import).ATTRIBUTE(archives) -->
         The list of <code>.a</code> files provided to Objective-C targets that
         depend on this target.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("archives", LABEL_LIST)
-            .mandatory()
-            .nonEmpty()
-            .allowedFileTypes(FileType.of(".a")))
+        .add(
+            attr("archives", LABEL_LIST).mandatory().nonEmpty().allowedFileTypes(FileType.of(".a")))
         .addRequiredToolchains(CppRuleClasses.ccToolchainTypeAttribute(environment))
         .build();
   }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
@@ -25,6 +25,7 @@
 import com.google.devtools.build.lib.analysis.skylark.SymbolGenerator;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.rules.cpp.CcCommon;
 import com.google.devtools.build.lib.rules.cpp.CcCompilationContext;
 import com.google.devtools.build.lib.rules.cpp.CcInfo;
 import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
@@ -60,6 +61,7 @@ private ObjcCommon common(RuleContext ruleContext) throws InterruptedException {
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException, ActionConflictException {
+    CcCommon.checkRuleLoadedThroughMacro(ruleContext);
     validateAttributes(ruleContext);
 
     ObjcCommon common = common(ruleContext);

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -257,6 +257,7 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
     config.create(
         "/bazel_tools_workspace/tools/launcher/BUILD",
         "package(default_visibility=['//visibility:public'])",
+        "load('@bazel_tools//third_party/cc_rules/macros:defs.bzl', 'cc_binary')",
         "cc_binary(name='launcher', srcs=['launcher_main.cc'])");
 
     config.create(

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockCcSupport.java
@@ -70,6 +70,7 @@ protected ImmutableList<String> getCrosstoolArchs() {
 
   @Override
   public void setup(MockToolsConfig config) throws IOException {
+    writeMacroFile(config);
     setupCcToolchainConfig(config);
     MockPlatformSupport.setup(config);
   }

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/Crosstool.java b/src/test/java/com/google/devtools/build/lib/packages/util/Crosstool.java
@@ -464,6 +464,10 @@ public void write() throws IOException {
                 "licenses(['restricted'])",
                 "",
                 "load(':cc_toolchain_config.bzl', 'cc_toolchain_config')",
+                "load('"
+                    + TestConstants.TOOLS_REPOSITORY
+                    + "//third_party/cc_rules/macros:defs.bzl', 'cc_library', 'cc_toolchain',"
+                    + " 'cc_toolchain_suite')",
                 "toolchain_type(name = 'toolchain_type')",
                 "cc_toolchain_alias(name = 'current_cc_toolchain')",
                 "alias(name = 'toolchain', actual = 'everything')",
@@ -533,6 +537,10 @@ public void writeOSX() throws IOException {
             .add(
                 "package(default_visibility=['//visibility:public'])",
                 "load(':cc_toolchain_config.bzl', 'cc_toolchain_config')",
+                "load('"
+                    + TestConstants.TOOLS_REPOSITORY
+                    + "//third_party/cc_rules/macros:defs.bzl', 'cc_library',"
+                    + " 'cc_toolchain_suite')",
                 "exports_files(glob(['**']))",
                 "cc_toolchain_suite(",
                 "    name = 'crosstool',",

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.packages.util;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
@@ -32,6 +33,7 @@
 import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
+import java.util.List;
 
 /**
  * Creates mock BUILD files required for the C/C++ rules.
@@ -255,4 +257,58 @@ protected String readCcToolchainConfigFile() throws IOException {
   public final Predicate<Label> labelFilter() {
     return ccLabelFilter;
   }
+
+  public void writeMacroFile(MockToolsConfig config) throws IOException {
+    List<String> ruleNames =
+        ImmutableList.of(
+            "cc_library",
+            "cc_binary",
+            "cc_test",
+            "cc_import",
+            "objc_import",
+            "objc_library",
+            "cc_toolchain",
+            "cc_toolchain_suite",
+            "fdo_profile",
+            "fdo_prefetch_hints",
+            "cc_proto_library");
+    config.create(TestConstants.TOOLS_REPOSITORY_SCRATCH + "third_party/cc_rules/macros/BUILD", "");
+
+    StringBuilder macros = new StringBuilder();
+    for (String ruleName : ruleNames) {
+      Joiner.on("\n")
+          .appendTo(
+              macros,
+              "def " + ruleName + "(**attrs):",
+              "    if 'tags' in attrs and attrs['tags'] != None:",
+              "        attrs['tags'] = attrs['tags'] +"
+                  + " ['__CC_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__']",
+              "    else:",
+              "        attrs['tags'] = ['__CC_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__']",
+              "    native." + ruleName + "(**attrs)");
+      macros.append("\n");
+    }
+    config.create(
+        TestConstants.TOOLS_REPOSITORY_SCRATCH + "third_party/cc_rules/macros/defs.bzl",
+        macros.toString());
+  }
+
+  public String getMacroLoadStatement(boolean loadMacro, String... ruleNames) {
+    if (!loadMacro) {
+      return "";
+    }
+    Preconditions.checkState(ruleNames.length > 0);
+    StringBuilder loadStatement =
+        new StringBuilder()
+            .append("load('")
+            .append(TestConstants.TOOLS_REPOSITORY)
+            .append("//third_party/cc_rules/macros:defs.bzl', ");
+    ImmutableList.Builder<String> quotedRuleNames = ImmutableList.builder();
+    for (String ruleName : ruleNames) {
+      quotedRuleNames.add(String.format("'%s'", ruleName));
+    }
+    Joiner.on(",").appendTo(loadStatement, quotedRuleNames.build());
+    loadStatement.append(")");
+    return loadStatement.toString();
+  }
 }