Add feature to enable gcc quoting for linker param files

This is the right quoting to use for most toolchains that are based on gcc or clang, but it breaks MSVC, so add a feature to control it. PiperOrigin-RevId: 471021383 Change-Id: Iad24e28512d07002b2f65bd2d5709d0f8252022b
bazelbuild · Aug 30, 2022 · a9e5a32 · a9e5a32
1 parent 9e084c4
commit a9e5a32
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 1 deletion.
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
@@ -947,13 +947,18 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
               Order.STABLE_ORDER,
               Iterables.filter(expandedLinkerArtifacts.toList(), Artifact::isTreeArtifact));
 
+      ParameterFile.ParameterFileType quoting =
+          featureConfiguration.isEnabled(CppRuleClasses.GCC_QUOTING_FOR_PARAM_FILES)
+              ? ParameterFile.ParameterFileType.GCC_QUOTED
+              : ParameterFile.ParameterFileType.UNQUOTED;
+
       Action parameterFileWriteAction =
           new ParameterFileWriteAction(
               getOwner(),
               paramFileActionInputs,
               paramFile,
               linkCommandLine.paramCmdLine(),
-              ParameterFile.ParameterFileType.UNQUOTED);
+              quoting);
       actionConstructionContext.registerAction(parameterFileWriteAction);
     }
 

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -454,6 +454,9 @@ public static ToolchainTypeRequirement ccToolchainTypeRequirement(RuleDefinition
 
   public static final String COMPILER_PARAM_FILE = "compiler_param_file";
 
+  /** A feature to use gcc quoting for linking param files. */
+  public static final String GCC_QUOTING_FOR_PARAM_FILES = "gcc_quoting_for_param_files";
+
   /**
    * A feature to indicate that this target generates debug symbols for a dSYM file. For Apple
    * platform only.

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl b/src/test/java/com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl
@@ -88,6 +88,7 @@ _FEATURE_NAMES = struct(
     dynamic_linking_mode = "dynamic_linking_mode",
     static_linking_mode = "static_linking_mode",
     compiler_param_file = "compiler_param_file",
+    gcc_quoting_for_param_files = "gcc_quoting_for_param_files",
     objcopy_embed_flags = "objcopy_embed_flags",
     ld_embed_flags = "ld_embed_flags",
     opt = "opt",
@@ -825,6 +826,11 @@ _compiler_param_file_feature = feature(
     enabled = True,
 )
 
+_gcc_quoting_for_param_files_feature = feature(
+    name = _FEATURE_NAMES.gcc_quoting_for_param_files,
+    enabled = True,
+)
+
 _static_link_cpp_runtimes_feature = feature(
     name = _FEATURE_NAMES.static_link_cpp_runtimes,
     enabled = True,
@@ -1333,6 +1339,7 @@ _feature_name_to_feature = {
     _FEATURE_NAMES.supports_pic: _supports_pic_feature,
     _FEATURE_NAMES.targets_windows: _targets_windows_feature,
     _FEATURE_NAMES.compiler_param_file: _compiler_param_file_feature,
+    _FEATURE_NAMES.gcc_quoting_for_param_files: _gcc_quoting_for_param_files_feature,
     _FEATURE_NAMES.module_maps: _module_maps_feature,
     _FEATURE_NAMES.static_link_cpp_runtimes: _static_link_cpp_runtimes_feature,
     _FEATURE_NAMES.simple_compile_feature: _simple_compile_feature,

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/mock/osx_cc_toolchain_config.bzl b/src/test/java/com/google/devtools/build/lib/packages/util/mock/osx_cc_toolchain_config.bzl
@@ -57,6 +57,11 @@ _default_feature = feature(
     ],
 )
 
+_gcc_quoting_for_param_files_feature = feature(
+    name = "gcc_quoting_for_param_files",
+    enabled = True,
+)
+
 _static_link_cpp_runtimes_feature = feature(
     name = "static_link_cpp_runtimes",
     enabled = True,
@@ -78,6 +83,7 @@ _parse_headers_feature = feature(
 
 _feature_name_to_feature = {
     "default_feature": _default_feature,
+    "gcc_quoting_for_param_files": _gcc_quoting_for_param_files_feature,
     "static_link_cpp_runtimes": _static_link_cpp_runtimes_feature,
     "supports_interface_shared_libraries": _supports_interface_shared_libraries_feature,
     "supports_dynamic_linker": _supports_dynamic_linker_feature,

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -434,6 +434,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
+        "//src/main/java/com/google/devtools/build/lib/analysis:actions/parameter_file_write_action",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
         "//src/main/java/com/google/devtools/build/lib/cmdline",

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
@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
+import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
 import com.google.devtools.build.lib.analysis.util.ActionTester;
 import com.google.devtools.build.lib.analysis.util.ActionTester.ActionCombinationFactory;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -1064,4 +1065,24 @@ public void testExposesLinkstampObjects() throws Exception {
     assertThat(artifactsToStrings(linkAction.getLinkstampObjectFileInputs()))
         .containsExactly("bin x/_objs/bin/x/linkstamp.o");
   }
+
+  @Test
+  public void testGccQuotingForParamFilesFeature_EnablesGccQuoting() throws Exception {
+    getAnalysisMock()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder().withFeatures(CppRuleClasses.GCC_QUOTING_FOR_PARAM_FILES));
+    useConfiguration();
+
+    scratch.file(
+        "foo/BUILD", "cc_binary(", "  name = 'foo',", "  srcs = ['space .cc', 'quote\".cc'],", ")");
+    ConfiguredTarget configuredTarget = getConfiguredTarget("//foo:foo");
+    CppLinkAction linkAction = (CppLinkAction) getGeneratingAction(configuredTarget, "foo/foo");
+    ParameterFileWriteAction parameterFileWriteAction = paramFileWriteActionForAction(linkAction);
+    assertThat(parameterFileWriteAction).isNotNull();
+
+    assertThat(parameterFileWriteAction.getStringContents()).contains("space\\ .o");
+    assertThat(parameterFileWriteAction.getStringContents()).contains("quote\\\".o");
+  }
 }