Windows: Fix Precondition check for addDynamicInputLinkOptions

The MSYS gcc and MINGW gcc toolchains do support linking against shared library. So the precondition check should be disabled for them. CcProtoAspect.java should set emitInterfaceSharedObjects to true when the toolchain supports interface shared library. Fixes #6171 Fixes #6292 Fixes #6169 RELNOTES: None PiperOrigin-RevId: 216258674
bazelbuild · Oct 23, 2018 · 539dce2 · 539dce2
1 parent 4e51339
commit 539dce2
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java
@@ -285,8 +285,10 @@ private void addDynamicInputLinkOptions(
     Preconditions.checkState(
         !Link.useStartEndLib(
             input, CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider)));
-    if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)) {
-      // On Windows, dynamic library (dll) cannot be linked directly.
+    if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)
+        && ccToolchainProvider.supportsInterfaceSharedObjects()) {
+      // On Windows, dynamic library (dll) cannot be linked directly when using toolchains that
+      // support interface library (eg. MSVC).
       Preconditions.checkState(
           !CppFileTypes.SHARED_LIBRARY.matches(input.getArtifact().getFilename()));
     }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -202,6 +202,9 @@ private static class Impl {
       depsBuilder.addAll(ruleContext.getPrerequisites("deps", TARGET));
       ImmutableList<TransitiveInfoCollection> deps = depsBuilder.build();
       CcLinkingHelper ccLinkingHelper = initializeLinkingHelper(featureConfiguration, deps);
+      if (ccToolchain(ruleContext).supportsInterfaceSharedObjects()) {
+        ccLinkingHelper.emitInterfaceSharedObjects(true);
+      }
       CcLinkingOutputs ccLinkingOutputs = CcLinkingOutputs.EMPTY;
       if (!ccCompilationOutputs.isEmpty()) {
         ccLinkingOutputs = ccLinkingHelper.link(ccCompilationOutputs);

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL b/src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL
@@ -584,6 +584,8 @@ toolchain {
    tool_path { name: "strip" path: "C:/tools/msys64/usr/bin/strip" }
    tool_path { name: "llvm-profdata" path: "C:/tools/msys64/usr/bin/llvm-profdata" }
    linking_mode_flags { mode: DYNAMIC }
+
+   supports_interface_shared_objects: true
 }
 
 toolchain {

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java
@@ -68,7 +68,8 @@ public void basic() throws Exception {
         "cc_proto_library(name = 'foo_cc_proto', deps = ['foo_proto'])",
         "proto_library(name = 'foo_proto', srcs = ['foo.proto'])");
     assertThat(prettyArtifactNames(getFilesToBuild(getConfiguredTarget("//x:foo_cc_proto"))))
-        .containsExactly("x/foo.pb.h", "x/foo.pb.cc", "x/libfoo_proto.a", "x/libfoo_proto.so");
+        .containsExactly("x/foo.pb.h", "x/foo.pb.cc", "x/libfoo_proto.a",
+            "x/libfoo_proto.ifso", "x/libfoo_proto.so");
   }
 
   @Test
@@ -207,7 +208,7 @@ public void commandLineControlsOutputFileSuffixes() throws Exception {
 
     assertThat(prettyArtifactNames(getFilesToBuild(getConfiguredTarget("//x:foo_cc_proto"))))
         .containsExactly("x/foo.pb.cc", "x/foo.pb.h", "x/foo.pb.cc.meta", "x/foo.proto.h",
-            "x/libfoo_proto.a", "x/libfoo_proto.so");
+            "x/libfoo_proto.a", "x/libfoo_proto.ifso", "x/libfoo_proto.so");
   }
 
   // TODO(carmi): test blacklisted protos. I don't currently understand what's the wanted behavior.