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: 153046587

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

@@ -36,6 +36,7 @@ public static String getCppActionConfigs(
       Set<String> features,
       String gccToolPath,
       String linkerToolPath,
+      String arToolPath,
       boolean supportsEmbeddedRuntimes) {
     String cppDynamicLibraryLinkerTool = "";
     if (!features.contains("dynamic_library_linker_tool")) {
@@ -155,47 +156,39 @@ public static String getCppActionConfigs(
                 "   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: 'static_library_archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   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: 'static_library_archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   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: 'static_library_archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   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: 'static_library_archiver_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'linker_param_file'",
                 "}",
@@ -305,10 +298,6 @@ public static String getCppActionConfigs(
                 "       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 {",
@@ -332,13 +321,31 @@ public static String getCppActionConfigs(
                 "       expand_if_all_available: 'library_search_directories'",
                 "       action: 'c++-link-executable'",
                 "       action: 'c++-link-dynamic-library'",
+                "       flag_group {",
+                "           iterate_over: 'library_search_directories'",
+                "           flag: '-L%{library_search_directories}'",
+                "       }",
+                "   }",
+                "}",
+                "feature {",
+                "   name: 'static_library_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 {",
-                "           iterate_over: 'library_search_directories'",
-                "           flag: '-L%{library_search_directories}'",
+                ifLinux(
+                    platform,
+                    "       flag: 'rcsD'",
+                    "       flag: '%{output_execpath}'"),
+                ifMac(
+                    platform,
+                    "       flag: '-static'",
+                    "       flag: '-s'",
+                    "       flag: '-o'",
+                    "       flag: '%{output_execpath}'"),
                 "       }",
                 "   }",
                 "}",

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

@@ -409,7 +409,6 @@ ImmutableList<String> evaluate(Iterable<String> features) {
 
   private final ImmutableList<String> objcopyOptions;
   private final ImmutableList<String> ldOptions;
-  private final ImmutableList<String> arOptions;
 
   private final ImmutableMap<String, String> additionalMakeVariables;
 
@@ -596,7 +595,6 @@ public boolean apply(Tool tool) {
 
     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();
@@ -776,6 +774,7 @@ private CToolchain addLegacyFeatures(CToolchain toolchain) {
         if (!actionsAreConfigured(toolchain)) {
           String gccToolPath = "DUMMY_GCC_TOOL";
           String linkerToolPath = "DUMMY_LINKER_TOOL";
+          String arToolPath = "DUMMY_AR_TOOL";
           for (ToolPath tool : toolchain.getToolPathList()) {
             if (tool.getName().equals(Tool.GCC.getNamePart())) {
               gccToolPath = tool.getPath();
@@ -784,6 +783,9 @@ private CToolchain addLegacyFeatures(CToolchain toolchain) {
                       .getRelative(PathFragment.create(tool.getPath()))
                       .getPathString();
             }
+            if (tool.getName().equals(Tool.AR.getNamePart())) {
+              arToolPath = tool.getPath();
+            }
           }
           if (getTargetLibc().equals("macosx")) {
             TextFormat.merge(
@@ -792,6 +794,7 @@ private CToolchain addLegacyFeatures(CToolchain toolchain) {
                     features,
                     gccToolPath,
                     linkerToolPath,
+                    arToolPath,
                     supportsEmbeddedRuntimes),
                 toolchainBuilder);
           } else {
@@ -801,6 +804,7 @@ private CToolchain addLegacyFeatures(CToolchain toolchain) {
                     features,
                     gccToolPath,
                     linkerToolPath,
+                    arToolPath,
                     supportsEmbeddedRuntimes),
                 toolchainBuilder);
           }
@@ -1064,11 +1068,6 @@ private CToolchain addLegacyFeatures(CToolchain toolchain) {
     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());
@@ -1386,13 +1385,6 @@ public Link.ArchiveType archiveType() {
     return useStartEndLib() ? Link.ArchiveType.START_END_LIB : Link.ArchiveType.REGULAR;
   }
 
-  /**
-   * 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.

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

@@ -130,6 +130,13 @@ public class CppLinkActionBuilder {
   /** A build variable whose presence indicates that this action is a cc_test linking action. */
   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_archiver";
+
   /**
    *  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).
@@ -1369,6 +1376,9 @@ public void addVariables(Variables.Builder buildVariables) {
         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,

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

@@ -396,11 +396,7 @@ CppLinkActionBuilder.LEGACY_LINK_FLAGS_VARIABLE, getToolchainFlags())
       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;
 

src/main/protobuf/crosstool_config.proto

@@ -384,6 +384,7 @@ message CToolchain {
   repeated string ld_embed_flag = 23;
   // Ar flags for combining object files into archives. If this is not set, it
   // defaults to "rcsD".
+  // TODO(b/37271982): Remove after blaze with ar action_config release
   repeated string ar_flag = 47;
   // Legacy field, ignored by Bazel.
   repeated string ar_thin_archives_flag = 48;

src/test/java/com/google/devtools/build/lib/analysis/mock/MOCK_CROSSTOOL

@@ -298,9 +298,6 @@ toolchain {
 
   builtin_sysroot: ""
   cxx_flag: "-std=c++0x"
-  ar_flag: "-static"
-  ar_flag: "-s"
-  ar_flag: "-o"
   linker_flag: "-lstdc++"
   cxx_builtin_include_directory: "/usr/include"
   cxx_builtin_include_directory: "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain"
@@ -341,9 +338,6 @@ toolchain {
 
   builtin_sysroot: ""
   cxx_flag: "-std=c++0x"
-  ar_flag: "-static"
-  ar_flag: "-s"
-  ar_flag: "-o"
   linker_flag: "-lstdc++"
   cxx_builtin_include_directory: "/usr/include"
   cxx_builtin_include_directory: "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain"
@@ -387,9 +381,6 @@ toolchain {
 
   builtin_sysroot: ""
   cxx_flag: "-std=c++0x"
-  ar_flag: "-static"
-  ar_flag: "-s"
-  ar_flag: "-o"
   linker_flag: "-lstdc++"
   cxx_builtin_include_directory: "/usr/include"
   cxx_builtin_include_directory: "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain"

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

@@ -88,6 +88,7 @@ private final FeatureConfiguration getMockFeatureConfiguration() throws Exceptio
                 ImmutableSet.<String>of(),
                 "gcc_tool",
                 "dynamic_library_linker_tool",
+                "ar_tool",
                 true))
         .getFeatureConfiguration(
             Link.LinkTargetType.EXECUTABLE.getActionName(),

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

@@ -223,7 +223,6 @@ public void testSimpleCompleteConfiguration() throws Exception {
 
     assertEquals(Arrays.asList("objcopy"), toolchain.getObjCopyOptionsForEmbedding());
     assertEquals(Arrays.<String>asList(), toolchain.getLdOptionsForEmbedding());
-    assertEquals(Arrays.asList("rcsD"), toolchain.getArFlags());
 
     assertThat(toolchain.getAdditionalMakeVariables().entrySet())
         .containsExactlyElementsIn(
@@ -297,7 +296,6 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
                 + "  objcopy_embed_flag: \"objcopy-embed-flag-A-2\"\n"
                 + "  ld_embed_flag: \"ld-embed-flag-A-1\"\n"
                 + "  ld_embed_flag: \"ld-embed-flag-A-2\"\n"
-                + "  ar_flag : \"ar-flag-A\"\n"
                 + "  compilation_mode_flags {\n"
                 + "    mode: FASTBUILD\n"
                 + "    compiler_flag: \"fastbuild-flag-A-1\"\n"
@@ -389,7 +387,6 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
                 + "  objcopy_embed_flag: \"objcopy-embed-flag-B-2\"\n"
                 + "  ld_embed_flag: \"ld-embed-flag-B-1\"\n"
                 + "  ld_embed_flag: \"ld-embed-flag-B-2\"\n"
-                + "  ar_flag : \"ar-flag-B\"\n"
                 + "  compilation_mode_flags {\n"
                 + "    mode: FASTBUILD\n"
                 + "    compiler_flag: \"fastbuild-flag-B-1\"\n"
@@ -582,7 +579,6 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
     assertEquals(
         Arrays.asList("ld-embed-flag-A-1", "ld-embed-flag-A-2"),
         toolchainA.getLdOptionsForEmbedding());
-    assertEquals(Arrays.asList("ar-flag-A"), toolchainA.getArFlags());
 
     assertThat(toolchainA.getAdditionalMakeVariables().entrySet())
         .containsExactlyElementsIn(

tools/cpp/CROSSTOOL

@@ -220,9 +220,6 @@ toolchain {
   tool_path { name: "dwp" path: "/usr/bin/dwp" }
   tool_path { name: "gcc" path: "osx_cc_wrapper.sh" }
   cxx_flag: "-std=c++0x"
-  ar_flag: "-static"
-  ar_flag: "-s"
-  ar_flag: "-o"
   linker_flag: "-lstdc++"
   linker_flag: "-undefined"
   linker_flag: "dynamic_lookup"

tools/cpp/CROSSTOOL.tpl

@@ -411,6 +411,7 @@ toolchain {
      tool {
          tool_path: 'wrapper/bin/msvc_link.bat'
      }
+     implies: 'cc_archiver_flags'
      implies: 'input_param_flags'
      implies: 'linker_param_file'
      implies: 'msvc_env'
@@ -422,6 +423,7 @@ toolchain {
      tool {
          tool_path: 'wrapper/bin/msvc_link.bat'
      }
+     implies: 'cc_archiver_flags'
      implies: 'input_param_flags'
      implies: 'linker_param_file'
      implies: 'msvc_env'
@@ -435,6 +437,7 @@ toolchain {
      tool {
          tool_path: 'wrapper/bin/msvc_link.bat'
      }
+     implies: 'cc_archiver_flags'
      implies: 'input_param_flags'
      implies: 'linker_param_file'
      implies: 'msvc_env'
@@ -446,6 +449,7 @@ toolchain {
      tool {
          tool_path: 'wrapper/bin/msvc_link.bat'
      }
+     implies: 'cc_archiver_flags'
      implies: 'input_param_flags'
      implies: 'linker_param_file'
      implies: 'msvc_env'
@@ -514,15 +518,26 @@ toolchain {
   }
 
   feature {
-    name: 'input_param_flags'
+    name: 'cc_archiver_flags'
     flag_set {
-      expand_if_all_available: 'library_search_directories'
-      action: 'c++-link-executable'
-      action: 'c++-link-dynamic-library'
+      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 {
+        flag: 'rcsD'
+        flag: '%{output_execpath}'
+      }
+    }
+  }
+
+  feature {
+    name: 'input_param_flags'
+    flag_set {
+      expand_if_all_available: 'library_search_directories'
+      action: 'c++-link-executable'
+      action: 'c++-link-dynamic-library'
       flag_group {
         iterate_over: 'library_search_directories'
         flag: "-L%{library_search_directories}"
@@ -532,10 +547,6 @@ toolchain {
       expand_if_all_available: 'libopts'
       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: '%{libopts}'
       }

tools/cpp/cc_configure.bzl

@@ -269,7 +269,6 @@ def _crosstool_content(repository_ctx, cc, cpu_value, darwin):
               repository_ctx, cc, "-pass-exit-codes"
           )
       ),
-      "ar_flag": ["-static", "-s", "-o"] if darwin else [],
       "cxx_builtin_include_directory": _get_cxx_inc_directories(repository_ctx, cc),
       "objcopy_embed_flag": ["-I", "binary"],
       "unfiltered_cxx_flag":