Simplify propagation of linker params by replacing global_whole_archive with

individual --whole_archive/--no_whole_archive blocks.

--
PiperOrigin-RevId: 141555295
MOS_MIGRATED_REVID=141555295

4 files changed, 189 insertions, 193 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
index 8c15eb7d0e..be01fd4fa5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -953,6 +953,85 @@ public class CcToolchainFeatures implements Serializable {
      * significantly reduces memory overhead.
      */
     @Immutable
+    public static final class LibraryToLinkValue implements VariableValue {
+
+      public static final String NAMES_FIELD_NAME = "names";
+      public static final String IS_WHOLE_ARCHIVE_FIELD_NAME = "is_whole_archive";
+      public static final String IS_LIB_GROUP_FIELD_NAME = "is_lib_group";
+
+      private final ImmutableList<String> names;
+      private final boolean isWholeArchive;
+      private final boolean isLibGroup;
+
+      public LibraryToLinkValue(String name, boolean isWholeArchive) {
+        this(ImmutableList.of(name), isWholeArchive, false);
+      }
+
+      public LibraryToLinkValue(
+          ImmutableList<String> names, boolean isWholeArchive, boolean isLibGroup) {
+        this.names = names;
+        this.isWholeArchive = isWholeArchive;
+        this.isLibGroup = isLibGroup;
+      }
+
+      @Override
+      public Iterable<? extends VariableValue> getSequenceValue(String variableName) {
+        throw new ExpansionException(
+            String.format(
+                "Invalid toolchain configuration: Cannot expand variable '%s': expected sequence, "
+                    + "found structure (LibraryToLink)",
+                variableName));
+      }
+
+      @Override
+      public boolean isSequence() {
+        return false;
+      }
+
+      @Override
+      public VariableValue getFieldValue(String variableName, String field) {
+        Preconditions.checkNotNull(field);
+        if (NAMES_FIELD_NAME.equals(field)) {
+          return new StringSequence(names);
+        } else if (IS_WHOLE_ARCHIVE_FIELD_NAME.equals(field)) {
+          return new IntegerValue(isWholeArchive ? 1 : 0);
+        } else if (IS_LIB_GROUP_FIELD_NAME.equals(field)) {
+          return new IntegerValue(isLibGroup ? 1 : 0);
+        } else if ("whole_archive_presence".equals(field)) {
+          // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
+          return isWholeArchive ? new IntegerValue(0) : null;
+        } else if ("no_whole_archive_presence".equals(field)) {
+          // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
+          return !isWholeArchive ? new IntegerValue(0) : null;
+        } else if ("lib_group_presence".equals(field)) {
+          // TODO(b/33403458): Cleanup this workaround once bazel >=0.4.3 is released.
+          return names.size() > 1 ? new IntegerValue(0) : null;
+        } else {
+          return null;
+        }
+      }
+
+      @Override
+      public String getStringValue(String variableName) {
+        throw new ExpansionException(
+            String.format(
+                "Invalid toolchain configuration: Cannot expand variable '%s': expected string, "
+                    + "found structure (LibraryToLink)",
+                variableName));
+      }
+
+      @Override
+      public boolean isTruthy() {
+        return true;
+      }
+    }
+
+    /**
+     * A sequence of structure values. Exists as a memory optimization - a typical build can contain
+     * millions of feature values, so getting rid of the overhead of {@code StructureValue} objects
+     * significantly reduces memory overhead.
+     */
+    @Immutable
     private static final class StructureSequence implements VariableValue {
 
       private final ImmutableList<ImmutableMap<String, VariableValue>> values;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 4b32f11dd0..35f16bfbbc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -39,6 +39,8 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.LibraryToLinkValue;
+import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.SequenceBuilder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
 import com.google.devtools.build.lib.rules.cpp.CppLinkAction.Context;
 import com.google.devtools.build.lib.rules.cpp.CppLinkAction.LinkArtifactFactory;
@@ -73,21 +75,8 @@ public class CppLinkActionBuilder {
    */
   public static final String LIBOPTS_VARIABLE = "libopts";
 
-  /**
-   * A build variable for flags providing files to link as inputs in the linker invocation that
-   * should not go in a -whole_archive block.
-   */
-  public static final String LINKER_INPUT_PARAMS_VARIABLE = "linker_input_params";
-
-  /**
-   * A build variable for flags providing files to link as inputs in the linker invocation that
-   * should go in a -whole_archive block.
-   */
-  public static final String WHOLE_ARCHIVE_LINKER_INPUT_PARAMS_VARIABLE =
-      "whole_archive_linker_params";
-
-  /** A build variable whose presence indicates that whole archive flags should be applied. */
-  public static final String GLOBAL_WHOLE_ARCHIVE_VARIABLE = "global_whole_archive";
+  /** A build variable for flags providing files to link as inputs in the linker invocation */
+  public static final String LIBRARIES_TO_LINK_VARIABLE = "libraries_to_link";
 
   /** A build variable for the execpath of the output of the linker. */
   public static final String OUTPUT_EXECPATH_VARIABLE = "output_execpath";
@@ -631,11 +620,6 @@ public class CppLinkActionBuilder {
             .setToolPath(getToolPath())
             .setFeatureConfiguration(featureConfiguration);
 
-    // TODO(b/30228443): Refactor noWholeArchiveInputs into action_configs, and remove this.
-    if (needWholeArchive) {
-      linkCommandLineBuilder.setNoWholeArchiveFlags(variablesExtension.getNoWholeArchiveInputs());
-    }
-
     if (!isLTOIndexing) {
       linkCommandLineBuilder
           .setOutput(output)
@@ -1184,9 +1168,7 @@ public class CppLinkActionBuilder {
     String rpathRoot;
     ImmutableList<String> rpathEntries;
     ImmutableSet<String> libopts;
-    ImmutableList<String> linkerInputParams;
-    ImmutableList<String> wholeArchiveLinkerInputParams;
-    ImmutableList<String> noWholeArchiveInputs;
+    SequenceBuilder librariesToLink;
 
     public void setRpathRoot(String rPathRoot) {
       this.rpathRoot = rPathRoot;
@@ -1200,16 +1182,8 @@ public class CppLinkActionBuilder {
       this.libopts = libopts;
     }
 
-    public void setLinkerInputParams(ImmutableList<String> linkerInputParams) {
-      this.linkerInputParams = linkerInputParams;
-    }
-
-    public void setWholeArchiveLinkerInputParams(ImmutableList<String> wholeArchiveInputParams) {
-      this.wholeArchiveLinkerInputParams = wholeArchiveInputParams;
-    }
-
-    public void setNoWholeArchiveInputs(ImmutableList<String> noWholeArchiveInputs) {
-      this.noWholeArchiveInputs = noWholeArchiveInputs;
+    public void setLibrariesToLink(SequenceBuilder librariesToLink) {
+      this.librariesToLink = librariesToLink;
     }
 
     public String getRpathRoot() {
@@ -1224,16 +1198,8 @@ public class CppLinkActionBuilder {
       return libopts;
     }
 
-    public ImmutableList<String> getLinkerInputParams() {
-      return linkerInputParams;
-    }
-
-    public ImmutableList<String> getWholeArchiveLinkerInputParams() {
-      return wholeArchiveLinkerInputParams;
-    }
-
-    public ImmutableList<String> getNoWholeArchiveInputs() {
-      return noWholeArchiveInputs;
+    public SequenceBuilder getLibrariesToLink() {
+      return librariesToLink;
     }
   }
 
@@ -1277,16 +1243,6 @@ public class CppLinkActionBuilder {
       addInputFileLinkOptions(linkArgCollector);
     }
 
-    /**
-     * Returns linker parameters indicating libraries that should not be linked inside a
-     * --whole_archive block.
-     *
-     * <p>TODO(b/30228443): Refactor into action configs
-     */
-    public List<String> getNoWholeArchiveInputs() {
-      return linkArgCollector.getNoWholeArchiveInputs();
-    }
-
     @Override
     public void addVariables(Variables.Builder buildVariables) {
 
@@ -1323,16 +1279,9 @@ public class CppLinkActionBuilder {
       }
 
       buildVariables.addStringSequenceVariable(LIBOPTS_VARIABLE, linkArgCollector.getLibopts());
-      buildVariables.addStringSequenceVariable(
-          LINKER_INPUT_PARAMS_VARIABLE, linkArgCollector.getLinkerInputParams());
-      buildVariables.addStringSequenceVariable(
-          WHOLE_ARCHIVE_LINKER_INPUT_PARAMS_VARIABLE,
-          linkArgCollector.getWholeArchiveLinkerInputParams());
 
-      // global archive
-      if (needWholeArchive) {
-        buildVariables.addStringVariable(GLOBAL_WHOLE_ARCHIVE_VARIABLE, "");
-      }
+      buildVariables.addCustomBuiltVariable(
+          LIBRARIES_TO_LINK_VARIABLE, linkArgCollector.getLibrariesToLink());
 
       // mostly static
       if (linkStaticness == LinkStaticness.MOSTLY_STATIC && cppConfiguration.skipStaticOutputs()) {
@@ -1385,15 +1334,6 @@ public class CppLinkActionBuilder {
       return isNativeDeps && cppConfiguration.shareNativeDeps();
     }
 
-    private boolean inputNeedsWholeArchive(LinkerInput input) {
-      if (Link.useStartEndLib(input, cppConfiguration.archiveType())) {
-        return false;
-      }
-
-      return input.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY
-           && !wholeArchive && !needWholeArchive;
-    }
-
     /**
      * When linking a shared library fully or mostly static then we need to link in *all* dependent
      * files, not just what the shared library needs for its own code. This is done by wrapping all
@@ -1410,7 +1350,7 @@ public class CppLinkActionBuilder {
 
       // List of command line parameters that need to be placed *outside* of
       // --whole-archive ... --no-whole-archive.
-      ImmutableList.Builder<String> noWholeArchiveInputs = ImmutableList.builder();
+      SequenceBuilder librariesToLink = new SequenceBuilder();
 
       PathFragment solibDir =
           configuration
@@ -1506,9 +1446,6 @@ public class CppLinkActionBuilder {
         }
       }
 
-      ImmutableList.Builder<String> wholeArchiveInputParams = ImmutableList.builder();
-      ImmutableList.Builder<String> standardArchiveInputParams = ImmutableList.builder();
-
       for (LinkerInput input : linkerInputs) {
         if (input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY) {
           PathFragment libDir = input.getArtifact().getExecPath().getParentDirectory();
@@ -1520,20 +1457,15 @@ public class CppLinkActionBuilder {
           if (libDir.equals(solibDir)) {
             includeSolibDir = true;
           }
-          addDynamicInputLinkOptions(
-              input, standardArchiveInputParams, libOpts, solibDir, rpathRoot);
+          addDynamicInputLinkOptions(input, librariesToLink, false, libOpts, solibDir, rpathRoot);
         } else {
-          addStaticInputLinkOptions(
-              input, wholeArchiveInputParams, standardArchiveInputParams, ltoMap);
+          addStaticInputLinkOptions(input, librariesToLink, false, ltoMap);
         }
       }
 
       boolean includeRuntimeSolibDir = false;
 
       for (LinkerInput input : runtimeLinkerInputs) {
-        ImmutableList.Builder<String> optionsList =
-            needWholeArchive ? noWholeArchiveInputs : standardArchiveInputParams;
-
         if (input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY) {
           PathFragment libDir = input.getArtifact().getExecPath().getParentDirectory();
           Preconditions.checkState(
@@ -1542,16 +1474,15 @@ public class CppLinkActionBuilder {
               input.getArtifact(),
               solibDir);
           includeRuntimeSolibDir = true;
-          addDynamicInputLinkOptions(input, optionsList, libOpts, solibDir, rpathRoot);
+          addDynamicInputLinkOptions(input, librariesToLink, true, libOpts, solibDir, rpathRoot);
         } else {
-          addStaticInputLinkOptions(input,
-              needWholeArchive ? noWholeArchiveInputs : wholeArchiveInputParams,
-              needWholeArchive ? noWholeArchiveInputs : standardArchiveInputParams,
-              ltoMap);
+          addStaticInputLinkOptions(input, librariesToLink, true, ltoMap);
         }
       }
       if (linkerParamsFile != null && ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT)) {
-        standardArchiveInputParams.add("-Wl,@" + linkerParamsFile.getExecPathString());
+        librariesToLink.addValue(
+            new LibraryToLinkValue(
+                "-Wl,@" + linkerParamsFile.getExecPathString(), needWholeArchive));
       }
 
       // rpath ordering matters for performance; first add the one where most libraries are found.
@@ -1564,19 +1495,22 @@ public class CppLinkActionBuilder {
 
       linkArgCollector.setLibopts(libOpts.build());
 
-      linkArgCollector.setLinkerInputParams(standardArchiveInputParams.build());
-      linkArgCollector.setWholeArchiveLinkerInputParams(wholeArchiveInputParams.build());
-      linkArgCollector.setNoWholeArchiveInputs(noWholeArchiveInputs.build());
+      linkArgCollector.setLibrariesToLink(librariesToLink);
 
       if (ltoMap != null) {
         Preconditions.checkState(ltoMap.isEmpty(), "Still have LTO objects left: %s", ltoMap);
       }
     }
 
-    /** Adds command-line options for a dynamic library input file into options and libOpts. */
+    /**
+     * Adds command-line options for a dynamic library input file into options and libOpts.
+     *
+     * @param librariesToLink - a collection that will be exposed as a build variable.
+     */
     private void addDynamicInputLinkOptions(
         LinkerInput input,
-        ImmutableList.Builder<String> options,
+        SequenceBuilder librariesToLink,
+        boolean isRuntimeLinkerInput,
         ImmutableSet.Builder<String> libOpts,
         PathFragment solibDir,
         String rpathRoot) {
@@ -1601,28 +1535,30 @@ public class CppLinkActionBuilder {
       libOpts.add("-L" + inputArtifact.getExecPath().getParentDirectory().getPathString());
 
       String name = inputArtifact.getFilename();
+      boolean inputIsWholeArchive = !isRuntimeLinkerInput && needWholeArchive;
       if (CppFileTypes.SHARED_LIBRARY.matches(name)) {
         String libName = name.replaceAll("(^lib|\\.(so|dylib)$)", "");
-        options.add("-l" + libName);
+        librariesToLink.addValue(new LibraryToLinkValue("-l" + libName, inputIsWholeArchive));
       } else {
         // Interface shared objects have a non-standard extension
         // that the linker won't be able to find.  So use the
         // filename directly rather than a -l option.  Since the
         // library has an SONAME attribute, this will work fine.
-        options.add(inputArtifact.getExecPathString());
+        librariesToLink.addValue(
+            new LibraryToLinkValue(inputArtifact.getExecPathString(), inputIsWholeArchive));
       }
     }
 
     /**
      * Adds command-line options for a static library or non-library input into options.
      *
+     * @param librariesToLink - a collection that will be exposed as a build variable.
      * @param ltoMap is a mutable list of exec paths that should be on the command-line, which must
-     *     be supplied for LTO final links.
      */
     private void addStaticInputLinkOptions(
         LinkerInput input,
-        ImmutableList.Builder<String> wholeArchiveOptions,
-        ImmutableList.Builder<String> standardOptions,
+        SequenceBuilder librariesToLink,
+        boolean isRuntimeLinkerInput,
         @Nullable Map<Artifact, Artifact> ltoMap) {
       Preconditions.checkState(!(input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY));
       // If we had any LTO artifacts, ltoMap whould be non-null. In that case,
@@ -1638,7 +1574,7 @@ public class CppLinkActionBuilder {
       if (Link.useStartEndLib(input, cppConfiguration.archiveType())) {
         Iterable<Artifact> archiveMembers = input.getObjectFiles();
         if (!Iterables.isEmpty(archiveMembers)) {
-          List<String> nonLTOArchiveMembers = new ArrayList<>();
+          ImmutableList.Builder<String> nonLTOArchiveMembersBuilder = ImmutableList.builder();
           for (Artifact member : archiveMembers) {
             if (ltoMap != null && ltoMap.remove(member) != null) {
               // The LTO artifacts that should be included in the final link
@@ -1647,19 +1583,23 @@ public class CppLinkActionBuilder {
               // files explicitly, or generate .o files from assembler.
               continue;
             }
-            nonLTOArchiveMembers.add(member.getExecPathString());
+            nonLTOArchiveMembersBuilder.add(member.getExecPathString());
           }
+          ImmutableList<String> nonLTOArchiveMembers = nonLTOArchiveMembersBuilder.build();
           if (!nonLTOArchiveMembers.isEmpty()) {
-            standardOptions.add("-Wl,--start-lib");
-            standardOptions.addAll(nonLTOArchiveMembers);
-            standardOptions.add("-Wl,--end-lib");
+            boolean inputIsWholeArchive = !isRuntimeLinkerInput && needWholeArchive;
+            librariesToLink.addValue(
+                new LibraryToLinkValue(nonLTOArchiveMembers, inputIsWholeArchive, true));
           }
         }
       } else {
-        ImmutableList.Builder<String> options =
-            inputNeedsWholeArchive(input) ? wholeArchiveOptions : standardOptions;
+        boolean isAlwaysLinkStaticLibrary =
+            input.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY;
         // For anything else, add the input directly.
         Artifact inputArtifact = input.getArtifact();
+        boolean inputIsWholeArchive =
+            (!isRuntimeLinkerInput && (isAlwaysLinkStaticLibrary || needWholeArchive))
+                || (isRuntimeLinkerInput && isAlwaysLinkStaticLibrary && !needWholeArchive);
 
         if (ltoMap != null && ltoMap.remove(inputArtifact) != null) {
           // The LTO artifacts that should be included in the final link
@@ -1668,9 +1608,13 @@ public class CppLinkActionBuilder {
         }
 
         if (input.isFake()) {
-          options.add(Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString());
+          librariesToLink.addValue(
+              new LibraryToLinkValue(
+                  Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString(),
+                  inputIsWholeArchive));
         } else {
-          options.add(inputArtifact.getExecPathString());
+          librariesToLink.addValue(
+              new LibraryToLinkValue(inputArtifact.getExecPathString(), inputIsWholeArchive));
         }
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
index dc7b250c0a..6eba06d0f7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
@@ -62,10 +62,9 @@ public class CppLinkActionConfigs {
                 "   implies: 'symbol_counts'",
                 "   implies: 'linkstamps'",
                 "   implies: 'output_execpath_flags_executable'",
-                "   implies: 'global_whole_archive_open'",
                 "   implies: 'runtime_root_flags'",
                 "   implies: 'input_param_flags'",
-                "   implies: 'global_whole_archive_close'",
+                "   implies: 'libraries_to_link'",
                 "   implies: 'force_pic_flags'",
                 "}",
                 "action_config {",
@@ -80,10 +79,9 @@ public class CppLinkActionConfigs {
                 "   implies: 'shared_flag'",
                 "   implies: 'linkstamps'",
                 "   implies: 'output_execpath_flags'",
-                "   implies: 'global_whole_archive_open'",
                 "   implies: 'runtime_root_flags'",
                 "   implies: 'input_param_flags'",
-                "   implies: 'global_whole_archive_close'",
+                "   implies: 'libraries_to_link'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-static-library'",
@@ -91,10 +89,9 @@ public class CppLinkActionConfigs {
                 "   tool {",
                 "       tool_path: 'DUMMY_TOOL'",
                 "   }",
-                "   implies: 'global_whole_archive_open'",
                 "   implies: 'runtime_root_flags'",
                 "   implies: 'input_param_flags'",
-                "   implies: 'global_whole_archive_close'",
+                "   implies: 'libraries_to_link'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-alwayslink-static-library'",
@@ -102,10 +99,9 @@ public class CppLinkActionConfigs {
                 "   tool {",
                 "       tool_path: 'DUMMY_TOOL'",
                 "   }",
-                "   implies: 'global_whole_archive_open'",
                 "   implies: 'runtime_root_flags'",
                 "   implies: 'input_param_flags'",
-                "   implies: 'global_whole_archive_close'",
+                "   implies: 'libraries_to_link'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-pic-static-library'",
@@ -113,10 +109,9 @@ public class CppLinkActionConfigs {
                 "   tool {",
                 "       tool_path: 'DUMMY_TOOL'",
                 "   }",
-                "   implies: 'global_whole_archive_open'",
                 "   implies: 'runtime_root_flags'",
                 "   implies: 'input_param_flags'",
-                "   implies: 'global_whole_archive_close'",
+                "   implies: 'libraries_to_link'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-alwayslink-pic-static-library'",
@@ -124,10 +119,9 @@ public class CppLinkActionConfigs {
                 "   tool {",
                 "       tool_path: 'DUMMY_TOOL'",
                 "   }",
-                "   implies: 'global_whole_archive_open'",
                 "   implies: 'runtime_root_flags'",
                 "   implies: 'input_param_flags'",
-                "   implies: 'global_whole_archive_close'",
+                "   implies: 'libraries_to_link'",
                 "}",
                 "feature {",
                 "   name: 'build_interface_libraries'",
@@ -215,24 +209,6 @@ public class CppLinkActionConfigs {
                 "   }",
                 "}",
                 "feature {",
-                "   name: 'global_whole_archive_open'",
-                "   flag_set {",
-                "      expand_if_all_available: 'global_whole_archive'",
-                "      action: 'c++-link-executable'",
-                "      action: 'c++-link-dynamic-library'",
-                "      action: 'c++-link-static-library'",
-                "      action: 'c++-link-alwayslink-static-library'",
-                "      action: 'c++-link-pic-static-library'",
-                "      action: 'c++-link-alwayslink-pic-static-library'",
-                "      flag_group {",
-                // TODO: Factor platform difference into respective linux and OSX crosstools.
-                String.format(
-                    "          flag:'%s'",
-                    platform == CppLinkPlatform.LINUX ? "-Wl,-whole-archive" : "-Wl,-all_load"),
-                "      }",
-                "   }",
-                "}",
-                "feature {",
                 "   name: 'runtime_root_flags',",
                 "   flag_set {",
                 "       expand_if_all_available: 'runtime_root_flags'",
@@ -273,39 +249,11 @@ public class CppLinkActionConfigs {
                 "           flag: '%{libopts}'",
                 "       }",
                 "   }",
-                "   flag_set {",
-                "       expand_if_all_available: 'whole_archive_linker_params'",
-                "       action: 'c++-link-executable'",
-                "       action: 'c++-link-dynamic-library'",
-                "       action: 'c++-link-static-library'",
-                "       action: 'c++-link-alwayslink-static-library'",
-                "       action: 'c++-link-pic-static-library'",
-                "       action: 'c++-link-alwayslink-pic-static-library'",
-                "       flag_group {",
-                platform == CppLinkPlatform.LINUX
-                    ? "          flag: '-Wl,-whole-archive'\n"
-                        + "          flag: '%{whole_archive_linker_params}'\n"
-                        + "          flag: '-Wl,-no-whole-archive'"
-                    : "          flag: '-Wl,-force_load,%{whole_archive_linker_params}'",
-                "       }",
-                "   }",
-                "   flag_set {",
-                "       expand_if_all_available: 'linker_input_params'",
-                "       action: 'c++-link-executable'",
-                "       action: 'c++-link-dynamic-library'",
-                "       action: 'c++-link-static-library'",
-                "       action: 'c++-link-alwayslink-static-library'",
-                "       action: 'c++-link-pic-static-library'",
-                "       action: 'c++-link-alwayslink-pic-static-library'",
-                "       flag_group {",
-                "           flag: '%{linker_input_params}'",
-                "       }",
-                "   }",
                 "}",
                 "feature {",
-                "   name: 'global_whole_archive_close'",
+                "   name: 'libraries_to_link'",
                 "   flag_set {",
-                "       expand_if_all_available: 'global_whole_archive'",
+                "       expand_if_all_available: 'libraries_to_link'",
                 "       action: 'c++-link-executable'",
                 "       action: 'c++-link-dynamic-library'",
                 "       action: 'c++-link-static-library'",
@@ -313,10 +261,41 @@ public class CppLinkActionConfigs {
                 "       action: 'c++-link-pic-static-library'",
                 "       action: 'c++-link-alwayslink-pic-static-library'",
                 "       flag_group {",
-                // TODO: Factor platform difference into respective linux and OSX crosstools.
-                String.format(
-                    "           flag: '%s'",
-                    platform == CppLinkPlatform.LINUX ? "-Wl,-no-whole-archive" : ""),
+                "           iterate_over: 'libraries_to_link'",
+                "           flag_group {",
+                "               expand_if_true: 'libraries_to_link.is_lib_group'",
+                "               flag: '-Wl,--start-lib'",
+                "           }",
+                ifLinux(
+                    platform,
+                    "       flag_group {",
+                    "           expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "           flag: '-Wl,-whole-archive'",
+                    "       }",
+                    "       flag_group {",
+                    "           iterate_over: 'libraries_to_link.names'",
+                    "           flag: '%{libraries_to_link.names}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "           flag: '-Wl,-no-whole-archive'",
+                    "       }"),
+                ifMac(
+                    platform,
+                    "       flag_group {",
+                    "           expand_if_true: 'libraries_to_link.is_whole_archive'",
+                    "           iterate_over: 'libraries_to_link.names'",
+                    "           flag: '-Wl,-force_load,%{libraries_to_link.names}'",
+                    "       }",
+                    "       flag_group {",
+                    "           expand_if_false: 'libraries_to_link.is_whole_archive'",
+                    "           iterate_over: 'libraries_to_link.names'",
+                    "           flag: '%{libraries_to_link.names}'",
+                    "       }"),
+                "           flag_group {",
+                "               expand_if_true: 'libraries_to_link.is_lib_group'",
+                "               flag: '-Wl,--end-lib'",
+                "           }",
                 "       }",
                 "   }",
                 "}",
@@ -331,4 +310,20 @@ public class CppLinkActionConfigs {
                 "   }",
                 "}"));
   }
+
+  private static String ifLinux(CppLinkPlatform platform, String... lines) {
+    if (platform == CppLinkPlatform.LINUX) {
+      return Joiner.on("\n").join(lines);
+    } else {
+      return "";
+    }
+  }
+
+  private static String ifMac(CppLinkPlatform platform, String... lines) {
+    if (platform == CppLinkPlatform.MAC) {
+      return Joiner.on("\n").join(lines);
+    } else {
+      return "";
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index 0cb237e113..145088e321 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -73,7 +73,6 @@ public final class LinkCommandLine extends CommandLine {
   @Nullable private final PathFragment runtimeSolibDir;
   private final boolean nativeDeps;
   private final boolean useTestOnlyFlags;
-  private final List<String> noWholeArchiveFlags;
 
   @Nullable private final Artifact paramFile;
 
@@ -96,9 +95,7 @@ public final class LinkCommandLine extends CommandLine {
       @Nullable PathFragment runtimeSolibDir,
       boolean nativeDeps,
       boolean useTestOnlyFlags,
-      boolean needWholeArchive,
       @Nullable Artifact paramFile,
-      List<String> noWholeArchiveFlags,
       CcToolchainFeatures.Variables variables,
       @Nullable FeatureConfiguration featureConfiguration) {
 
@@ -126,7 +123,6 @@ public final class LinkCommandLine extends CommandLine {
     this.nativeDeps = nativeDeps;
     this.useTestOnlyFlags = useTestOnlyFlags;
     this.paramFile = paramFile;
-    this.noWholeArchiveFlags = noWholeArchiveFlags;
   }
 
   @Nullable
@@ -380,14 +376,12 @@ public final class LinkCommandLine extends CommandLine {
       case EXECUTABLE:
         argv.add(cppConfiguration.getCppExecutable().getPathString());
         argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
-        argv.addAll(noWholeArchiveFlags);
         addToolchainFlags(argv);
         break;
 
       case DYNAMIC_LIBRARY:
         argv.add(toolPath);
         argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
-        argv.addAll(noWholeArchiveFlags);
         addToolchainFlags(argv);
         break;
 
@@ -401,7 +395,6 @@ public final class LinkCommandLine extends CommandLine {
         argv.addAll(cppConfiguration.getArFlags());
         argv.add(output.getExecPathString());
         argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
-        argv.addAll(noWholeArchiveFlags);
         break;
 
         // Since the objc case is not hardcoded in CppConfiguration, we can use the actual tool.
@@ -417,7 +410,6 @@ public final class LinkCommandLine extends CommandLine {
       default:
         throw new IllegalArgumentException();
     }
-
     return argv;
   }
 
@@ -641,12 +633,10 @@ public final class LinkCommandLine extends CommandLine {
     @Nullable private PathFragment runtimeSolibDir;
     private boolean nativeDeps;
     private boolean useTestOnlyFlags;
-    private boolean needWholeArchive;
     @Nullable private Artifact paramFile;
     @Nullable private CcToolchainProvider toolchain;
     private Variables variables;
     private FeatureConfiguration featureConfiguration;
-    private List<String> noWholeArchiveFlags = ImmutableList.of();
 
     // This interface is needed to support tests that don't create a
     // ruleContext, in which case the configuration and action owner
@@ -719,9 +709,7 @@ public final class LinkCommandLine extends CommandLine {
           runtimeSolibDir,
           nativeDeps,
           useTestOnlyFlags,
-          needWholeArchive,
           paramFile,
-          noWholeArchiveFlags,
           variables,
           featureConfiguration);
     }
@@ -874,15 +862,5 @@ public final class LinkCommandLine extends CommandLine {
       this.runtimeSolibDir = runtimeSolibDir;
       return this;
     }
-
-    /**
-     * Set flags that should not be in a --whole_archive block.
-     *
-     * <p>TODO(b/30228443): Refactor into action_configs.
-     */
-    public Builder setNoWholeArchiveFlags(List<String> noWholeArchiveFlags) {
-      this.noWholeArchiveFlags = noWholeArchiveFlags;
-      return this;
-    }
   }
 }