diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java index 6840cd1add7d39..f8f6cc7b4e6a7e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -79,14 +80,20 @@ public String getToolPath() { * @param overwrittenVariables: Variables that will overwrite original build variables. When null, * unmodified original variables are used. */ - protected List getArguments(@Nullable CcToolchainVariables overwrittenVariables) { + protected List getArguments( + @Nullable PathFragment parameterFilePath, + @Nullable CcToolchainVariables overwrittenVariables) { List commandLine = new ArrayList<>(); // first: The command name. commandLine.add(getToolPath()); // second: The compiler options. - commandLine.addAll(getCompilerOptions(overwrittenVariables)); + if (parameterFilePath != null) { + commandLine.add("@" + parameterFilePath.getSafePathString()); + } else { + commandLine.addAll(getCompilerOptions(overwrittenVariables)); + } return commandLine; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 267a1e0773d92c..bb442ada072c17 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -38,9 +38,11 @@ import com.google.devtools.build.lib.actions.ArtifactResolver; import com.google.devtools.build.lib.actions.CommandAction; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier; import com.google.devtools.build.lib.actions.ExecutionRequirements; +import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.extra.CppCompileInfo; @@ -73,6 +75,7 @@ import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -174,6 +177,9 @@ public class CppCompileAction extends AbstractAction private CcToolchainVariables overwrittenVariables = null; + private ParamFileActionInput paramFileActionInput; + private PathFragment paramFilePath; + /** * Creates a new action to compile C/C++ source files. * @@ -275,6 +281,13 @@ public class CppCompileAction extends AbstractAction this.topLevelModules = null; this.overwrittenVariables = null; this.grepIncludes = grepIncludes; + if (featureConfiguration.isEnabled(CppRuleClasses.COMPIILER_PARAM_FILE)) { + paramFilePath = + outputFile + .getExecPath() + .getParentDirectory() + .getChild(outputFile.getFilename() + ".params"); + } } /** @@ -684,7 +697,11 @@ public ImmutableMap getEnvironment(Map clientEnv @Override public List getArguments() { - return compileCommandLine.getArguments(overwrittenVariables); + return compileCommandLine.getArguments(paramFilePath, overwrittenVariables); + } + + public ParamFileActionInput getParamFileActionInput() { + return paramFileActionInput; } @Override @@ -1106,6 +1123,15 @@ private byte[] computeCommandLineKey(List compilerOptions) { public ActionResult execute(ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException { setModuleFileFlags(); + if (featureConfiguration.isEnabled(CppRuleClasses.COMPIILER_PARAM_FILE)) { + paramFileActionInput = + new ParamFileActionInput( + paramFilePath, + compileCommandLine.getCompilerOptions(overwrittenVariables), + ParameterFileType.SHELL_QUOTED, + StandardCharsets.ISO_8859_1); + } + CppCompileActionContext.Reply reply; if (!shouldScanDotdFiles()) { 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 index 3168624b34f783..dd9069e9feb403 100644 --- 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 @@ -395,6 +395,8 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) { */ public static final String DO_NOT_SPLIT_LINKING_CMDLINE = "do_not_split_linking_cmdline"; + public static final String COMPIILER_PARAM_FILE = "compiler_param_file"; + /** Ancestor for all rules that do include scanning. */ public static final class CcIncludeScanningRule implements RuleDefinition { @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java index 8cc28f7a08f4ef..842b250661baf6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java @@ -58,11 +58,14 @@ public CppCompileActionResult execWithReply( throws ExecException, InterruptedException { // Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed // for execution. - ImmutableList inputs = + ImmutableList.Builder inputsBuilder = new ImmutableList.Builder() .addAll(action.getMandatoryInputs()) - .addAll(action.getAdditionalInputs()) - .build(); + .addAll(action.getAdditionalInputs()); + if (action.getParamFileActionInput() != null) { + inputsBuilder.add(action.getParamFileActionInput()); + } + ImmutableList inputs = inputsBuilder.build(); action.clearAdditionalInputs(); ImmutableMap executionInfo = action.getExecutionInfo(); 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 index f212a4a8275443..b2622864a8ff85 100644 --- 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 @@ -73,6 +73,7 @@ _FEATURE_NAMES = struct( link_env = "link_env", dynamic_linking_mode = "dynamic_linking_mode", static_linking_mode = "static_linking_mode", + compiler_param_file = "compiler_param_file", ) _no_legacy_features_feature = feature(name = _FEATURE_NAMES.no_legacy_features) @@ -587,6 +588,11 @@ _targets_windows_feature = feature( implies = ["copy_dynamic_libraries_to_binary"], ) +_compiler_param_file_feature = feature( + name = _FEATURE_NAMES.compiler_param_file, + enabled = True, +) + _static_link_cpp_runtimes_feature = feature( name = _FEATURE_NAMES.static_link_cpp_runtimes, enabled = True, @@ -682,6 +688,7 @@ _feature_name_to_feature = { _FEATURE_NAMES.supports_start_end_lib: _supports_start_end_lib_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.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/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index ef86df77cbf2e8..84739c46f281b7 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java @@ -70,11 +70,9 @@ public boolean apply(Artifact artifact) { public static final String SUPPORTS_INTERFACE_SHARED_LIBRARIES_FEATURE = "feature { name: '" + CppRuleClasses.SUPPORTS_INTERFACE_SHARED_LIBRARIES + "' enabled: true}"; - /** Feature expected by the C++ rules when pic build is requested */ public static final String PIC_FEATURE = - "" - + "feature {" + "feature {" + " name: 'pic'" + " enabled: true" + " flag_set {" @@ -94,8 +92,7 @@ public boolean apply(Artifact artifact) { /** A feature configuration snippet useful for testing header processing. */ public static final String PARSE_HEADERS_FEATURE_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'parse_headers'" + " flag_set {" + " action: 'c++-header-parsing'" @@ -107,8 +104,7 @@ public boolean apply(Artifact artifact) { /** A feature configuration snippet useful for testing the layering check. */ public static final String LAYERING_CHECK_FEATURE_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'layering_check'" + " flag_set {" + " action: 'c-compile'" @@ -122,11 +118,9 @@ public boolean apply(Artifact artifact) { + " }" + "}"; - /** A feature configuration snippet useful for testing header modules. */ public static final String HEADER_MODULES_FEATURE_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'header_modules'" + " implies: 'use_header_modules'" + " implies: 'header_module_compile'" @@ -181,8 +175,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String MODULE_MAP_HOME_CWD_FEATURE = - "" - + "feature {" + "feature {" + " name: 'module_map_home_cwd'" + " enabled: true" + " flag_set {" @@ -199,8 +192,7 @@ public boolean apply(Artifact artifact) { /** A feature configuration snippet useful for testing environment variables. */ public static final String ENV_VAR_FEATURE_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'env_feature'" + " implies: 'static_env_feature'" + " implies: 'module_maps'" @@ -234,8 +226,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String HOST_AND_NONHOST_CONFIGURATION = - "" - + "feature { " + "feature { " + " name: 'host'" + " flag_set {" + " action: 'c-compile'" @@ -257,8 +248,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String USER_COMPILE_FLAGS_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'user_compile_flags'" + " enabled: true" + " flag_set {" @@ -281,8 +271,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String LEGACY_COMPILE_FLAGS_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'legacy_compile_flags'" + " enabled: true" + " flag_set {" @@ -306,8 +295,7 @@ public boolean apply(Artifact artifact) { + "compiler_flag: 'legacy_compile_flag'"; public static final String THIN_LTO_CONFIGURATION = - "" - + "feature { " + "feature { " + " name: 'thin_lto'" + " requires { feature: 'nonhost' }" + " flag_set {" @@ -357,47 +345,43 @@ public boolean apply(Artifact artifact) { + "}"; public static final String THIN_LTO_LINKSTATIC_TESTS_USE_SHARED_NONLTO_BACKENDS_CONFIGURATION = - "" + "feature { name: 'thin_lto_linkstatic_tests_use_shared_nonlto_backends'}"; + "feature { name: 'thin_lto_linkstatic_tests_use_shared_nonlto_backends'}"; public static final String THIN_LTO_ALL_LINKSTATIC_USE_SHARED_NONLTO_BACKENDS_CONFIGURATION = - "" + "feature { name: 'thin_lto_all_linkstatic_use_shared_nonlto_backends'}"; + "feature { name: 'thin_lto_all_linkstatic_use_shared_nonlto_backends'}"; public static final String ENABLE_AFDO_THINLTO_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'enable_afdo_thinlto'" + " requires { feature: 'autofdo_implicit_thinlto' }" + " implies: 'thin_lto'" + "}"; public static final String AUTOFDO_IMPLICIT_THINLTO_CONFIGURATION = - "" + "feature { name: 'autofdo_implicit_thinlto'}"; + "feature { name: 'autofdo_implicit_thinlto'}"; public static final String ENABLE_FDO_THINLTO_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'enable_fdo_thinlto'" + " requires { feature: 'fdo_implicit_thinlto' }" + " implies: 'thin_lto'" + "}"; public static final String FDO_IMPLICIT_THINLTO_CONFIGURATION = - "" + "feature { name: 'fdo_implicit_thinlto'}"; + "feature { name: 'fdo_implicit_thinlto'}"; public static final String ENABLE_XFDO_THINLTO_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'enable_xbinaryfdo_thinlto'" + " requires { feature: 'xbinaryfdo_implicit_thinlto' }" + " implies: 'thin_lto'" + "}"; public static final String XFDO_IMPLICIT_THINLTO_CONFIGURATION = - "" + "feature { name: 'xbinaryfdo_implicit_thinlto'}"; + "feature { name: 'xbinaryfdo_implicit_thinlto'}"; public static final String AUTO_FDO_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'autofdo'" + " provides: 'profile'" + " flag_set {" @@ -416,8 +400,7 @@ public boolean apply(Artifact artifact) { "feature { name: 'is_cc_fake_binary' }"; public static final String XBINARY_FDO_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'xbinaryfdo'" + " provides: 'profile'" + " flag_set {" @@ -434,8 +417,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String FDO_OPTIMIZE_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'fdo_optimize'" + " provides: 'profile'" + " flag_set {" @@ -453,8 +435,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String FDO_INSTRUMENT_CONFIGURATION = - "" - + "feature { " + "feature { " + " name: 'fdo_instrument'" + " provides: 'profile'" + " flag_set {" @@ -471,8 +452,7 @@ public boolean apply(Artifact artifact) { + "}"; public static final String PER_OBJECT_DEBUG_INFO_CONFIGURATION = - "" - + "feature { " + "feature { " + " name: 'per_object_debug_info'" + " enabled: true" + " flag_set {" @@ -490,41 +470,37 @@ public boolean apply(Artifact artifact) { + "}"; public static final String COPY_DYNAMIC_LIBRARIES_TO_BINARY_CONFIGURATION = - "" + "feature { name: 'copy_dynamic_libraries_to_binary' }"; + "feature { name: 'copy_dynamic_libraries_to_binary' }"; public static final String SUPPORTS_START_END_LIB_FEATURE = - "" + "feature { name: 'supports_start_end_lib' enabled: true }"; + "feature { name: 'supports_start_end_lib' enabled: true }"; public static final String SUPPORTS_PIC_FEATURE = - "" + "feature { name: 'supports_pic' enabled: true }"; + "feature { name: 'supports_pic' enabled: true }"; public static final String TARGETS_WINDOWS_CONFIGURATION = - "" - + "feature {" + "feature {" + " name: 'targets_windows'" + " implies: 'copy_dynamic_libraries_to_binary'" + " enabled: true" + "}"; public static final String STATIC_LINK_TWEAKED_CONFIGURATION = - "" - + "artifact_name_pattern {" + "artifact_name_pattern {" + " category_name: 'static_library'" + " prefix: 'lib'" + " extension: '.lib'" + "}"; public static final String STATIC_LINK_AS_DOT_A_CONFIGURATION = - "" - + "artifact_name_pattern {" + "artifact_name_pattern {" + " category_name: 'static_library'" + " prefix: 'lib'" + " extension: '.a'" + "}"; public static final String MODULE_MAPS_FEATURE = - "" - + "feature {" + "feature {" + " name: 'module_maps'" + " enabled: true" + " flag_set {" @@ -539,6 +515,9 @@ public boolean apply(Artifact artifact) { + " }" + "}"; + public static final String COMPILER_PARAM_FILE = + "feature { name: 'compiler_param_file' enabled: true }"; + public static final String EMPTY_COMPILE_ACTION_CONFIG = emptyActionConfigFor(CppActionNames.CPP_COMPILE); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java index 9d76c5f9427dc6..067e5df8d49fd9 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java @@ -1068,4 +1068,22 @@ public void testWhenSupportsPicNotPresentAndForcePicPassedIsError() throws Excep "PIC compilation is requested but the toolchain does not support it" + " (feature named 'supports_pic' is not enabled"); } + + @Test + public void testCompilationParameterFile() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool(mockToolsConfig, MockCcSupport.COMPILER_PARAM_FILE); + scratch.file("a/BUILD", "cc_library(name='foo', srcs=['foo.cc'])"); + CppCompileAction cppCompileAction = getCppCompileAction("//a:foo"); + assertThat( + cppCompileAction.getArguments().stream() + .map(x -> removeOutDirectory(x)) + .collect(ImmutableList.toImmutableList())) + .containsExactly("/usr/bin/mock-gcc", "@/k8-fastbuild/bin/a/_objs/foo/foo.o.params"); + } + + private String removeOutDirectory(String s) { + return s.replace("blaze-out", "").replace("bazel-out", ""); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java index b9dbcc39d1d5b1..bf4c600e2bf15c 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java @@ -99,7 +99,9 @@ public void testFeatureConfigurationCommandLineIsUsed() throws Exception { " }", "}")) .build(); - assertThat(compileCommandLine.getArguments(/* overwrittenVariables= */ null)) + assertThat( + compileCommandLine.getArguments( + /* parameterFilePath= */ null, /* overwrittenVariables= */ null)) .contains("-some_foo_flag"); } @@ -142,7 +144,8 @@ private List getCompileCommandLineWithCoptsFilter(String featureName) th "}")) .setCoptsFilter(CoptsFilter.fromRegex(Pattern.compile(".*i_am_a_flag.*"))) .build(); - return compileCommandLine.getArguments(/* overwrittenVariables= */ null); + return compileCommandLine.getArguments( + /* parameterFilePath= */ null, /* overwrittenVariables= */ null); } private CompileCommandLine.Builder makeCompileCommandLineBuilder() throws Exception { diff --git a/src/test/py/bazel/bazel_windows_test.py b/src/test/py/bazel/bazel_windows_test.py index 32bc97c380f530..6e15dcad2b91e9 100644 --- a/src/test/py/bazel/bazel_windows_test.py +++ b/src/test/py/bazel/bazel_windows_test.py @@ -45,6 +45,23 @@ def testWindowsUnixRoot(self): ['--batch', 'build', '//foo:x', '--cpu=x64_windows_msys']) self.AssertExitCode(exit_code, 0, stderr) + def testWindowsParameterFile(self): + self.createProjectFiles() + + _, stdout, _ = self.RunBazel(['info', 'bazel-bin']) + bazel_bin = stdout[0] + + exit_code, _, stderr = self.RunBazel([ + 'build', + '--materialize_param_files', + '--features=compiler_param_file', + '//foo:x', + ]) + + self.AssertExitCode(exit_code, 0, stderr) + self.assertTrue( + os.path.exists(os.path.join(bazel_bin, 'foo\\_objs\\x\\x.obj.params'))) + def testWindowsCompilesAssembly(self): self.ScratchFile('WORKSPACE') exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin']) diff --git a/tools/cpp/cc_toolchain_config.bzl.tpl b/tools/cpp/cc_toolchain_config.bzl.tpl index 01dbefc9f4d3f4..d0248eb43b57b6 100644 --- a/tools/cpp/cc_toolchain_config.bzl.tpl +++ b/tools/cpp/cc_toolchain_config.bzl.tpl @@ -325,6 +325,10 @@ def _windows_msvc_impl(ctx): ], ) + compiler_param_file_feature = feature( + name = "compiler_param_file", + ) + copy_dynamic_libraries_to_binary_feature = feature(name = "copy_dynamic_libraries_to_binary") input_param_flags_feature = feature( @@ -1054,6 +1058,7 @@ def _windows_msvc_impl(ctx): user_compile_flags_feature, sysroot_feature, unfiltered_compile_flags_feature, + compiler_param_file_feature, compiler_output_flags_feature, compiler_input_flags_feature, def_file_feature, @@ -1226,6 +1231,10 @@ def _windows_msys_mingw_impl(ctx): ], ) + compiler_param_file_feature = feature( + name = "compiler_param_file", + ) + default_link_flags_feature = feature( name = "default_link_flags", enabled = True, @@ -1244,6 +1253,7 @@ def _windows_msys_mingw_impl(ctx): copy_dynamic_libraries_to_binary_feature, gcc_env_feature, default_compile_flags_feature, + compiler_param_file_feature, default_link_flags_feature, supports_dynamic_linker_feature, ]