Remove runtime dynamic libraries from default output group of cc_binary

Bazel copies dynamic libraries to the binary's directory so that it's available at runtime. Before this change, all runtime dynamic libraries are added into the default output group, this is bad because it makes the target name (eg. //:foo.dll) refer to multiple artifacts. In this change, we are having a separate output group (runtime_dynamic_libraries) for them.

Fixes #8707

RELNOTES: The runtime dynamic libraries are no longer in default output group of cc_binary.
PiperOrigin-RevId: 254931976

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

@@ -573,24 +573,13 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
 
     // If the binary is linked dynamically and COPY_DYNAMIC_LIBRARIES_TO_BINARY is enabled, collect
     // all the dynamic libraries we need at runtime. Then copy these libraries next to the binary.
+    NestedSet<Artifact> copiedRuntimeDynamicLibraries = null;
     if (featureConfiguration.isEnabled(CppRuleClasses.COPY_DYNAMIC_LIBRARIES_TO_BINARY)) {
-      ImmutableList.Builder<Artifact> runtimeLibraries = ImmutableList.builder();
-      for (LibraryToLink libraryToLink : depsCcLinkingContext.getLibraries()) {
-        Artifact library =
-            libraryToLink.getDynamicLibraryForRuntimeOrNull(/* linkingStatically= */ isStaticMode);
-        if (library != null) {
-          runtimeLibraries.add(library);
-        }
-      }
-      filesToBuild =
-          NestedSetBuilder.fromNestedSet(filesToBuild)
-              .addAll(
-                  createDynamicLibrariesCopyActions(
-                      ruleContext,
-                      NestedSetBuilder.<Artifact>linkOrder()
-                          .addAll(runtimeLibraries.build())
-                          .build()))
-              .build();
+      copiedRuntimeDynamicLibraries =
+          createDynamicLibrariesCopyActions(
+              ruleContext,
+              LibraryToLink.getDynamicLibrariesForRuntime(
+                  isStaticMode, depsCcLinkingContext.getLibraries()));
     }
 
     // TODO(bazel-team): Do we need to put original shared libraries (along with
@@ -670,6 +659,15 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
       }
     }
 
+    if (copiedRuntimeDynamicLibraries != null) {
+      // When COPY_DYNAMIC_LIBRARIES_TO_BINARY is enabled, runtime dynamic libraries should be
+      // copied to the binary's directory. Therefore, we add them to HIDDEN_TOP_LEVEL output group
+      // to make sure they are built for the target and runtime_dynamic_libraries output group
+      // to make them easier to access.
+      ruleBuilder.addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, copiedRuntimeDynamicLibraries);
+      ruleBuilder.addOutputGroup("runtime_dynamic_libraries", copiedRuntimeDynamicLibraries);
+    }
+
     CcSkylarkApiProvider.maybeAdd(ruleContext, ruleBuilder);
     return ruleBuilder
         .addProvider(RunfilesProvider.class, RunfilesProvider.simple(runfiles))
@@ -1019,16 +1017,20 @@ private static Packager createIntermediateDwpPackagers(
    * @param dynamicLibrariesForRuntime The libraries to be copied.
    * @return The result artifacts of the copies.
    */
-  private static ImmutableList<Artifact> createDynamicLibrariesCopyActions(
-      RuleContext ruleContext, NestedSet<Artifact> dynamicLibrariesForRuntime) {
-    ImmutableList.Builder<Artifact> result = ImmutableList.builder();
+  private static NestedSet<Artifact> createDynamicLibrariesCopyActions(
+      RuleContext ruleContext, Iterable<Artifact> dynamicLibrariesForRuntime) {
+    NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
     for (Artifact target : dynamicLibrariesForRuntime) {
       if (!ruleContext.getLabel().getPackageName().equals(target.getOwner().getPackageName())) {
         // SymlinkAction on file is actually copy on Windows.
         Artifact copy = ruleContext.getBinArtifact(target.getFilename());
         ruleContext.registerAction(SymlinkAction.toArtifact(
             ruleContext.getActionOwner(), target, copy, "Copying Execution Dynamic Library"));
         result.add(copy);
+      } else {
+        // If the target is already in the same directory as the binary, we don't need to copy it,
+        // but we still add it the result.
+        result.add(target);
       }
     }
     return result.build();

src/test/py/bazel/bazel_windows_cpp_test.py

@@ -357,7 +357,8 @@ def testBuildSharedLibraryFromCcBinaryWithStaticLink(self):
     bazel_bin = self.getBazelInfo('bazel-bin')
 
     exit_code, _, stderr = self.RunBazel([
-        'build', '//main:main.dll', '--output_groups=default,interface_library'
+        'build', '//main:main.dll',
+        '--output_groups=default,runtime_dynamic_libraries,interface_library'
     ])
     self.AssertExitCode(exit_code, 0, stderr)
 
@@ -387,11 +388,19 @@ def testBuildSharedLibraryFromCcBinaryWithDynamicLink(self):
             '  linkshared = 1,'
             '  features=["windows_export_all_symbols"]',
             ')',
+            '',
+            'genrule(',
+            '  name = "renamed_main",',
+            '  srcs = [":main.dll"],',
+            '  outs = ["main_renamed.dll"],',
+            '  cmd = "cp $< $@",',
+            ')',
         ])
     bazel_bin = self.getBazelInfo('bazel-bin')
 
     exit_code, _, stderr = self.RunBazel([
-        'build', '//main:main.dll', '--output_groups=default,interface_library'
+        'build', '//main:main.dll',
+        '--output_groups=default,runtime_dynamic_libraries,interface_library'
     ])
     self.AssertExitCode(exit_code, 0, stderr)
 
@@ -401,14 +410,20 @@ def testBuildSharedLibraryFromCcBinaryWithDynamicLink(self):
     self.assertTrue(os.path.exists(main_library))
     self.assertTrue(os.path.exists(main_interface))
     self.assertTrue(os.path.exists(def_file))
-    # A.dll and B.dll should be copied.
+    # 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')))
     # hello_A and hello_B should not be exported.
     self.AssertFileContentNotContains(def_file, 'hello_A')
     self.AssertFileContentNotContains(def_file, 'hello_B')
     self.AssertFileContentContains(def_file, 'hello_C')
 
+    # The copy should succeed since //main:main.dll is only supposed to refer to
+    # main.dll, A.dll and B.dll should be in a separate output group.
+    exit_code, _, stderr = self.RunBazel(['build', '//main:renamed_main'])
+    self.AssertExitCode(exit_code, 0, stderr)
+
   def testGetDefFileOfSharedLibraryFromCcBinary(self):
     self.createProjectFiles()
     self.ScratchFile(