Make C++ archiving use action_configs instead of hardcoded flags

RELNOTES: Use action_config in crosstool for static library archiving, remove ar_flag.
PiperOrigin-RevId: 157685703

4 files changed, 54 insertions, 54 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 7fc17ec06f..56cc775214 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -297,7 +297,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
 
   private final ImmutableList<String> objcopyOptions;
   private final ImmutableList<String> ldOptions;
-  private final ImmutableList<String> arOptions;
 
   private final ImmutableMap<String, String> additionalMakeVariables;
 
@@ -488,7 +487,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
 
     this.objcopyOptions = ImmutableList.copyOf(toolchain.getObjcopyEmbedFlagList());
     this.ldOptions = ImmutableList.copyOf(toolchain.getLdEmbedFlagList());
-    this.arOptions = copyOrDefaultIfEmpty(toolchain.getArFlagList(), "rcsD");
 
     this.abi = toolchain.getAbiVersion();
     this.abiGlibcVersion = toolchain.getAbiLibcVersion();
@@ -651,6 +649,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
       try {
         if (!linkActionsAreConfigured(toolchain)) {
           String linkerToolPath = "DUMMY_LINKER_TOOL";
+          String arToolPath = "DUMMY_AR_TOOL";
           for (ToolPath tool : toolchain.getToolPathList()) {
             if (tool.getName().equals(Tool.GCC.getNamePart())) {
               linkerToolPath =
@@ -658,18 +657,18 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
                       .getRelative(PathFragment.create(tool.getPath()))
                       .getPathString();
             }
+            if (tool.getName().equals(Tool.AR.getNamePart())) {
+              arToolPath = tool.getPath();
+            }
           }
-          if (getTargetLibc().equals("macosx")) {
-            TextFormat.merge(
-                CppLinkActionConfigs.getCppLinkActionConfigs(
-                    CppLinkPlatform.MAC, features, linkerToolPath, supportsEmbeddedRuntimes),
-                toolchainBuilder);
-          } else {
-            TextFormat.merge(
-                CppLinkActionConfigs.getCppLinkActionConfigs(
-                    CppLinkPlatform.LINUX, features, linkerToolPath, supportsEmbeddedRuntimes),
-                toolchainBuilder);
-          }
+          TextFormat.merge(
+              CppLinkActionConfigs.getCppLinkActionConfigs(
+                  getTargetLibc().equals("macosx") ? CppLinkPlatform.MAC : CppLinkPlatform.LINUX,
+                  features,
+                  linkerToolPath,
+                  arToolPath,
+                  supportsEmbeddedRuntimes),
+              toolchainBuilder);
         }
 
         if (!features.contains("dependency_file")) {
@@ -950,11 +949,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return toolchainBuilder.build();
   }
 
-  private static ImmutableList<String> copyOrDefaultIfEmpty(List<String> list,
-      String defaultValue) {
-    return list.isEmpty() ? ImmutableList.of(defaultValue) : ImmutableList.copyOf(list);
-  }
-
   @VisibleForTesting
   static CompilationMode importCompilationMode(CrosstoolConfig.CompilationMode mode) {
     return CompilationMode.valueOf(mode.name());
@@ -1265,13 +1259,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
-   * Returns the ar flags to be used.
-   */
-  public ImmutableList<String> getArFlags() {
-    return arOptions;
-  }
-
-  /**
    * Returns the built-in list of system include paths for the toolchain compiler. All paths in this
    * list should be relative to the exec directory. They may be absolute if they are also installed
    * on the remote build nodes or for local compilation.
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 18d7102092..0b34fb40b4 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
@@ -138,6 +138,13 @@ public class CppLinkActionBuilder {
   public static final String IS_CC_TEST_LINK_ACTION_VARIABLE = "is_cc_test_link_action";
 
   /**
+   * Temporary build variable for migrating osx crosstool.
+   * TODO(b/37271982): Remove after blaze with ar action_config release
+   */
+  public static final String USES_ACTION_CONFIG_FOR_AR_VARIABLE =
+      "uses_action_configs_for_cc_archiving";
+
+  /**
    *  A build variable whose presence indicates that files were compiled with fission (debug
    *  info is in .dwo files instead of .o files and linker needs to know).
    */
@@ -1435,6 +1442,9 @@ public class CppLinkActionBuilder {
         buildVariables.addStringVariable(IS_NOT_CC_TEST_LINK_ACTION_VARIABLE, "");
       }
 
+      // TODO(b/37271982): Remove after blaze with ar action_config release
+      buildVariables.addStringVariable(USES_ACTION_CONFIG_FOR_AR_VARIABLE, "");
+
       if (linkArgCollector.getRuntimeLibrarySearchDirectories() != null) {
         buildVariables.addStringSequenceVariable(
             RUNTIME_LIBRARY_SEARCH_DIRECTORIES_VARIABLE,
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 7a1c6da7b4..10fc419628 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
@@ -35,6 +35,7 @@ public class CppLinkActionConfigs {
       CppLinkPlatform platform,
       Set<String> features,
       String cppLinkDynamicLibraryToolPath,
+      String arToolPath,
       boolean supportsEmbeddedRuntimes) {
     String cppDynamicLibraryLinkerTool = "";
     if (!features.contains("dynamic_library_linker_tool")) {
@@ -98,11 +99,9 @@ public class CppLinkActionConfigs {
                 "   config_name: 'c++-link-static-library'",
                 "   action_name: 'c++-link-static-library'",
                 "   tool {",
-                "       tool_path: 'DUMMY_TOOL'",
+                "       tool_path: '" + arToolPath + "'",
                 "   }",
-                "   implies: 'strip_debug_symbols'",
-                "   implies: 'runtime_library_search_directories'",
-                "   implies: 'library_search_directories'",
+                "   implies: 'archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
@@ -110,11 +109,9 @@ public class CppLinkActionConfigs {
                 "   config_name: 'c++-link-alwayslink-static-library'",
                 "   action_name: 'c++-link-alwayslink-static-library'",
                 "   tool {",
-                "       tool_path: 'DUMMY_TOOL'",
+                "       tool_path: '" + arToolPath + "'",
                 "   }",
-                "   implies: 'strip_debug_symbols'",
-                "   implies: 'runtime_library_search_directories'",
-                "   implies: 'library_search_directories'",
+                "   implies: 'archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
@@ -122,11 +119,9 @@ public class CppLinkActionConfigs {
                 "   config_name: 'c++-link-pic-static-library'",
                 "   action_name: 'c++-link-pic-static-library'",
                 "   tool {",
-                "       tool_path: 'DUMMY_TOOL'",
+                "       tool_path: '" + arToolPath + "'",
                 "   }",
-                "   implies: 'strip_debug_symbols'",
-                "   implies: 'runtime_library_search_directories'",
-                "   implies: 'library_search_directories'",
+                "   implies: 'archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
@@ -134,11 +129,9 @@ public class CppLinkActionConfigs {
                 "   config_name: 'c++-link-alwayslink-pic-static-library'",
                 "   action_name: 'c++-link-alwayslink-pic-static-library'",
                 "   tool {",
-                "       tool_path: 'DUMMY_TOOL'",
+                "       tool_path: '" + arToolPath + "'",
                 "   }",
-                "   implies: 'strip_debug_symbols'",
-                "   implies: 'runtime_library_search_directories'",
-                "   implies: 'library_search_directories'",
+                "   implies: 'archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
@@ -233,10 +226,6 @@ public class CppLinkActionConfigs {
                 "       expand_if_all_available: 'runtime_library_search_directories'",
                 "       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 {",
                 "           iterate_over: 'runtime_library_search_directories'",
                 "           flag_group {",
@@ -260,10 +249,6 @@ public class CppLinkActionConfigs {
                 "       expand_if_all_available: 'library_search_directories'",
                 "       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 {",
                 "           iterate_over: 'library_search_directories'",
                 "           flag: '-L%{library_search_directories}'",
@@ -271,6 +256,28 @@ public class CppLinkActionConfigs {
                 "   }",
                 "}",
                 "feature {",
+                "  name: 'archiver_flags'",
+                "  flag_set {",
+                "    expand_if_all_available: 'output_execpath'",
+                "    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 {",
+                ifLinux(
+                    platform,
+                    "  flag: 'rcsD'",
+                    "  flag: '%{output_execpath}'"),
+                ifMac(
+                    platform,
+                    "  flag: '-static'",
+                    "  flag: '-s'",
+                    "  flag: '-o'",
+                    "  flag: '%{output_execpath}'"),
+                "    }",
+                "  }",
+                "}",
+                "feature {",
                 "   name: 'libraries_to_link'",
                 "   flag_set {",
                 "       expand_if_all_available: 'libraries_to_link'",
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 0ea0ccfb2b..2abda844fe 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
@@ -399,11 +399,7 @@ public final class LinkCommandLine extends CommandLine {
       case PIC_STATIC_LIBRARY:
       case ALWAYS_LINK_STATIC_LIBRARY:
       case ALWAYS_LINK_PIC_STATIC_LIBRARY:
-        // The static library link command follows this template:
-        // ar <cmd> <output_archive> <input_files...>
-        argv.add(cppConfiguration.getArExecutable().getPathString());
-        argv.addAll(cppConfiguration.getArFlags());
-        argv.add(output.getExecPathString());
+        argv.add(toolPath);
         argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
         break;