bazel: flip --incompatible_avoid_conflict_dlls default to true

- Disabled some test cases in internal testing - Fixed Windows tests by adding the suffix, which is calculated based on repository name and package path. Closes #11600 RELNOTES: --incompatible_avoid_conflict_dlls=true is now the default. If enabled, all C++ dynamic linked libraries (DLLs) generated by cc_library on Windows will be renamed to name_{hash}.dll where hash is calculated based on the RepositoryName and the DLL's package path. This option is useful when you have one package which depends on severals cc_library with the same name (e.g //foo/bar1:utils and //foo/bar2:utils). PiperOrigin-RevId: 342037946
bazelbuild · Nov 12, 2020 · b1d1485 · b1d1485
1 parent 15e10cf
commit b1d1485
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 24 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -258,7 +258,8 @@ public static void init(
               ruleContext,
               ccToolchain,
               ruleContext.getConfiguration(),
-              LinkTargetType.NODEPS_DYNAMIC_LIBRARY));
+              LinkTargetType.NODEPS_DYNAMIC_LIBRARY,
+              CppHelper.getDLLHashSuffix(ruleContext, featureConfiguration)));
       if (CppHelper.useInterfaceSharedLibraries(
           cppConfiguration, ccToolchain, featureConfiguration)) {
         dynamicLibraries.add(
@@ -287,7 +288,8 @@ public static void init(
               ruleContext,
               ccToolchain,
               ruleContext.getConfiguration(),
-              LinkTargetType.NODEPS_DYNAMIC_LIBRARY));
+              LinkTargetType.NODEPS_DYNAMIC_LIBRARY,
+              CppHelper.getDLLHashSuffix(ruleContext, featureConfiguration)));
       if (CppHelper.useInterfaceSharedLibraries(
           cppConfiguration, ccToolchain, featureConfiguration)) {
         dynamicLibraries.add(

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
@@ -246,7 +246,7 @@ public String getTypeDescription() {
 
   @Option(
       name = "incompatible_avoid_conflict_dlls",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
       metadataTags = {

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
@@ -1059,6 +1059,9 @@ public void testCompilationParameterFile() throws Exception {
 
   @Test
   public void testClangClParameters() throws Exception {
+    if (!AnalysisMock.get().isThisBazel()) {
+      return;
+    }
     AnalysisMock.get()
         .ccSupport()
         .setupCcToolchainConfig(

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -443,6 +443,9 @@ public void testObjectFileNamesCanBeSpecifiedInToolchain() throws Exception {
 
   @Test
   public void testWindowsFileNamePatternsCanBeSpecifiedInToolchain() throws Exception {
+    if (!AnalysisMock.get().isThisBazel()) {
+      return;
+    }
     AnalysisMock.get()
         .ccSupport()
         .setupCcToolchainConfig(
@@ -485,7 +488,7 @@ public void testWindowsFileNamePatternsCanBeSpecifiedInToolchain() throws Except
 
     assertThat(
             artifactsToStrings(getOutputGroup(hello, CcLibrary.DYNAMIC_LIBRARY_OUTPUT_GROUP_NAME)))
-        .containsExactly("bin hello/hello.dll", "bin hello/hello.if.lib");
+        .containsExactly("bin hello/hello_59017c88f7.dll", "bin hello/hello.if.lib");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java
@@ -1254,6 +1254,9 @@ public void testCreateCompilationOutputs_empty() throws Exception {
 
   @Test
   public void testCcLinkingContextOnWindows() throws Exception {
+    if (!AnalysisMock.get().isThisBazel()) {
+      return;
+    }
     AnalysisMock.get()
         .ccSupport()
         .setupCcToolchainConfig(
@@ -1266,7 +1269,9 @@ public void testCcLinkingContextOnWindows() throws Exception {
     doTestCcLinkingContext(
         ImmutableList.of("a.a", "libdep2.a", "b.a", "c.a", "d.a", "libdep1.a"),
         ImmutableList.of("a.pic.a", "b.pic.a", "c.pic.a", "e.pic.a"),
-        ImmutableList.of("a.so", "libdep2.so", "b.so", "e.so", "libdep1.so"));
+        // The suffix of dynamic library is caculated based on repository name and package path
+        // to avoid conflicts with dynamic library from other packages.
+        ImmutableList.of("a.so", "libdep2_6b43f83676.so", "b.so", "e.so", "libdep1_6b43f83676.so"));
   }
 
   @Test

diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py
@@ -141,7 +141,7 @@ def testBuildDynamicLibraryWithUserExportedSymbol(self):
     # TODO(pcloudy): change suffixes to .lib and .dll after making DLL
     # extensions correct on Windows.
     import_library = os.path.join(bazel_bin, 'A.if.lib')
-    shared_library = os.path.join(bazel_bin, 'A.dll')
+    shared_library = os.path.join(bazel_bin, 'A_592cffea6a.dll')
     empty_def_file = os.path.join(bazel_bin, 'A.gen.empty.def')
 
     self.assertTrue(os.path.exists(import_library))
@@ -162,7 +162,7 @@ def testBuildDynamicLibraryWithExportSymbolFeature(self):
     # TODO(pcloudy): change suffixes to .lib and .dll after making DLL
     # extensions correct on Windows.
     import_library = os.path.join(bazel_bin, 'B.if.lib')
-    shared_library = os.path.join(bazel_bin, 'B.dll')
+    shared_library = os.path.join(bazel_bin, 'B_592cffea6a.dll')
     def_file = os.path.join(bazel_bin, 'B.gen.def')
     self.assertTrue(os.path.exists(import_library))
     self.assertTrue(os.path.exists(shared_library))
@@ -177,7 +177,7 @@ def testBuildDynamicLibraryWithExportSymbolFeature(self):
     ])
     self.AssertExitCode(exit_code, 0, stderr)
     import_library = os.path.join(bazel_bin, 'B.if.lib')
-    shared_library = os.path.join(bazel_bin, 'B.dll')
+    shared_library = os.path.join(bazel_bin, 'B_592cffea6a.dll')
     empty_def_file = os.path.join(bazel_bin, 'B.gen.empty.def')
     self.assertTrue(os.path.exists(import_library))
     self.assertTrue(os.path.exists(shared_library))
@@ -200,13 +200,13 @@ def testBuildCcBinaryWithDependenciesDynamicallyLinked(self):
     # a_import_library
     self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.if.lib')))
     # a_shared_library
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.dll')))
+    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A_592cffea6a.dll')))
     # a_def_file
     self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.gen.empty.def')))
     # b_import_library
     self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.if.lib')))
     # b_shared_library
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.dll')))
+    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B_592cffea6a.dll')))
     # b_def_file
     self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'B.gen.def')))
     # c_exe
@@ -230,8 +230,10 @@ def testBuildCcBinaryFromDifferentPackage(self):
     # Test if A.dll and B.dll are copied to the directory of main.exe
     main_bin = os.path.join(bazel_bin, 'main/main.exe')
     self.assertTrue(os.path.exists(main_bin))
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/A.dll')))
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/B.dll')))
+    self.assertTrue(
+        os.path.exists(os.path.join(bazel_bin, 'main/A_592cffea6a.dll')))
+    self.assertTrue(
+        os.path.exists(os.path.join(bazel_bin, 'main/B_592cffea6a.dll')))
 
     # Run the binary to see if it runs successfully
     exit_code, stdout, stderr = self.RunProgram([main_bin])
@@ -253,8 +255,7 @@ def testBuildCcBinaryDependsOnConflictDLLs(self):
     bazel_bin = self.getBazelInfo('bazel-bin')
 
     # //main:main depends on both //lib:A and //:A
-    exit_code, _, stderr = self.RunBazel(
-        ['build', '//main:main', '--incompatible_avoid_conflict_dlls'])
+    exit_code, _, stderr = self.RunBazel(['build', '//main:main'])
     self.AssertExitCode(exit_code, 0, stderr)
 
     # Run the binary to see if it runs successfully
@@ -367,13 +368,13 @@ def testDLLIsCopiedFromExternalRepo(self):
 
     bazel_bin = self.getBazelInfo('bazel-bin')
 
-    exit_code, _, stderr = self.RunBazel(['build', '//:main'])
+    exit_code, _, stderr = self.RunBazel(['build', '//:main', '-s'])
     self.AssertExitCode(exit_code, 0, stderr)
 
     # Test if A.dll is copied to the directory of main.exe
     main_bin = os.path.join(bazel_bin, 'main.exe')
     self.assertTrue(os.path.exists(main_bin))
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A.dll')))
+    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'A_adb8507c73.dll')))
 
     # Run the binary to see if it runs successfully
     exit_code, stdout, stderr = self.RunProgram([main_bin])
@@ -388,7 +389,7 @@ def testDynamicLinkingMSVCRT(self):
     exit_code, _, stderr = self.RunBazel(
         ['build', '//:A', '--output_groups=dynamic_library', '-s'])
     paramfile = os.path.join(
-        bazel_output, 'x64_windows-fastbuild/bin/A.dll-2.params')
+        bazel_output, 'x64_windows-fastbuild/bin/A_592cffea6a.dll-2.params')
     self.AssertExitCode(exit_code, 0, stderr)
     self.assertIn('/MD', ''.join(stderr))
     self.AssertFileContentContains(paramfile, '/DEFAULTLIB:msvcrt.lib')
@@ -398,7 +399,8 @@ def testDynamicLinkingMSVCRT(self):
     # Test build in debug mode.
     exit_code, _, stderr = self.RunBazel(
         ['build', '-c', 'dbg', '//:A', '--output_groups=dynamic_library', '-s'])
-    paramfile = os.path.join(bazel_output, 'x64_windows-dbg/bin/A.dll-2.params')
+    paramfile = os.path.join(bazel_output,
+                             'x64_windows-dbg/bin/A_592cffea6a.dll-2.params')
     self.AssertExitCode(exit_code, 0, stderr)
     self.assertIn('/MDd', ''.join(stderr))
     self.AssertFileContentContains(paramfile, '/DEFAULTLIB:msvcrtd.lib')
@@ -415,7 +417,7 @@ def testStaticLinkingMSVCRT(self):
         '--features=static_link_msvcrt', '-s'
     ])
     paramfile = os.path.join(
-        bazel_output, 'x64_windows-fastbuild/bin/A.dll-2.params')
+        bazel_output, 'x64_windows-fastbuild/bin/A_592cffea6a.dll-2.params')
     self.AssertExitCode(exit_code, 0, stderr)
     self.assertNotIn('/MD', ''.join(stderr))
     self.AssertFileContentNotContains(paramfile, '/DEFAULTLIB:msvcrt.lib')
@@ -427,7 +429,8 @@ def testStaticLinkingMSVCRT(self):
         'build', '-c', 'dbg', '//:A', '--output_groups=dynamic_library',
         '--features=static_link_msvcrt', '-s'
     ])
-    paramfile = os.path.join(bazel_output, 'x64_windows-dbg/bin/A.dll-2.params')
+    paramfile = os.path.join(bazel_output,
+                             'x64_windows-dbg/bin/A_592cffea6a.dll-2.params')
     self.AssertExitCode(exit_code, 0, stderr)
     self.assertNotIn('/MDd', ''.join(stderr))
     self.AssertFileContentNotContains(paramfile, '/DEFAULTLIB:msvcrtd.lib')
@@ -506,8 +509,10 @@ def testBuildSharedLibraryFromCcBinaryWithDynamicLink(self):
     self.assertTrue(os.path.exists(def_file))
     # A.dll and B.dll should be built and copied because they belong to
     # runtime_dynamic_libraries output group.
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/A.dll')))
-    self.assertTrue(os.path.exists(os.path.join(bazel_bin, 'main/B.dll')))
+    self.assertTrue(
+        os.path.exists(os.path.join(bazel_bin, 'main/A_592cffea6a.dll')))
+    self.assertTrue(
+        os.path.exists(os.path.join(bazel_bin, 'main/B_592cffea6a.dll')))
     # hello_A and hello_B should not be exported.
     self.AssertFileContentNotContains(def_file, 'hello_A')
     self.AssertFileContentNotContains(def_file, 'hello_B')

diff --git a/src/test/shell/bazel/bazel_windows_example_test.sh b/src/test/shell/bazel/bazel_windows_example_test.sh
@@ -119,7 +119,7 @@ function test_cpp_with_msys_gcc() {
     ./bazel-bin/${cpp_pkg}/libhello-lib.a ${cpp_pkg}:hello-world \
     --compiler=msys-gcc
   assert_build_output \
-    ./bazel-bin/${cpp_pkg}/libhello-lib.so ${cpp_pkg}:hello-lib\
+    ./bazel-bin/${cpp_pkg}/libhello-lib_7e140a9408.so ${cpp_pkg}:hello-lib\
     --compiler=msys-gcc --output_groups=dynamic_library
   assert_build ${cpp_pkg}:hello-world --compiler=msys-gcc
   ./bazel-bin/${cpp_pkg}/hello-world foo >& $TEST_log \
@@ -138,7 +138,7 @@ function test_cpp_with_mingw_gcc() {
     ./bazel-bin/${cpp_pkg}/libhello-lib.a ${cpp_pkg}:hello-world \
     --compiler=mingw-gcc --experimental_strict_action_env
   assert_build_output \
-    ./bazel-bin/${cpp_pkg}/libhello-lib.so ${cpp_pkg}:hello-lib\
+    ./bazel-bin/${cpp_pkg}/libhello-lib_7e140a9408.so ${cpp_pkg}:hello-lib\
     --compiler=mingw-gcc --output_groups=dynamic_library \
     --experimental_strict_action_env
   assert_build ${cpp_pkg}:hello-world --compiler=mingw-gcc \