Implement alwayslink behavior for object_file_group artifacts.
The current implementation of alwayslink for object_file_group artifacts is incorrect, based on the assumption that `-whole-archive --start-lib <objects> -no-whole-archive --end-lib` is equivalent to `<objects>`.
Although --start-lib/--end-lib flags do give objects archive-like resolution behavior, -whole-archive/-no-whole-archive only apply to static libraries. Therefore, all object_file_group artifacts are currently being linked with archive-like resolution behavior.
To correctly achieve alwayslink behavior with object groups, it suffices to leave out the --start-lib/--end-lib flags, so that the object files are received with their ordinary, alwayslink behavior.
In addition to implementing alwayslink behavior, this CL also restricts applications of -whole-archive/-no-whole-archive to static libraries. This produces no change in behavior.
PiperOrigin-RevId: 351362941
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
index 1319aa8..2aea052 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -672,12 +672,17 @@
" variable: 'libraries_to_link.type'",
" value: 'object_file_group'",
" }",
+ " expand_if_false: 'libraries_to_link.is_whole_archive'",
" flag: '-Wl,--start-lib'",
" }",
ifLinux(
platform,
" flag_group {",
" expand_if_true: 'libraries_to_link.is_whole_archive'",
+ " expand_if_equal: {",
+ " variable: 'libraries_to_link.type'",
+ " value: 'static_library'",
+ " }",
" flag: '-Wl,-whole-archive'",
" }",
" flag_group {",
@@ -725,6 +730,10 @@
" }",
" flag_group {",
" expand_if_true: 'libraries_to_link.is_whole_archive'",
+ " expand_if_equal: {",
+ " variable: 'libraries_to_link.type'",
+ " value: 'static_library'",
+ " }",
" flag: '-Wl,-no-whole-archive'",
" }"),
ifMac(
@@ -805,6 +814,7 @@
" variable: 'libraries_to_link.type'",
" value: 'object_file_group'",
" }",
+ " expand_if_false: 'libraries_to_link.is_whole_archive'",
" flag: '-Wl,--end-lib'",
" }",
" }",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
index e36068e..9c1edf0 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -277,45 +277,6 @@
assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
}
- @Test
- public void testLegacyWholeArchive() throws Exception {
- getAnalysisMock()
- .ccSupport()
- .setupCcToolchainConfig(
- mockToolsConfig,
- CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));
- scratch.file(
- "x/BUILD",
- "cc_binary(",
- " name = 'libfoo.so',",
- " srcs = ['foo.cc'],",
- " linkshared = 1,",
- ")");
- // --incompatible_remove_legacy_whole_archive not flipped, --legacy_whole_archive wins.
- useConfiguration("--legacy_whole_archive", "--noincompatible_remove_legacy_whole_archive");
- assertThat(getLibfooArguments()).contains("-Wl,-whole-archive");
- useConfiguration("--nolegacy_whole_archive", "--noincompatible_remove_legacy_whole_archive");
- assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
-
- // --incompatible_remove_legacy_whole_archive flipped, --legacy_whole_archive ignored.
- useConfiguration("--legacy_whole_archive", "--incompatible_remove_legacy_whole_archive");
- assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
- useConfiguration("--nolegacy_whole_archive", "--incompatible_remove_legacy_whole_archive");
- assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
-
- // Even when --nolegacy_whole_archive, features can still add the behavior back.
- useConfiguration(
- "--nolegacy_whole_archive",
- "--noincompatible_remove_legacy_whole_archive",
- "--features=legacy_whole_archive");
- assertThat(getLibfooArguments()).contains("-Wl,-whole-archive");
- // Even when --nolegacy_whole_archive, features can still add the behavior, but not when
- // --incompatible_remove_legacy_whole_archive is flipped.
- useConfiguration(
- "--incompatible_remove_legacy_whole_archive", "--features=legacy_whole_archive");
- assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
- }
-
private List<String> getLibfooArguments() throws Exception {
ConfiguredTarget configuredTarget = getConfiguredTarget("//x:libfoo.so");
CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/libfoo.so");
