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 https://github.com/bazelbuild/bazel/issues/8707
RELNOTES: The runtime dynamic libraries are no longer in default output group of cc_binary.
PiperOrigin-RevId: 254931976
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 6ec918f..f6acae4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -573,24 +573,13 @@
// 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 @@
}
}
+ 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,9 +1017,9 @@
* @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.
@@ -1029,6 +1027,10 @@
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();
diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py
index b3b97b0..038148e 100644
--- a/src/test/py/bazel/bazel_windows_cpp_test.py
+++ b/src/test/py/bazel/bazel_windows_cpp_test.py
@@ -357,7 +357,8 @@
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 @@
' 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,7 +410,8 @@
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.
@@ -409,6 +419,11 @@
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(
