From 914b4ce14624171a97ff8b41f9202058f10d15b2 Mon Sep 17 00:00:00 2001
From: pcloudy <pcloudy@google.com>
Date: Mon, 8 Oct 2018 15:15:31 -0700
Subject: [PATCH] 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
---
 .../build/lib/rules/cpp/LibrariesToLinkCollector.java       | 6 ++++--
 .../devtools/build/lib/rules/cpp/proto/CcProtoAspect.java   | 3 +++
 .../google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL  | 2 ++
 .../build/lib/rules/cpp/proto/CcProtoLibraryTest.java       | 5 +++--
 4 files changed, 12 insertions(+), 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
index e09e74f1097523..77debb9fa0cf32 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 @@ 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
index bfe880ff015bad..71830964297dff 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
@@ -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
index 74ba25996a7627..0ef230997778ed 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 @@ 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
index a213ba25cef002..532736ab7af72b 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 @@ 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.