Enable switching def_parser's Windows target for remote execution

When --define EXECUTOR=remote is specified in bazel command, embedded
tools 'def_parser' will be compiled remotely from source.

Because def_parser itself is a cc_binary, if we want to compile it
remotely, to avoid cycle dependency it cannot be a dependency of
cc_toolchain. Therefore, we make it a dependency of cc rules.

Change-Id: I77faf77238f8edd3585d0e5e5c780b14e9782a40
PiperOrigin-RevId: 180534568

7 files changed, 49 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 5826e9987f..749f747f8c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -348,8 +348,7 @@ public class BazelRuleClassProvider {
           builder.addBuildInfoFactory(new CppBuildInfo());
           builder.addDynamicTransitionMaps(CppRuleClasses.DYNAMIC_TRANSITIONS_MAP);
 
-          builder.addRuleDefinition(
-              new CcToolchainRule("@bazel_tools//tools/def_parser:def_parser"));
+          builder.addRuleDefinition(new CcToolchainRule());
           builder.addRuleDefinition(new CcToolchainSuiteRule());
           builder.addRuleDefinition(new CcToolchainAlias.CcToolchainAliasRule());
           builder.addRuleDefinition(new CcIncLibraryRule());
@@ -358,7 +357,8 @@ public class BazelRuleClassProvider {
           builder.addRuleDefinition(new BazelCppRuleClasses.CcLinkingRule());
           builder.addRuleDefinition(new BazelCppRuleClasses.CcDeclRule());
           builder.addRuleDefinition(new BazelCppRuleClasses.CcBaseRule());
-          builder.addRuleDefinition(new BazelCppRuleClasses.CcRule());
+          builder.addRuleDefinition(
+              new BazelCppRuleClasses.CcRule(TOOLS_REPOSITORY + "//tools/def_parser:def_parser"));
           builder.addRuleDefinition(new BazelCppRuleClasses.CcBinaryBaseRule());
           builder.addRuleDefinition(new BazelCcBinaryRule());
           builder.addRuleDefinition(new BazelCcTestRule());
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 6ed5a96106..811f1c45ca 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
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses;
 import com.google.devtools.build.lib.analysis.PlatformConfiguration;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
@@ -242,8 +243,51 @@ public class BazelCppRuleClasses {
    * Helper rule class.
    */
   public static final class CcRule implements RuleDefinition {
+
+    /**
+     * The label points to the Windows object file parser. In bazel, it should be
+     * //tools/def_parser:def_parser, otherwise it should be null.
+     *
+     * <p>TODO(pcloudy): Remove this after Bazel rule definitions are not used internally anymore.
+     * Related bug b/63658220
+     */
+    private final String defParserLabel;
+
+    public CcRule(String defParserLabel) {
+      this.defParserLabel = defParserLabel;
+    }
+
+    public CcRule() {
+      this.defParserLabel = null;
+    }
+
     @Override
     public RuleClass build(Builder builder, final RuleDefinitionEnvironment env) {
+      if (defParserLabel != null) {
+        builder.add(
+            attr("$def_parser", LABEL)
+                .cfg(HostTransition.INSTANCE)
+                .singleArtifact()
+                .value(
+                    new Attribute.ComputedDefault() {
+                      @Override
+                      public Object getDefault(AttributeMap rule) {
+                        // Every cc_rule depends implicitly on def_parser tool.
+                        // The only exceptions are the rules for building def_parser itself.
+                        // To avoid cycles in the dependency graph, return null for rules under
+                        // @bazel_tools//third_party/def_parser and @bazel_tools//tools/cpp
+                        String label = rule.getLabel().toString();
+                        String toolsRepository = env.getToolsRepository();
+                        return label.startsWith(toolsRepository + "//third_party/def_parser")
+                                // @bazel_tools//tools/cpp:malloc and @bazel_tools//tools/cpp:stl
+                                // are implicit dependency of all cc rules,
+                                // thus a dependency of def_parser.
+                                || label.startsWith(toolsRepository + "//tools/cpp")
+                            ? null
+                            : env.getLabel(defParserLabel);
+                      }
+                    }));
+      }
       return builder
           /*<!-- #BLAZE_RULE($cc_rule).ATTRIBUTE(srcs) -->
           The list of C and C++ files that are processed to create the target.
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 246b21e96b..69bd049dad 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
@@ -323,7 +323,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
         generatedDefFile =
             CppHelper.createDefFileActions(
                 ruleContext,
-                ccToolchain.getDefParserTool(),
+                ruleContext.getPrerequisiteArtifact("$def_parser", Mode.HOST),
                 objectFiles.build(),
                 binary.getFilename());
 
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 fa9753b0dd..880495b011 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
@@ -525,9 +525,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
             toolchainInfo.supportsInterfaceSharedObjects()
                 ? ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST)
                 : null,
-            ruleContext.attributes().has("$def_parser")
-                ? ruleContext.getPrerequisiteArtifact("$def_parser", Mode.HOST)
-                : null,
             getEnvironment(ruleContext),
             builtInIncludeDirectories,
             sysroot);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
index dc387e55c7..7ad4afdac5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
@@ -77,7 +77,6 @@ public final class CcToolchainProvider extends ToolchainInfo {
           ImmutableList.<Artifact>of(),
           NestedSetBuilder.<Pair<String, String>>emptySet(Order.COMPILE_ORDER),
           null,
-          null,
           ImmutableMap.<String, String>of(),
           ImmutableList.<PathFragment>of(),
           null);
@@ -107,7 +106,6 @@ public final class CcToolchainProvider extends ToolchainInfo {
   private final ImmutableList<Artifact> builtinIncludeFiles;
   private final NestedSet<Pair<String, String>> coverageEnvironment;
   @Nullable private final Artifact linkDynamicLibraryTool;
-  @Nullable private final Artifact defParser;
   private final ImmutableMap<String, String> environment;
   private final ImmutableList<PathFragment> builtInIncludeDirectories;
   @Nullable private final PathFragment sysroot;
@@ -139,7 +137,6 @@ public final class CcToolchainProvider extends ToolchainInfo {
       ImmutableList<Artifact> builtinIncludeFiles,
       NestedSet<Pair<String, String>> coverageEnvironment,
       Artifact linkDynamicLibraryTool,
-      Artifact defParser,
       ImmutableMap<String, String> environment,
       ImmutableList<PathFragment> builtInIncludeDirectories,
       @Nullable PathFragment sysroot) {
@@ -169,7 +166,6 @@ public final class CcToolchainProvider extends ToolchainInfo {
     this.builtinIncludeFiles = builtinIncludeFiles;
     this.coverageEnvironment = coverageEnvironment;
     this.linkDynamicLibraryTool = linkDynamicLibraryTool;
-    this.defParser = defParser;
     this.environment = environment;
     this.builtInIncludeDirectories = builtInIncludeDirectories;
     this.sysroot = sysroot;
@@ -498,14 +494,6 @@ public final class CcToolchainProvider extends ToolchainInfo {
   }
 
   /**
-   * Returns the tool which should be used to parser object files for generating DEF file on
-   * Windows. The label of this tool is //third_party/def_parser:def_parser.
-   */
-  public Artifact getDefParserTool() {
-    return defParser;
-  }
-
-  /**
    * Returns the tool that builds interface libraries from dynamic libraries.
    */
   public Artifact getInterfaceSoBuilder() {
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 93a5f9ca63..139065790d 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
@@ -38,24 +38,6 @@ import com.google.devtools.build.lib.rules.cpp.transitions.LipoContextCollectorT
  * Rule definition for compiler definition.
  */
 public final class CcToolchainRule implements RuleDefinition {
-
-  /**
-   * The label points to the Windows object file parser. In bazel, it should be
-   * //tools/def_parser:def_parser, otherwise it should be null.
-   *
-   * <p>TODO(pcloudy): Remove this after Bazel rule definitions are not used internally anymore.
-   * Related bug b/63658220
-   */
-  private final String defParserLabel;
-
-  public CcToolchainRule(String defParser) {
-    this.defParserLabel = defParser;
-  }
-
-  public CcToolchainRule() {
-    this.defParserLabel = null;
-  }
-
   /**
    * Determines if the given target is a cc_toolchain or one of its subclasses. New subclasses
    * should be added to this method.
@@ -74,13 +56,6 @@ public final class CcToolchainRule implements RuleDefinition {
   @Override
   public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
     final Label zipper = env.getToolsLabel("//tools/zip:zipper");
-    if (defParserLabel != null) {
-      builder.add(
-          attr("$def_parser", LABEL)
-              .cfg(HostTransition.INSTANCE)
-              .singleArtifact()
-              .value(env.getLabel(defParserLabel)));
-    }
     return builder
         .setUndocumented()
         .requiresConfigurationFragments(CppConfiguration.class, PlatformConfiguration.class)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index 30c9e977c4..b973eacde2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -1427,7 +1427,7 @@ public final class CppModel {
         Artifact generatedDefFile =
             CppHelper.createDefFileActions(
                 ruleContext,
-                ccToolchain.getDefParserTool(),
+                ruleContext.getPrerequisiteArtifact("$def_parser", Mode.HOST),
                 ccOutputs.getObjectFiles(false),
                 SolibSymlinkAction.getDynamicLibrarySoname(soImpl.getRootRelativePath(), true));
         dynamicLinkActionBuilder.setDefFile(generatedDefFile);