Do not use link_dynamic_library.sh when toolchain doesn't support int…

…erface libraries Before, Bazel would always use link_dynamic_library.sh, even though the toolchain doesn't support interface libraries and therefore its arguments would always be "no ignored ignored ignored". And since link_dynamic_library.sh is a shell script, it unnecessarily complicates the Windows toolchain. This cl also removes the script as a dependency of the toolchain (so it won't be set as an input of c++ actions). RELNOTES: None. PiperOrigin-RevId: 159827038
dotordogh · Jun 26, 2017 · 3fdb917 · 3fdb917
1 parent 08d7c8e
commit 3fdb917
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 26 deletions.
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
@@ -368,7 +368,9 @@ public ConfiguredTarget create(RuleContext ruleContext)
             getBuildVariables(ruleContext),
             getBuiltinIncludes(ruleContext),
             coverageEnvironment.build(),
-            ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST),
+            cppConfiguration.supportsInterfaceSharedObjects()
+                ? ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST)
+                : null,
             getEnvironment(ruleContext),
             builtInIncludeDirectories,
             sysroot);

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -37,9 +37,10 @@ public static String getCppActionConfigs(
       String gccToolPath,
       String cppLinkDynamicLibraryToolPath,
       String arToolPath,
-      boolean supportsEmbeddedRuntimes) {
+      boolean supportsEmbeddedRuntimes,
+      boolean supportsInterfaceSharedLibraries) {
     String cppDynamicLibraryLinkerTool = "";
-    if (!features.contains("dynamic_library_linker_tool")) {
+    if (!features.contains("dynamic_library_linker_tool") && supportsInterfaceSharedLibraries) {
       cppDynamicLibraryLinkerTool =
           ""
               + "feature {"
@@ -138,8 +139,10 @@ public static String getCppActionConfigs(
                 "  tool {",
                 "    tool_path: '" + gccToolPath + "'",
                 "  }",
-                "  implies: 'build_interface_libraries'",
-                "  implies: 'dynamic_library_linker_tool'",
+                ifTrue(
+                    supportsInterfaceSharedLibraries,
+                    "implies: 'build_interface_libraries'",
+                    "implies: 'dynamic_library_linker_tool'"),
                 "  implies: 'symbol_counts'",
                 "  implies: 'strip_debug_symbols'",
                 "  implies: 'shared_flag'",
@@ -192,22 +195,24 @@ public static String getCppActionConfigs(
                 "  implies: 'libraries_to_link'",
                 "  implies: 'linker_param_file'",
                 "}",
-                "feature {",
-                "  name: 'build_interface_libraries'",
-                "  flag_set {",
-                "    expand_if_all_available: 'generate_interface_library'",
-                "    action: 'c++-link-dynamic-library'",
-                "    flag_group {",
-                "      flag: '%{generate_interface_library}'",
-                "      flag: '%{interface_library_builder_path}'",
-                "      flag: '%{interface_library_input_path}'",
-                "      flag: '%{interface_library_output_path}'",
-                "    }",
-                "  }",
-                "}",
-                // Order of feature declaration matters, cppDynamicLibraryLinkerTool has to follow
-                // right after build_interface_libraries.
-                cppDynamicLibraryLinkerTool,
+                ifTrue(
+                    supportsInterfaceSharedLibraries,
+                    "feature {",
+                    "  name: 'build_interface_libraries'",
+                    "  flag_set {",
+                    "    expand_if_all_available: 'generate_interface_library'",
+                    "    action: 'c++-link-dynamic-library'",
+                    "    flag_group {",
+                    "      flag: '%{generate_interface_library}'",
+                    "      flag: '%{interface_library_builder_path}'",
+                    "      flag: '%{interface_library_input_path}'",
+                    "      flag: '%{interface_library_output_path}'",
+                    "    }",
+                    "  }",
+                    "}",
+                    // Order of feature declaration matters, cppDynamicLibraryLinkerTool has to
+                    // follow right after build_interface_libraries.
+                    cppDynamicLibraryLinkerTool),
                 "feature {",
                 "  name: 'symbol_counts'",
                 "  flag_set {",

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
@@ -667,7 +667,8 @@ private CToolchain addLegacyFeatures(CToolchain toolchain) {
                   gccToolPath,
                   linkerToolPath,
                   arToolPath,
-                  supportsEmbeddedRuntimes),
+                  supportsEmbeddedRuntimes,
+                  toolchain.getSupportsInterfaceSharedObjects()),
               toolchainBuilder);
         }
 

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -758,8 +758,7 @@ public CppLinkAction build() throws InterruptedException {
             .setFeatureConfiguration(featureConfiguration);
 
     // TODO(b/62693279): Cleanup once internal crosstools specify ifso building correctly.
-    if (linkType.equals(LinkTargetType.DYNAMIC_LIBRARY)
-        && !featureConfiguration.hasConfiguredLinkerPathInActionConfig()) {
+    if (shouldUseLinkDynamicLibraryTool()) {
       linkCommandLineBuilder.forceToolPath(
           toolchain.getLinkDynamicLibraryTool().getExecPathString());
     }
@@ -783,8 +782,11 @@ public CppLinkAction build() throws InterruptedException {
     // Compute the set of inputs - we only need stable order here.
     NestedSetBuilder<Artifact> dependencyInputsBuilder = NestedSetBuilder.stableOrder();
     dependencyInputsBuilder.addTransitive(crosstoolInputs);
-    dependencyInputsBuilder.add(toolchain.getLinkDynamicLibraryTool());
     dependencyInputsBuilder.addTransitive(linkActionInputs.build());
+    // TODO(b/62693279): Cleanup once internal crosstools specify ifso building correctly.
+    if (shouldUseLinkDynamicLibraryTool()) {
+      dependencyInputsBuilder.add(toolchain.getLinkDynamicLibraryTool());
+    }
     if (runtimeMiddleman != null) {
       dependencyInputsBuilder.add(runtimeMiddleman);
     }
@@ -895,6 +897,12 @@ public CppLinkAction build() throws InterruptedException {
         executionRequirements.build());
   }
 
+  private boolean shouldUseLinkDynamicLibraryTool() {
+    return linkType.equals(LinkTargetType.DYNAMIC_LIBRARY)
+        && cppConfiguration.supportsInterfaceSharedObjects()
+        && !featureConfiguration.hasConfiguredLinkerPathInActionConfig();
+  }
+
   /** The default heuristic on whether we need to use whole-archive for the link. */
   private static boolean needWholeArchive(
       LinkStaticness staticness,

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -87,7 +87,8 @@ private final FeatureConfiguration getMockFeatureConfiguration() throws Exceptio
                 "gcc_tool",
                 "dynamic_library_linker_tool",
                 "ar_tool",
-                true))
+                true,
+                false))
         .getFeatureConfiguration(
             FeatureSpecification.create(
                 ImmutableSet.of(