Automated rollback of commit 64a966d.

*** Reason for rollback ***

Breaks a target in the canary's nightly, thus preventing us from releasing tomorrow:

[]

b/124656723

*** Original change description ***

Introduce --incompatible_remove_legacy_whole_archive

This flag disables legacy whole archive behavior, as well as makes the --legacy_whole_archive option a noop. See details in
#7362.

It also introduces a possibility to enable legacy behavior per target by `--features=legacy_whole_archive` option, or `features = [ "legacy_whole_archive" ]` attribute.

RELNOTES: None.
PiperOrigin-RevId: 234481950

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

@@ -396,10 +396,6 @@ public boolean legacyWholeArchive() {
     return cppOptions.legacyWholeArchive;
   }
 
-  public boolean removeLegacyWholeArchive() {
-    return cppOptions.removeLegacyWholeArchive;
-  }
-
   public boolean getInmemoryDotdFiles() {
     return cppOptions.inmemoryDotdFiles;
   }

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

@@ -646,7 +646,7 @@ public CppLinkAction build() throws InterruptedException {
 
     boolean needWholeArchive =
         wholeArchive
-            || needWholeArchive(featureConfiguration, linkType, linkopts, cppConfiguration);
+            || needWholeArchive(linkingMode, linkType, linkopts, isNativeDeps, cppConfiguration);
     // Disallow LTO indexing for test targets that link statically, and optionally for any
     // linkstatic target (which can be used to disable LTO indexing for non-testonly cc_binary
     // built due to data dependences for a blaze test invocation). Otherwise this will provoke
@@ -1117,37 +1117,15 @@ private boolean shouldUseLinkDynamicLibraryTool() {
 
   /** The default heuristic on whether we need to use whole-archive for the link. */
   private static boolean needWholeArchive(
-      FeatureConfiguration featureConfiguration,
+      Link.LinkingMode staticness,
       LinkTargetType type,
       Collection<String> linkopts,
+      boolean isNativeDeps,
       CppConfiguration cppConfig) {
+    boolean mostlyStatic = (staticness == Link.LinkingMode.STATIC);
     boolean sharedLinkopts =
         type.isDynamicLibrary() || linkopts.contains("-shared") || cppConfig.hasSharedLinkOption();
-    // Fasten your seat belt, the logic below doesn't make perfect sense and it's full of obviously
-    // missed corner cases. The world still stands and depends on this behavior, so ¯\_(ツ)_/¯.
-    if (!sharedLinkopts) {
-      // We are not producing shared library, there is no reason to use --whole-archive with
-      // executable (if the executable doesn't use the symbols, nobody else will, so --whole-archive
-      // is not needed).
-      return false;
-    }
-    if (cppConfig.removeLegacyWholeArchive()) {
-      // --incompatible_remove_legacy_whole_archive has been flipped, no --whole-archive for the
-      // entire build.
-      return false;
-    }
-    if (featureConfiguration.getRequestedFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) {
-      // --incompatible_remove_legacy_whole_archive has not been flipped, and this target requested
-      // --whole-archive using features.
-      return true;
-    }
-    if (cppConfig.legacyWholeArchive()) {
-      // --incompatible_remove_legacy_whole_archive has not been flipped, so whether to
-      // use --whole-archive depends on --legacy_whole_archive.
-      return true;
-    }
-    // Hopefully future default.
-    return false;
+    return (isNativeDeps || cppConfig.legacyWholeArchive()) && mostlyStatic && sharedLinkopts;
   }
 
   private static ImmutableSet<Artifact> constructOutputs(

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

@@ -360,18 +360,16 @@ public String getTypeDescription() {
   public Label customMalloc;
 
   @Option(
-      name = "legacy_whole_archive",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
-      effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.AFFECTS_OUTPUTS},
-      metadataTags = {OptionMetadataTag.DEPRECATED},
-      help =
-          "Deprecated, superseded by --incompatible_remove_legacy_whole_archive "
-              + "(see https://github.com/bazelbuild/bazel/issues/7362 for details). "
-              + "When on, use --whole-archive for cc_binary rules that have "
-              + "linkshared=1 and either linkstatic=1 or '-static' in linkopts. "
-              + "This is for backwards compatibility only. "
-              + "A better alternative is to use alwayslink=1 where required.")
+    name = "legacy_whole_archive",
+    defaultValue = "true",
+    documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
+    effectTags = {OptionEffectTag.ACTION_COMMAND_LINES, OptionEffectTag.AFFECTS_OUTPUTS},
+    help =
+        "When on, use --whole-archive for cc_binary rules that have "
+            + "linkshared=1 and either linkstatic=1 or '-static' in linkopts. "
+            + "This is for backwards compatibility only. "
+            + "A better alternative is to use alwayslink=1 where required."
+  )
   public boolean legacyWholeArchive;
 
   @Option(
@@ -714,20 +712,6 @@ public Label getFdoPrefetchHintsLabel() {
               + "(see https://github.com/bazelbuild/bazel/issues/6861 for migration instructions).")
   public boolean disableLegacyCrosstoolFields;
 
-  @Option(
-      name = "incompatible_remove_legacy_whole_archive",
-      defaultValue = "false",
-      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help =
-          "If true, Bazel will not link library dependencies as whole archive by default "
-              + "(see https://github.com/bazelbuild/bazel/issues/7362 for migration instructions).")
-  public boolean removeLegacyWholeArchive;
-
   @Option(
       name = "incompatible_remove_cpu_and_compiler_attributes_from_cc_toolchain",
       defaultValue = "false",
@@ -884,7 +868,6 @@ public FragmentOptions getHost() {
     host.disableLegacyCrosstoolFields = disableLegacyCrosstoolFields;
     host.disableCrosstool = disableCrosstool;
     host.enableCcToolchainResolution = enableCcToolchainResolution;
-    host.removeLegacyWholeArchive = removeLegacyWholeArchive;
     host.dontEnableHostNonhost = dontEnableHostNonhost;
     return host;
   }

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

@@ -412,9 +412,6 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) {
    */
   public static final String SUPPORTS_DYNAMIC_LINKER = "supports_dynamic_linker";
 
-  /** A feature marking that the target needs to link its deps in --whole-archive block. */
-  public static final String LEGACY_WHOLE_ARCHIVE = "legacy_whole_archive";
-
   /** Ancestor for all rules that do include scanning. */
   public static final class CcIncludeScanningRule implements RuleDefinition {
     @Override

src/main/java/com/google/devtools/build/lib/rules/nativedeps/NativeDepsHelper.java

@@ -230,15 +230,13 @@ public static NativeDepsRunfiles createNativeDepsAction(
       sharedLibrary = nativeDeps;
     }
     FdoContext fdoContext = toolchain.getFdoContext();
-    ImmutableSet.Builder<String> requestedFeatures =
-        ImmutableSet.<String>builder().addAll(ruleContext.getFeatures()).add(STATIC_LINKING_MODE);
-    if (!ruleContext.getDisabledFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) {
-      requestedFeatures.add(CppRuleClasses.LEGACY_WHOLE_ARCHIVE);
-    }
     FeatureConfiguration featureConfiguration =
         CcCommon.configureFeaturesOrReportRuleError(
             ruleContext,
-            /* requestedFeatures= */ requestedFeatures.build(),
+            /* requestedFeatures= */ ImmutableSet.<String>builder()
+                .addAll(ruleContext.getFeatures())
+                .add(STATIC_LINKING_MODE)
+                .build(),
             /* unsupportedFeatures= */ ruleContext.getDisabledFeatures(),
             toolchain);
     CppLinkActionBuilder builder =

src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java

@@ -41,7 +41,6 @@
 import com.google.devtools.build.lib.analysis.util.ActionTester.ActionCombinationFactory;
 import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -212,53 +211,6 @@ public void testLibOptsAndLibSrcsAreInCorrectOrder() throws Exception {
         .matches(".* -Wl,-rpath[^ ]*some-dir(?= ).* -Wl,-rpath[^ ]*some-other-dir .*");
   }
 
-  @Test
-  public void testLegacyWholeArchive() throws Exception {
-    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(
-        "--cpu=k8", "--legacy_whole_archive", "--noincompatible_remove_legacy_whole_archive");
-    assertThat(getLibfooArguments()).contains("-Wl,-whole-archive");
-    useConfiguration(
-        "--cpu=k8", "--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(
-        "--cpu=k8", "--legacy_whole_archive", "--incompatible_remove_legacy_whole_archive");
-    assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
-    useConfiguration(
-        "--cpu=k8", "--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(
-        "--cpu=k8",
-        "--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(
-        "--cpu=k8",
-        "--incompatible_remove_legacy_whole_archive",
-        "--features=legacy_whole_archive");
-    assertThat(getLibfooArguments()).doesNotContain("-Wl,-whole-archive");
-  }
-
-  private List<String> getLibfooArguments() throws LabelSyntaxException {
-    ConfiguredTarget configuredTarget = getConfiguredTarget("//x:libfoo.so");
-    CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "x/libfoo.so");
-    return linkAction.getArguments();
-  }
-
   @Test
   public void testExposesRuntimeLibrarySearchDirectoriesVariable() throws Exception {
     scratch.file(