diff options
author | hlopko <hlopko@google.com> | 2018-03-27 02:49:49 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-03-27 02:51:41 -0700 |
commit | 31ab0b88ec52f293e713b9369ea4a706b6c0a57d (patch) | |
tree | 5429052cca9ce7033faac9733f3ad83cd125da31 /src/main/java | |
parent | 8c864137d46cf6c3714fb47799a34e4859aa2f13 (diff) |
Isolate C++ compile build variables
This is a preparation work to expose Variables instance for all compile actions
to Skylark.
I didn't do linking variables in this cl, because this cl is already too big.
But they're coming shortly in a separate cl.
This is also in line with our goal to make build variables more discoverable and
better document.
RELNOTES: None.
PiperOrigin-RevId: 190591080
Diffstat (limited to 'src/main/java')
9 files changed, 585 insertions, 485 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBuildVariables.java new file mode 100644 index 0000000000..42b5b6d4f1 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBuildVariables.java @@ -0,0 +1,335 @@ +// Copyright 2014 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.rules.cpp; + +import com.google.common.base.Supplier; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.RuleContext; +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.StringSequenceBuilder; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension; +import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Collection; + +/** + * Utility class for collecting knowledge about what build variables we create and when we create + * them. Having this central location is useful so we can reason about what variables expose to + * Skylark, what variables are redundant etc. + */ +public class CcBuildVariables { + + /** Enum covering all build variables we create for 'strip' actions. */ + public enum StripBuildVariables { + /** Variable for the path to the compilation output file. */ + OUTPUT_FILE("output_file"), + /** Variable for stripopts. */ + STRIPOPTS("stripopts"); + + private final String variableName; + + StripBuildVariables(String variableName) { + this.variableName = variableName; + } + + public String getVariableName() { + return variableName; + } + } + + /** Enum covering all build variables we create for all various {@link CppCompileAction}. */ + public enum CompileBuildVariables { + /** Variable for the path to the source file being compiled. */ + SOURCE_FILE("source_file"), + /** + * Variable for all flags coming from copt rule attribute, and from --copt, --cxxopt, or + * --conlyopt options. + */ + USER_COMPILE_FLAGS("user_compile_flags"), + /** + * Variable for all flags coming from legacy crosstool fields, such as compiler_flag, + * optional_compiler_flag, cxx_flag, optional_cxx_flag. + */ + LEGACY_COMPILE_FLAGS("legacy_compile_flags"), + /** Variable for flags coming from unfiltered_cxx_flag CROSSTOOL fields. */ + UNFILTERED_COMPILE_FLAGS("unfiltered_compile_flags"), + /** Variable for the path to the output file when output is an object file. */ + OUTPUT_OBJECT_FILE("output_object_file"), + /** Variable for the path to the compilation output file. */ + OUTPUT_FILE("output_file"), + /** Variable for the dependency file path */ + DEPENDENCY_FILE("dependency_file"), + /** Variable for the module file name. */ + MODULE_NAME("module_name"), + /** + * Variable for the collection of include paths. + * + * @see CcCompilationInfo#getIncludeDirs(). + */ + INCLUDE_PATHS("include_paths"), + /** + * Variable for the collection of quote include paths. + * + * @see CcCompilationInfo#getIncludeDirs(). + */ + QUOTE_INCLUDE_PATHS("quote_include_paths"), + /** + * Variable for the collection of system include paths. + * + * @see CcCompilationInfo#getIncludeDirs(). + */ + SYSTEM_INCLUDE_PATHS("system_include_paths"), + /** Variable for the module map file name. */ + MODULE_MAP_FILE("module_map_file"), + /** Variable for the dependent module map file name. */ + DEPENDENT_MODULE_MAP_FILES("dependent_module_map_files"), + /** Variable for the collection of module files. */ + MODULE_FILES("module_files"), + /** Variable for the collection of macros defined for preprocessor. */ + PREPROCESSOR_DEFINES("preprocessor_defines"), + /** Variable for the gcov coverage file path. */ + GCOV_GCNO_FILE("gcov_gcno_file"), + /** Variable for the LTO indexing bitcode file. */ + LTO_INDEXING_BITCODE_FILE("lto_indexing_bitcode_file"), + /** Variable for the per object debug info file. */ + PER_OBJECT_DEBUG_INFO_FILE("per_object_debug_info_file"), + /** Variable present when the output is compiled as position independent. */ + PIC("pic"), + /** Variable marking that we are generating preprocessed sources (from --save_temps). */ + OUTPUT_PREPROCESS_FILE("output_preprocess_file"), + /** Variable marking that we are generating assembly source (from --save_temps). */ + OUTPUT_ASSEMBLY_FILE("output_assembly_file"), + /** Path to the fdo instrument artifact */ + FDO_INSTRUMENT_PATH("fdo_instrument_path"), + /** Path to the fdo profile artifact */ + FDO_PROFILE_PATH("fdo_profile_path"), + /** Variable for includes that compiler needs to include into sources. */ + INCLUDES("includes"); + + private final String variableName; + + CompileBuildVariables(String variableName) { + this.variableName = variableName; + } + + public String getVariableName() { + return variableName; + } + } + + public static Variables setupCompileBuildVariables( + RuleContext ruleContext, + FeatureConfiguration featureConfiguration, + CcToolchainProvider ccToolchainProvider, + Artifact sourceFile, + Artifact outputFile, + Artifact gcnoFile, + Artifact dwoFile, + Artifact ltoIndexingFile, + CcCompilationInfo ccCompilationInfo, + ImmutableList<String> includes, + ImmutableList<String> userCompileFlags, + CppModuleMap cppModuleMap, + boolean usePic, + PathFragment realOutputFilePath, + String fdoStamp, + String dotdFileExecPath, + ImmutableList<VariablesExtension> variablesExtensions, + ImmutableMap<String, String> additionalBuildVariables, + // TODO(b/76449614): Remove use of optional_*_flag from CROSSTOOL and get rid of this param + ImmutableSet<String> features) { + Variables.Builder buildVariables = + new Variables.Builder(ccToolchainProvider.getBuildVariables()); + + buildVariables.addStringSequenceVariable( + CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName(), userCompileFlags); + + buildVariables.addStringVariable( + CompileBuildVariables.SOURCE_FILE.getVariableName(), sourceFile.getExecPathString()); + + String sourceFilename = sourceFile.getExecPathString(); + buildVariables.addLazyStringSequenceVariable( + CompileBuildVariables.LEGACY_COMPILE_FLAGS.getVariableName(), + getLegacyCompileFlagsSupplier( + ruleContext.getFragment(CppConfiguration.class), + ccToolchainProvider, + sourceFilename, + features)); + + if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename) + && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) { + buildVariables.addLazyStringSequenceVariable( + CompileBuildVariables.UNFILTERED_COMPILE_FLAGS.getVariableName(), + getUnfilteredCompileFlagsSupplier(ccToolchainProvider, features)); + } + + // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are updated. + if (!FileType.contains( + outputFile, + CppFileTypes.ASSEMBLER, + CppFileTypes.PIC_ASSEMBLER, + CppFileTypes.PREPROCESSED_C, + CppFileTypes.PREPROCESSED_CPP, + CppFileTypes.PIC_PREPROCESSED_C, + CppFileTypes.PIC_PREPROCESSED_CPP)) { + buildVariables.addStringVariable( + CompileBuildVariables.OUTPUT_OBJECT_FILE.getVariableName(), + realOutputFilePath.getSafePathString()); + } + buildVariables.addStringVariable( + CompileBuildVariables.OUTPUT_FILE.getVariableName(), + realOutputFilePath.getSafePathString()); + + // Set dependency_file to enable <object>.d file generation. + if (dotdFileExecPath != null) { + buildVariables.addStringVariable( + CompileBuildVariables.DEPENDENCY_FILE.getVariableName(), dotdFileExecPath); + } + + if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) { + // If the feature is enabled and cppModuleMap is null, we are about to fail during analysis + // in any case, but don't crash. + buildVariables.addStringVariable( + CompileBuildVariables.MODULE_NAME.getVariableName(), cppModuleMap.getName()); + buildVariables.addStringVariable( + CompileBuildVariables.MODULE_MAP_FILE.getVariableName(), + cppModuleMap.getArtifact().getExecPathString()); + StringSequenceBuilder sequence = new StringSequenceBuilder(); + for (Artifact artifact : ccCompilationInfo.getDirectModuleMaps()) { + sequence.addValue(artifact.getExecPathString()); + } + buildVariables.addCustomBuiltVariable( + CompileBuildVariables.DEPENDENT_MODULE_MAP_FILES.getVariableName(), sequence); + } + if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) { + // Module inputs will be set later when the action is executed. + buildVariables.addStringSequenceVariable( + CompileBuildVariables.MODULE_FILES.getVariableName(), ImmutableSet.of()); + } + if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) { + buildVariables.addStringSequenceVariable( + CompileBuildVariables.INCLUDE_PATHS.getVariableName(), + getSafePathStrings(ccCompilationInfo.getIncludeDirs())); + buildVariables.addStringSequenceVariable( + CompileBuildVariables.QUOTE_INCLUDE_PATHS.getVariableName(), + getSafePathStrings(ccCompilationInfo.getQuoteIncludeDirs())); + buildVariables.addStringSequenceVariable( + CompileBuildVariables.SYSTEM_INCLUDE_PATHS.getVariableName(), + getSafePathStrings(ccCompilationInfo.getSystemIncludeDirs())); + } + + if (!includes.isEmpty()) { + buildVariables.addStringSequenceVariable( + CompileBuildVariables.INCLUDES.getVariableName(), includes); + } + + if (featureConfiguration.isEnabled(CppRuleClasses.PREPROCESSOR_DEFINES)) { + ImmutableList<String> defines; + if (fdoStamp != null) { + // Stamp FDO builds with FDO subtype string + defines = + ImmutableList.<String>builder() + .addAll(ccCompilationInfo.getDefines()) + .add(CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoStamp + "\"") + .build(); + } else { + defines = ccCompilationInfo.getDefines(); + } + + buildVariables.addStringSequenceVariable( + CompileBuildVariables.PREPROCESSOR_DEFINES.getVariableName(), defines); + } + + if (usePic) { + if (!featureConfiguration.isEnabled(CppRuleClasses.PIC)) { + ruleContext.ruleError(CcCompilationHelper.PIC_CONFIGURATION_ERROR); + } + buildVariables.addStringVariable(CompileBuildVariables.PIC.getVariableName(), ""); + } + + if (gcnoFile != null) { + buildVariables.addStringVariable( + CompileBuildVariables.GCOV_GCNO_FILE.getVariableName(), gcnoFile.getExecPathString()); + } + + if (dwoFile != null) { + buildVariables.addStringVariable( + CompileBuildVariables.PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), + dwoFile.getExecPathString()); + } + + if (ltoIndexingFile != null) { + buildVariables.addStringVariable( + CompileBuildVariables.LTO_INDEXING_BITCODE_FILE.getVariableName(), + ltoIndexingFile.getExecPathString()); + } + + buildVariables.addAllStringVariables(additionalBuildVariables); + for (VariablesExtension extension : variablesExtensions) { + extension.addVariables(buildVariables); + } + + return buildVariables.build(); + } + + /** Get the safe path strings for a list of paths to use in the build variables. */ + private static ImmutableSet<String> getSafePathStrings(Collection<PathFragment> paths) { + ImmutableSet.Builder<String> result = ImmutableSet.builder(); + for (PathFragment path : paths) { + result.add(path.getSafePathString()); + } + return result.build(); + } + + /** + * Supplier that computes legacy_compile_flags lazily at the execution phase. + * + * <p>Dear friends of the lambda, this method exists to limit the scope of captured variables only + * to arguments (to prevent accidental capture of enclosing instance which could regress memory). + */ + private static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier( + CppConfiguration cppConfiguration, + CcToolchainProvider toolchain, + String sourceFilename, + ImmutableSet<String> features) { + return () -> { + ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder(); + legacyCompileFlags.addAll( + CppHelper.getCrosstoolCompilerOptions(cppConfiguration, toolchain, features)); + if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) + || CppFileTypes.CPP_HEADER.matches(sourceFilename) + || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) + || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { + legacyCompileFlags.addAll( + CppHelper.getCrosstoolCxxOptions(cppConfiguration, toolchain, features)); + } + return legacyCompileFlags.build(); + }; + } + + /** + * Supplier that computes unfiltered_compile_flags lazily at the execution phase. + * + * <p>Dear friends of the lambda, this method exists to limit the scope of captured variables only + * to arguments (to prevent accidental capture of enclosing instance which could regress memory). + */ + private static Supplier<ImmutableList<String>> getUnfilteredCompileFlagsSupplier( + CcToolchainProvider ccToolchain, ImmutableSet<String> features) { + return () -> ccToolchain.getUnfilteredCompilerOptions(features); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index a2da30d674..8d4abd3e6c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -77,15 +77,6 @@ public final class CcCommon { /** Name of the build variable for the path to the input file being processed. */ public static final String INPUT_FILE_VARIABLE_NAME = "input_file"; - /** - * Name of the build variable for includes that compiler needs to include into sources to be - * compiled. - */ - public static final String INCLUDES_VARIABLE_NAME = "includes"; - - /** Name of the build variable for stripopts for the strip action. */ - public static final String STRIPOPTS_VARIABLE_NAME = "stripopts"; - private static final String NO_COPTS_ATTRIBUTE = "nocopts"; /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 5ff2a934d9..c90c97696c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -20,8 +20,8 @@ import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Predicates; -import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableList.Builder; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -47,17 +47,16 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.rules.cpp.CcBuildVariables.CompileBuildVariables; import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; -import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.StringSequenceBuilder; +import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension; -import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.syntax.Type; -import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -89,95 +88,8 @@ public final class CcCompilationHelper { + "hidden_header_tokens" + OutputGroupInfo.INTERNAL_SUFFIX; - private static final String PIC_CONFIGURATION_ERROR = + public static final String PIC_CONFIGURATION_ERROR = "PIC compilation is requested but the toolchain does not support it"; - /** Name of the build variable for the path to the source file being compiled. */ - public static final String SOURCE_FILE_VARIABLE_NAME = "source_file"; - - /** Name of the build variable for the path to the compilation output file. */ - public static final String OUTPUT_FILE_VARIABLE_NAME = "output_file"; - - /** - * Build variable for all flags coming from copt rule attribute, and from --copt, --cxxopt, or - * --conlyopt options. - */ - public static final String USER_COMPILE_FLAGS_VARIABLE_NAME = "user_compile_flags"; - - /** - * Build variable for all flags coming from legacy crosstool fields, such as compiler_flag, - * optional_compiler_flag, cxx_flag, optional_cxx_flag. - */ - public static final String LEGACY_COMPILE_FLAGS_VARIABLE_NAME = "legacy_compile_flags"; - - /** Build variable for flags coming from unfiltered_cxx_flag CROSSTOOL fields. */ - public static final String UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME = "unfiltered_compile_flags"; - - /** - * Name of the build variable marking that we are generating assembly source (from --save_temps). - */ - private static final String OUTPUT_ASSEMBLY_FILE_VARIABLE_NAME = "output_assembly_file"; - - /** - * Name of the build variable marking that we are generating preprocessed sources (from - * --save_temps). - */ - public static final String OUTPUT_PREPROCESS_FILE_VARIABLE_NAME = "output_preprocess_file"; - - /** Name of the build variable for the path to the output file when output is an object file. */ - public static final String OUTPUT_OBJECT_FILE_VARIABLE_NAME = "output_object_file"; - - /** - * Name of the build variable for the collection of include paths. - * - * @see CcCompilationInfo#getIncludeDirs(). - */ - public static final String INCLUDE_PATHS_VARIABLE_NAME = "include_paths"; - - /** - * Name of the build variable for the collection of quote include paths. - * - * @see CcCompilationInfo#getIncludeDirs(). - */ - public static final String QUOTE_INCLUDE_PATHS_VARIABLE_NAME = "quote_include_paths"; - - /** - * Name of the build variable for the collection of system include paths. - * - * @see CcCompilationInfo#getIncludeDirs(). - */ - public static final String SYSTEM_INCLUDE_PATHS_VARIABLE_NAME = "system_include_paths"; - - /** Name of the build variable for the collection of macros defined for preprocessor. */ - public static final String PREPROCESSOR_DEFINES_VARIABLE_NAME = "preprocessor_defines"; - - /** Name of the build variable present when the output is compiled as position independent. */ - public static final String PIC_VARIABLE_NAME = "pic"; - - /** Name of the build variable for the dependency file path */ - private static final String DEPENDENCY_FILE_VARIABLE_NAME = "dependency_file"; - - /** Name of the build variable for the module file name. */ - private static final String MODULE_NAME_VARIABLE_NAME = "module_name"; - - /** Name of the build variable for the module map file name. */ - private static final String MODULE_MAP_FILE_VARIABLE_NAME = "module_map_file"; - - /** Name of the build variable for the dependent module map file name. */ - private static final String DEPENDENT_MODULE_MAP_FILES_VARIABLE_NAME = - "dependent_module_map_files"; - - /** Name of the build variable for the collection of module files. */ - private static final String MODULE_FILES_VARIABLE_NAME = "module_files"; - - /** Name of the build variable for the gcov coverage file path. */ - private static final String GCOV_GCNO_FILE_VARIABLE_NAME = "gcov_gcno_file"; - - /** Name of the build variable for the per object debug info file. */ - private static final String PER_OBJECT_DEBUG_INFO_FILE_VARIABLE_NAME = - "per_object_debug_info_file"; - - /** Name of the build variable for the LTO indexing bitcode file. */ - private static final String LTO_INDEXING_BITCODE_FILE_VARIABLE_NAME = "lto_indexing_bitcode_file"; /** * A group of source file types and action names for builds controlled by CcCompilationHelper. @@ -1129,67 +1041,6 @@ public final class CcCompilationHelper { return this; } - /** - * Return flags that were specified on the Blaze command line. Take the filetype of sourceFilename - * into account. - */ - public static ImmutableList<String> getCoptsFromOptions( - CppConfiguration config, String sourceFilename) { - ImmutableList.Builder<String> flagsBuilder = ImmutableList.builder(); - - flagsBuilder.addAll(config.getCopts()); - - if (CppFileTypes.C_SOURCE.matches(sourceFilename)) { - flagsBuilder.addAll(config.getCOptions()); - } - - if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) - || CppFileTypes.CPP_HEADER.matches(sourceFilename) - || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) - || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { - flagsBuilder.addAll(config.getCxxopts()); - } - - return flagsBuilder.build(); - } - - /** - * Supplier that computes legacy_compile_flags lazily at the execution phase. - * - * <p>Dear friends of the lambda, this method exists to limit the scope of captured variables only - * to arguments (to prevent accidental capture of enclosing instance which could regress memory). - */ - public static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier( - CppConfiguration cppConfiguration, - CcToolchainProvider toolchain, - String sourceFilename, - ImmutableSet<String> features) { - return () -> { - ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder(); - legacyCompileFlags.addAll( - CppHelper.getCrosstoolCompilerOptions(cppConfiguration, toolchain, features)); - if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) - || CppFileTypes.CPP_HEADER.matches(sourceFilename) - || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) - || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { - legacyCompileFlags.addAll( - CppHelper.getCrosstoolCxxOptions(cppConfiguration, toolchain, features)); - } - return legacyCompileFlags.build(); - }; - } - - /** - * Supplier that computes unfiltered_compile_flags lazily at the execution phase. - * - * <p>Dear friends of the lambda, this method exists to limit the scope of captured variables only - * to arguments (to prevent accidental capture of enclosing instance which could regress memory). - */ - public static Supplier<ImmutableList<String>> getUnfilteredCompileFlagsSupplier( - CcToolchainProvider ccToolchain, ImmutableSet<String> features) { - return () -> ccToolchain.getUnfilteredCompilerOptions(features); - } - private UmbrellaHeaderAction createUmbrellaHeaderAction( Artifact umbrellaHeader, PublicHeaders publicHeaders) { return new UmbrellaHeaderAction( @@ -1500,18 +1351,19 @@ public final class CcCompilationHelper { CppHelper.getCompileOutputTreeArtifact(ruleContext, sourceArtifact, outputName, usePic); // TODO(rduan): Dotd file output is not supported yet. builder.setOutputs(outputFiles, /* dotdFile= */ null); - setupCompileBuildVariables( - builder, - source.getLabel(), - usePic, - /* ccRelativeName= */ null, - /* outputName= */ null, - /* autoFdoImportPath= */ null, - /* gcnoFile= */ null, - /* dwoFile= */ null, - /* ltoIndexingFile= */ null, - builder.getCcCompilationInfo().getCppModuleMap(), - /* additionalBuildVariables= */ ImmutableMap.of()); + builder.setVariables( + setupCompileBuildVariables( + builder, + /* sourceLabel= */ null, + /* outputName= */ null, + usePic, + /* ccRelativeName= */ null, + /* autoFdoImportPath= */ null, + ccCompilationInfo.getCppModuleMap(), + /* gcnoFile= */ null, + /* dwoFile= */ null, + /* ltoIndexingFile= */ null, + /* additionalBuildVariables= */ ImmutableMap.of())); semantics.finalizeCompileActionBuilder(ruleContext, builder); // Make sure this builder doesn't reference ruleContext outside of analysis phase. CppCompileActionTemplate actionTemplate = @@ -1527,160 +1379,90 @@ public final class CcCompilationHelper { return outputFiles; } - private void setupCompileBuildVariables( + /** + * Return flags that were specified on the Blaze command line. Take the filetype of sourceFilename + * into account. + */ + public static ImmutableList<String> getCoptsFromOptions( + CppConfiguration config, String sourceFilename) { + ImmutableList.Builder<String> flagsBuilder = ImmutableList.builder(); + + flagsBuilder.addAll(config.getCopts()); + + if (CppFileTypes.C_SOURCE.matches(sourceFilename)) { + flagsBuilder.addAll(config.getCOptions()); + } + + if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) + || CppFileTypes.CPP_HEADER.matches(sourceFilename) + || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) + || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { + flagsBuilder.addAll(config.getCxxopts()); + } + + return flagsBuilder.build(); + } + + private Variables setupCompileBuildVariables( CppCompileActionBuilder builder, Label sourceLabel, + String outputName, boolean usePic, PathFragment ccRelativeName, - String outputName, PathFragment autoFdoImportPath, + CppModuleMap cppModuleMap, Artifact gcnoFile, Artifact dwoFile, Artifact ltoIndexingFile, - CppModuleMap cppModuleMap, ImmutableMap<String, String> additionalBuildVariables) { - CcToolchainFeatures.Variables.Builder buildVariables = - new CcToolchainFeatures.Variables.Builder(ccToolchain.getBuildVariables()); - - CcCompilationInfo builderCcCompilationInfo = builder.getCcCompilationInfo(); Artifact sourceFile = builder.getSourceFile(); - String sourceFilename = sourceFile.getExecPathString(); - - buildVariables.addStringVariable(SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString()); - buildVariables.addStringSequenceVariable( - USER_COMPILE_FLAGS_VARIABLE_NAME, - ImmutableList.<String>builder() - // Add after the copts that come from the target. - .addAll(getCoptsFromOptions(cppConfiguration, sourceFilename)) - .addAll(copts) - .addAll(collectPerFileCopts(sourceFile, sourceLabel)) - .build()); - - buildVariables.addLazyStringSequenceVariable( - LEGACY_COMPILE_FLAGS_VARIABLE_NAME, - getLegacyCompileFlagsSupplier(cppConfiguration, ccToolchain, sourceFilename, features)); - - if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename) - && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) { - buildVariables.addLazyStringSequenceVariable( - UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME, - getUnfilteredCompileFlagsSupplier(ccToolchain, features)); - } - - // TODO(hlopko): Remove once blaze with this is released and crosstools are updated. - if (!FileType.contains( - builder.getOutputFile(), - CppFileTypes.ASSEMBLER, - CppFileTypes.PIC_ASSEMBLER, - CppFileTypes.PREPROCESSED_C, - CppFileTypes.PREPROCESSED_CPP, - CppFileTypes.PIC_PREPROCESSED_C, - CppFileTypes.PIC_PREPROCESSED_CPP)) { - buildVariables.addStringVariable( - OUTPUT_OBJECT_FILE_VARIABLE_NAME, builder.getRealOutputFilePath().getSafePathString()); - } - buildVariables.addStringVariable( - OUTPUT_FILE_VARIABLE_NAME, builder.getRealOutputFilePath().getSafePathString()); - - DotdFile dotdFile = - isGenerateDotdFile(sourceFile) ? Preconditions.checkNotNull(builder.getDotdFile()) : null; - // Set dependency_file to enable <object>.d file generation. - if (dotdFile != null) { - buildVariables.addStringVariable( - DEPENDENCY_FILE_VARIABLE_NAME, dotdFile.getSafeExecPath().getPathString()); - } - - if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) { - // If the feature is enabled and cppModuleMap is null, we are about to fail during analysis - // in any case, but don't crash. - buildVariables.addStringVariable(MODULE_NAME_VARIABLE_NAME, cppModuleMap.getName()); - buildVariables.addStringVariable( - MODULE_MAP_FILE_VARIABLE_NAME, cppModuleMap.getArtifact().getExecPathString()); - StringSequenceBuilder sequence = new StringSequenceBuilder(); - for (Artifact artifact : builderCcCompilationInfo.getDirectModuleMaps()) { - sequence.addValue(artifact.getExecPathString()); - } - buildVariables.addCustomBuiltVariable(DEPENDENT_MODULE_MAP_FILES_VARIABLE_NAME, sequence); - } - if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) { - // Module inputs will be set later when the action is executed. - buildVariables.addStringSequenceVariable(MODULE_FILES_VARIABLE_NAME, ImmutableSet.of()); - } - if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) { - buildVariables.addStringSequenceVariable( - INCLUDE_PATHS_VARIABLE_NAME, - getSafePathStrings(builderCcCompilationInfo.getIncludeDirs())); - buildVariables.addStringSequenceVariable( - QUOTE_INCLUDE_PATHS_VARIABLE_NAME, - getSafePathStrings(builderCcCompilationInfo.getQuoteIncludeDirs())); - buildVariables.addStringSequenceVariable( - SYSTEM_INCLUDE_PATHS_VARIABLE_NAME, - getSafePathStrings(builderCcCompilationInfo.getSystemIncludeDirs())); - } - - if (featureConfiguration.isEnabled(CppRuleClasses.PREPROCESSOR_DEFINES)) { - String fdoBuildStamp = CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()); - ImmutableList<String> defines; - if (fdoBuildStamp != null) { - // Stamp FDO builds with FDO subtype string - defines = - ImmutableList.<String>builder() - .addAll(builderCcCompilationInfo.getDefines()) - .add( - CppConfiguration.FDO_STAMP_MACRO - + "=\"" - + CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()) - + "\"") - .build(); - } else { - defines = builderCcCompilationInfo.getDefines(); - } - - buildVariables.addStringSequenceVariable(PREPROCESSOR_DEFINES_VARIABLE_NAME, defines); - } - - if (usePic) { - if (!featureConfiguration.isEnabled(CppRuleClasses.PIC)) { - ruleContext.ruleError(PIC_CONFIGURATION_ERROR); - } - buildVariables.addStringVariable(PIC_VARIABLE_NAME, ""); - } - + Builder<String> userCompileFlags = ImmutableList.builder(); + userCompileFlags.addAll(getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString())); + userCompileFlags.addAll(copts); + if (sourceFile != null && sourceLabel != null) { + userCompileFlags.addAll(collectPerFileCopts(sourceFile, sourceLabel)); + } + String dotdFileExecPath = null; + if (isGenerateDotdFile(builder.getSourceFile())) { + Preconditions.checkNotNull(builder.getDotdFile()); + dotdFileExecPath = builder.getDotdFile().getSafeExecPath().getPathString(); + } + ImmutableMap.Builder<String, String> allAdditionalBuildVariables = ImmutableMap.builder(); + allAdditionalBuildVariables.putAll(additionalBuildVariables); if (ccRelativeName != null) { - fdoSupport - .getFdoSupport() - .configureCompilation( - builder, - buildVariables, - ruleContext, - ccRelativeName, - autoFdoImportPath, - PathFragment.create(outputName), - usePic, - featureConfiguration, - fdoSupport); - } - if (gcnoFile != null) { - buildVariables.addStringVariable(GCOV_GCNO_FILE_VARIABLE_NAME, gcnoFile.getExecPathString()); - } - - if (dwoFile != null) { - buildVariables.addStringVariable( - PER_OBJECT_DEBUG_INFO_FILE_VARIABLE_NAME, dwoFile.getExecPathString()); - } - - if (ltoIndexingFile != null) { - buildVariables.addStringVariable( - LTO_INDEXING_BITCODE_FILE_VARIABLE_NAME, ltoIndexingFile.getExecPathString()); - } - - buildVariables.addAllStringVariables(additionalBuildVariables); - for (VariablesExtension extension : variablesExtensions) { - extension.addVariables(buildVariables); - } - - CcToolchainFeatures.Variables variables = buildVariables.build(); - builder.setVariables(variables); + allAdditionalBuildVariables.putAll( + fdoSupport + .getFdoSupport() + .configureCompilation( + builder, + ruleContext, + ccRelativeName, + autoFdoImportPath, + PathFragment.create(outputName), + usePic, + featureConfiguration, + fdoSupport)); + } + return CcBuildVariables.setupCompileBuildVariables( + ruleContext, + featureConfiguration, + ccToolchain, + sourceFile, + builder.getOutputFile(), + gcnoFile, + dwoFile, + ltoIndexingFile, + ccCompilationInfo, + ImmutableList.of(), + userCompileFlags.build(), + cppModuleMap, + usePic, + builder.getRealOutputFilePath(), + CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()), + dotdFileExecPath, + ImmutableList.copyOf(variablesExtensions), + allAdditionalBuildVariables.build(), + features); } /** @@ -1750,18 +1532,19 @@ public final class CcCompilationHelper { && CppFileTypes.LTO_SOURCE.matches(module.getFilename()); Preconditions.checkState(!bitcodeOutput); - setupCompileBuildVariables( - builder, - sourceLabel, - /* usePic= */ pic, - ccRelativeName, - outputName, - module.getExecPath(), - gcnoFile, - dwoFile, - /* ltoIndexingFile= */ null, - builder.getCcCompilationInfo().getCppModuleMap(), - /* additionalBuildVariables= */ ImmutableMap.of()); + builder.setVariables( + setupCompileBuildVariables( + builder, + sourceLabel, + outputName, + /* usePic= */ pic, + ccRelativeName, + module.getExecPath(), + ccCompilationInfo.getCppModuleMap(), + gcnoFile, + dwoFile, + /* ltoIndexingFile= */ null, + /* additionalBuildVariables= */ ImmutableMap.of())); builder.setGcnoFile(gcnoFile); builder.setDwoFile(dwoFile); @@ -1800,18 +1583,19 @@ public final class CcCompilationHelper { .setOutputs(ruleContext, ArtifactCategory.PROCESSED_HEADER, outputNameBase, generateDotd) // If we generate pic actions, we prefer the header actions to use the pic artifacts. .setPicMode(getGeneratePicActions()); - setupCompileBuildVariables( - builder, - sourceLabel, - this.getGeneratePicActions(), - /* ccRelativeName= */ null, - /* outputName= */ null, - /* autoFdoImportPath= */ null, - /* gcnoFile= */ null, - /* dwoFile= */ null, - /* ltoIndexingFile= */ null, - builder.getCcCompilationInfo().getCppModuleMap(), - /* additionalBuildVariables= */ ImmutableMap.of()); + builder.setVariables( + setupCompileBuildVariables( + builder, + sourceLabel, + /* outputName= */ null, + this.getGeneratePicActions(), + /* ccRelativeName= */ null, + /* autoFdoImportPath= */ null, + ccCompilationInfo.getCppModuleMap(), + /* gcnoFile= */ null, + /* dwoFile= */ null, + /* ltoIndexingFile= */ null, + /* additionalBuildVariables= */ ImmutableMap.of())); semantics.finalizeCompileActionBuilder(ruleContext, builder); CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext); env.registerAction(compileAction); @@ -1917,18 +1701,19 @@ public final class CcCompilationHelper { Artifact ltoIndexingFile = bitcodeOutput ? getLtoIndexingFile(picBuilder.getOutputFile()) : null; - setupCompileBuildVariables( - picBuilder, - sourceLabel, - /* usePic= */ true, - ccRelativeName, - outputName, - sourceArtifact.getExecPath(), - gcnoFile, - dwoFile, - ltoIndexingFile, - cppModuleMap, - /* additionalBuildVariables= */ ImmutableMap.of()); + picBuilder.setVariables( + setupCompileBuildVariables( + picBuilder, + sourceLabel, + outputName, + /* usePic= */ true, + ccRelativeName, + sourceArtifact.getExecPath(), + ccCompilationInfo.getCppModuleMap(), + gcnoFile, + dwoFile, + ltoIndexingFile, + /* additionalBuildVariables= */ ImmutableMap.of())); result.addTemps( createTempsActions( @@ -1986,18 +1771,19 @@ public final class CcCompilationHelper { Artifact ltoIndexingFile = bitcodeOutput ? getLtoIndexingFile(builder.getOutputFile()) : null; - setupCompileBuildVariables( - builder, - sourceLabel, - /* usePic= */ false, - ccRelativeName, - outputName, - sourceArtifact.getExecPath(), - gcnoFile, - noPicDwoFile, - ltoIndexingFile, - cppModuleMap, - /* additionalBuildVariables= */ ImmutableMap.of()); + builder.setVariables( + setupCompileBuildVariables( + builder, + sourceLabel, + outputName, + /* usePic= */ false, + ccRelativeName, + sourceArtifact.getExecPath(), + cppModuleMap, + gcnoFile, + noPicDwoFile, + ltoIndexingFile, + /* additionalBuildVariables= */ ImmutableMap.of())); result.addTemps( createTempsActions( @@ -2090,18 +1876,19 @@ public final class CcCompilationHelper { .setOutputs(ruleContext, outputCategory, outputNameBase, generateDotd) .setTempOutputFile(PathFragment.create(tempOutputName)); - setupCompileBuildVariables( - builder, - sourceLabel, - usePic, - ccRelativeName, - outputName, - execPath, - /* gcnoFile= */ null, - /* dwoFile= */ null, - /* ltoIndexingFile= */ null, - builder.getCcCompilationInfo().getCppModuleMap(), - /* additionalBuildVariables= */ ImmutableMap.of()); + builder.setVariables( + setupCompileBuildVariables( + builder, + sourceLabel, + outputName, + usePic, + ccRelativeName, + execPath, + ccCompilationInfo.getCppModuleMap(), + /* gcnoFile= */ null, + /* dwoFile= */ null, + /* ltoIndexingFile= */ null, + /* additionalBuildVariables= */ ImmutableMap.of())); semantics.finalizeCompileActionBuilder(ruleContext, builder); CppCompileAction action = builder.buildOrThrowRuleError(ruleContext); env.registerAction(action); @@ -2159,15 +1946,6 @@ public final class CcCompilationHelper { .collect(ImmutableList.toImmutableList()); } - /** Get the safe path strings for a list of paths to use in the build variables. */ - private ImmutableSet<String> getSafePathStrings(Collection<PathFragment> paths) { - ImmutableSet.Builder<String> result = ImmutableSet.builder(); - for (PathFragment path : paths) { - result.add(path.getSafePathString()); - } - return result.build(); - } - private Artifact getDwoFile(Artifact outputFile) { return ruleContext.getRelatedArtifact(outputFile.getRootRelativePath(), ".dwo"); } @@ -2206,20 +1984,21 @@ public final class CcCompilationHelper { CppCompileActionBuilder dBuilder = new CppCompileActionBuilder(builder); dBuilder.setOutputs(ruleContext, category, outputArtifactNameBase, generateDotd); - setupCompileBuildVariables( - dBuilder, - sourceLabel, - usePic, - ccRelativeName, - outputName, - source.getExecPath(), - /* gcnoFile= */ null, - /* dwoFile= */ null, - /* ltoIndexingFile= */ null, - builder.getCcCompilationInfo().getCppModuleMap(), - ImmutableMap.of( - OUTPUT_PREPROCESS_FILE_VARIABLE_NAME, - dBuilder.getRealOutputFilePath().getSafePathString())); + dBuilder.setVariables( + setupCompileBuildVariables( + dBuilder, + sourceLabel, + outputName, + usePic, + ccRelativeName, + source.getExecPath(), + ccCompilationInfo.getCppModuleMap(), + /* gcnoFile= */ null, + /* dwoFile= */ null, + /* ltoIndexingFile= */ null, + ImmutableMap.of( + CompileBuildVariables.OUTPUT_PREPROCESS_FILE.getVariableName(), + dBuilder.getRealOutputFilePath().getSafePathString()))); semantics.finalizeCompileActionBuilder(ruleContext, dBuilder); CppCompileAction dAction = dBuilder.buildOrThrowRuleError(ruleContext); ruleContext.registerAction(dAction); @@ -2227,20 +2006,21 @@ public final class CcCompilationHelper { CppCompileActionBuilder sdBuilder = new CppCompileActionBuilder(builder); sdBuilder.setOutputs( ruleContext, ArtifactCategory.GENERATED_ASSEMBLY, outputArtifactNameBase, generateDotd); - setupCompileBuildVariables( - sdBuilder, - sourceLabel, - usePic, - ccRelativeName, - outputName, - source.getExecPath(), - /* gcnoFile= */ null, - /* dwoFile= */ null, - /* ltoIndexingFile= */ null, - builder.getCcCompilationInfo().getCppModuleMap(), - ImmutableMap.of( - OUTPUT_ASSEMBLY_FILE_VARIABLE_NAME, - sdBuilder.getRealOutputFilePath().getSafePathString())); + sdBuilder.setVariables( + setupCompileBuildVariables( + sdBuilder, + sourceLabel, + outputName, + usePic, + ccRelativeName, + source.getExecPath(), + ccCompilationInfo.getCppModuleMap(), + /* gcnoFile= */ null, + /* dwoFile= */ null, + /* ltoIndexingFile= */ null, + ImmutableMap.of( + CompileBuildVariables.OUTPUT_ASSEMBLY_FILE.getVariableName(), + sdBuilder.getRealOutputFilePath().getSafePathString()))); semantics.finalizeCompileActionBuilder(ruleContext, sdBuilder); CppCompileAction sdAction = sdBuilder.buildOrThrowRuleError(ruleContext); ruleContext.registerAction(sdAction); 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 0477aef97a..bb267eda89 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 @@ -2147,6 +2147,10 @@ public class CcToolchainFeatures implements Serializable { enabledFeatureNames, enabledFeatures); } + + public ImmutableSet<String> getEnabledFeatureNames() { + return enabledFeatureNames; + } } /** All artifact name patterns defined in this feature configuration. */ 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 af40a0b39f..e15eaabe29 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 @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.rules.cpp.CcBuildVariables.CompileBuildVariables; import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; @@ -150,9 +151,9 @@ public final class CompileCommandLine { * explicit attribute, not using platform-dependent garbage bag that copts is). */ public ImmutableList<String> getCopts() { - if (variables.isAvailable(CcCompilationHelper.USER_COMPILE_FLAGS_VARIABLE_NAME)) { + if (variables.isAvailable(CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName())) { return Variables.toStringList( - variables, CcCompilationHelper.USER_COMPILE_FLAGS_VARIABLE_NAME); + variables, CompileBuildVariables.USER_COMPILE_FLAGS.getVariableName()); } else { return ImmutableList.of(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index 0264d56bd6..332ea96762 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.RuleErrorConsumer; +import com.google.devtools.build.lib.rules.cpp.CcBuildVariables.StripBuildVariables; import com.google.devtools.build.lib.rules.cpp.CcCompilationInfo.Builder; import com.google.devtools.build.lib.rules.cpp.CcLinkParams.Linkstamp; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -966,9 +967,9 @@ public class CppHelper { Variables variables = new Variables.Builder(toolchain.getBuildVariables()) .addStringVariable( - CcCompilationHelper.OUTPUT_FILE_VARIABLE_NAME, output.getExecPathString()) + StripBuildVariables.OUTPUT_FILE.getVariableName(), output.getExecPathString()) .addStringSequenceVariable( - CcCommon.STRIPOPTS_VARIABLE_NAME, cppConfiguration.getStripOpts()) + StripBuildVariables.STRIPOPTS.getVariableName(), cppConfiguration.getStripOpts()) .addStringVariable(CcCommon.INPUT_FILE_VARIABLE_NAME, input.getExecPathString()) .build(); ImmutableList<String> commandLine = 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 cb6776b25f..7167c49321 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 @@ -151,13 +151,6 @@ public class CppLinkActionBuilder { public static final String IS_CC_TEST_VARIABLE = "is_cc_test"; /** - * 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). */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java index 54f6839fe6..670137705a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java @@ -17,11 +17,12 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList.Builder; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; 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.vfs.PathFragment; import java.util.regex.Pattern; /** Handles creation of CppCompileAction used to compile linkstamp sources. */ @@ -55,6 +56,7 @@ public class CppLinkstampCompileHelper { .addMandatoryInputs(compilationInputs) .setVariables( getVariables( + ruleContext, sourceFile, outputFile, labelReplacement, @@ -125,6 +127,7 @@ public class CppLinkstampCompileHelper { } private static Variables getVariables( + RuleContext ruleContext, Artifact sourceFile, Artifact outputFile, String labelReplacement, @@ -140,58 +143,42 @@ public class CppLinkstampCompileHelper { // TODO(b/34761650): Remove all this hardcoding by separating a full blown compile action. Preconditions.checkArgument( featureConfiguration.actionIsConfigured(CppCompileAction.LINKSTAMP_COMPILE)); + CcCompilationInfo ccCompilationInfo = + new CcCompilationInfo.Builder(ruleContext) + .addIncludeDir(PathFragment.create(".")) + .addDefines( + computeAllLinkstampDefines( + labelReplacement, + outputReplacement, + additionalLinkstampDefines, + cppConfiguration, + fdoBuildStamp, + codeCoverageEnabled)) + .build(); - Variables.Builder variables = new Variables.Builder(ccToolchainProvider.getBuildVariables()); - // We need to force inclusion of build_info headers - variables.addStringSequenceVariable( - CcCommon.INCLUDES_VARIABLE_NAME, + return CcBuildVariables.setupCompileBuildVariables( + ruleContext, + featureConfiguration, + ccToolchainProvider, + sourceFile, + outputFile, + /* gcnoFile= */ null, + /* dwoFile= */ null, + /* ltoIndexingFile= */ null, + ccCompilationInfo, buildInfoHeaderArtifacts .stream() .map(Artifact::getExecPathString) - .collect(ImmutableList.toImmutableList())); - // Input/Output files. - variables.addStringVariable( - CcCompilationHelper.SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString()); - variables.addStringVariable( - CcCompilationHelper.OUTPUT_FILE_VARIABLE_NAME, outputFile.getExecPathString()); - variables.addStringVariable( - CcCompilationHelper.OUTPUT_OBJECT_FILE_VARIABLE_NAME, outputFile.getExecPathString()); - // Include directories for (normal includes with ".", empty quote- and system- includes). - variables.addStringSequenceVariable( - CcCompilationHelper.INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of(".")); - variables.addStringSequenceVariable( - CcCompilationHelper.QUOTE_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); - variables.addStringSequenceVariable( - CcCompilationHelper.SYSTEM_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); - // Legacy flags coming from fields such as compiler_flag - variables.addLazyStringSequenceVariable( - CcCompilationHelper.LEGACY_COMPILE_FLAGS_VARIABLE_NAME, - CcCompilationHelper.getLegacyCompileFlagsSupplier( - cppConfiguration, - ccToolchainProvider, - sourceFile.getExecPathString(), - ImmutableSet.of())); - // Unfilterable flags coming from unfiltered_cxx_flag fields - variables.addLazyStringSequenceVariable( - CcCompilationHelper.UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME, - CcCompilationHelper.getUnfilteredCompileFlagsSupplier( - ccToolchainProvider, ImmutableSet.of())); - // Collect all preprocessor defines, and in each replace ${LABEL} by labelReplacements, and - // ${OUTPUT_PATH} with outputPathReplacement. - variables.addStringSequenceVariable( - CcCompilationHelper.PREPROCESSOR_DEFINES_VARIABLE_NAME, - computeAllLinkstampDefines( - labelReplacement, - outputReplacement, - additionalLinkstampDefines, - cppConfiguration, - fdoBuildStamp, - codeCoverageEnabled)); - // For dynamic libraries, produce position independent code. - if (needsPic) { - variables.addStringVariable(CcCompilationHelper.PIC_VARIABLE_NAME, ""); - } - - return variables.build(); + .collect(ImmutableList.toImmutableList()), + /* userCompileFlags= */ ImmutableList.of(), + /* cppModuleMap= */ null, + needsPic, + outputFile.getExecPath(), + fdoBuildStamp, + /* dotdFileExecPath= */ null, + /* variablesExtensions= */ ImmutableList.of(), + /* additionalBuildVariables= */ ImmutableMap.of(), + // TODO(b/76449614): Remove use of optional_*_flag from CROSSTOOL and get rid of this param + ruleContext.getFeatures()); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java index 890c68a364..443c2d41ad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.rules.cpp.CcBuildVariables.CompileBuildVariables; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.skyframe.FileValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue; @@ -594,9 +595,8 @@ public class FdoSupport { * according to the FDO configuration. This method does nothing If FDO is disabled. */ @ThreadSafe - public void configureCompilation( + public ImmutableMap<String, String> configureCompilation( CppCompileActionBuilder builder, - CcToolchainFeatures.Variables.Builder buildVariables, RuleContext ruleContext, PathFragment sourceName, PathFragment sourceExecPath, @@ -607,11 +607,14 @@ public class FdoSupport { // FDO is disabled -> do nothing. if ((fdoInstrument == null) && (fdoRoot == null)) { - return; + return ImmutableMap.of(); } + ImmutableMap.Builder<String, String> variablesBuilder = ImmutableMap.builder(); + if (featureConfiguration.isEnabled(CppRuleClasses.FDO_INSTRUMENT)) { - buildVariables.addStringVariable("fdo_instrument_path", fdoInstrument); + variablesBuilder.put( + CompileBuildVariables.FDO_INSTRUMENT_PATH.getVariableName(), fdoInstrument); } // Optimization phase @@ -619,7 +622,7 @@ public class FdoSupport { AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); // Declare dependency on contents of zip file. if (env.getSkyframeEnv().valuesMissing()) { - return; + return ImmutableMap.of(); } Iterable<Artifact> auxiliaryInputs = getAuxiliaryInputs( @@ -627,19 +630,24 @@ public class FdoSupport { builder.addMandatoryInputs(auxiliaryInputs); if (!Iterables.isEmpty(auxiliaryInputs)) { if (featureConfiguration.isEnabled(CppRuleClasses.AUTOFDO)) { - buildVariables.addStringVariable( - "fdo_profile_path", getAutoProfilePath(fdoProfile, fdoRootExecPath).getPathString()); + variablesBuilder.put( + CompileBuildVariables.FDO_PROFILE_PATH.getVariableName(), + getAutoProfilePath(fdoProfile, fdoRootExecPath).getPathString()); } if (featureConfiguration.isEnabled(CppRuleClasses.FDO_OPTIMIZE)) { if (fdoMode == FdoMode.LLVM_FDO) { - buildVariables.addStringVariable( - "fdo_profile_path", fdoSupportProvider.getProfileArtifact().getExecPathString()); + variablesBuilder.put( + CompileBuildVariables.FDO_PROFILE_PATH.getVariableName(), + fdoSupportProvider.getProfileArtifact().getExecPathString()); } else { - buildVariables.addStringVariable("fdo_profile_path", fdoRootExecPath.getPathString()); + variablesBuilder.put( + CompileBuildVariables.FDO_PROFILE_PATH.getVariableName(), + fdoRootExecPath.getPathString()); } } } } + return variablesBuilder.build(); } /** Returns the auxiliary files that need to be added to the {@link CppCompileAction}. */ |