commit e1b4a8b19a65ce90d7f52162ce6a7156b3394166
author pcloudy <pcloudy@google.com> Mon Oct 08 15:15:31 2018 -0700
committer Klaus Aehlig <aehlig@google.com> Mon Oct 29 10:49:59 2018 +0100
tree 30fbbd30b563ebacc6af6e2c974bd39932e05f06
parent 4fcc22d92849b575c148cdb8b526c54207d2960d

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 https://github.com/bazelbuild/bazel/issues/6171
Fixes https://github.com/bazelbuild/bazel/issues/6292
Fixes https://github.com/bazelbuild/bazel/issues/6169

RELNOTES: None
PiperOrigin-RevId: 216258674

src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java

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
index e09e74f..77debb9 100644
--- 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 @@
     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()));
     }

src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java

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
index a4871d5..728ddc5 100644
--- 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
@@ -268,6 +268,10 @@
                   toolchain.getFdoProvider(),
                   ruleContext.getConfiguration())
               .enableCcNativeLibrariesProvider();
+      if (toolchain.supportsInterfaceSharedObjects()) {
+        helper.enableInterfaceSharedObjects();
+      }
+
       TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime();
       if (runtime != null) {
         helper.addDeps(ImmutableList.of(runtime));

src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL

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
index 74ba259..0ef2309 100644
--- 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 @@
    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 {

src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java

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
index a213ba2..532736a 100644
--- 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 @@
         "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 @@
 
     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.