diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules')
8 files changed, 172 insertions, 97 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 750d666cbb..371690aba5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -119,6 +119,7 @@ public final class CcLibraryHelper { CppCompileAction.ASSEMBLE, CppCompileAction.PREPROCESS_ASSEMBLE, CppCompileAction.CLIF_MATCH, + CppCompileAction.LINKSTAMP_COMPILE, Link.LinkTargetType.STATIC_LIBRARY.getActionName(), // We need to create pic-specific actions for link actions, as they will produce // differently named outputs. @@ -147,6 +148,7 @@ public final class CcLibraryHelper { CppCompileAction.CPP_HEADER_PREPROCESSING, CppCompileAction.ASSEMBLE, CppCompileAction.PREPROCESS_ASSEMBLE, + CppCompileAction.LINKSTAMP_COMPILE, Link.LinkTargetType.STATIC_LIBRARY.getActionName(), // We need to create pic-specific actions for link actions, as they will produce // differently named outputs. 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 472701ceec..561260581f 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 @@ -88,7 +88,7 @@ public final class CompileCommandLine { // second: The compiler options. commandLine.addAll(getCompilerOptions(overwrittenVariables)); - if (!featureConfiguration.isEnabled("compile_action_flags_in_flag_set")) { + if (!featureConfiguration.isEnabled(CppRuleClasses.COMPILE_ACTION_FLAGS_IN_FLAG_SET)) { // third: The file to compile! commandLine.add("-c"); commandLine.add(sourceFile.getExecPathString()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java index e965d4a82f..caf3e36efe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java @@ -83,6 +83,17 @@ public class CppActionConfigs { " implies: 'unfiltered_compile_flags'", "}", "action_config {", + " config_name: 'linkstamp-compile'", + " action_name: 'linkstamp-compile'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + " implies: 'legacy_compile_flags'", + " implies: 'user_compile_flags'", + " implies: 'sysroot'", + " implies: 'unfiltered_compile_flags'", + "}", + "action_config {", " config_name: 'c-compile'", " action_name: 'c-compile'", " tool {", @@ -157,6 +168,7 @@ public class CppActionConfigs { " expand_if_all_available: 'legacy_compile_flags'", " action: 'assemble'", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-header-parsing'", @@ -229,6 +241,7 @@ public class CppActionConfigs { " flag_set {", " action: 'assemble'", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-module-codegen'", @@ -261,6 +274,7 @@ public class CppActionConfigs { " name: 'preprocessor_defines'", " flag_set {", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-header-parsing'", @@ -274,11 +288,36 @@ public class CppActionConfigs { " }", "}"), ifTrue( + !existingFeatureNames.contains("includes"), + "feature {", + " name: 'includes'", + " enabled: true", + " flag_set {", + " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'c++-header-parsing'", + " action: 'c++-header-preprocessing'", + " action: 'c++-module-compile'", + " action: 'clif-match'", + " action: 'objc-compile'", + " action: 'objc++-compile'", + " flag_group {", + " expand_if_all_available: 'includes'", + " iterate_over: 'includes'", + " flag: '-include'", + " flag: '%{includes}'", + " }", + " }", + "}"), + ifTrue( !existingFeatureNames.contains(CppRuleClasses.INCLUDE_PATHS), "feature {", " name: 'include_paths'", " flag_set {", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-header-parsing'", @@ -942,6 +981,7 @@ public class CppActionConfigs { " expand_if_all_available: 'user_compile_flags'", " action: 'assemble'", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-header-parsing'", @@ -963,6 +1003,7 @@ public class CppActionConfigs { " enabled: true", " flag_set {", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-header-parsing'", @@ -991,6 +1032,7 @@ public class CppActionConfigs { " expand_if_all_available: 'unfiltered_compile_flags'", " action: 'assemble'", " action: 'preprocess-assemble'", + " action: 'linkstamp-compile'", " action: 'c-compile'", " action: 'c++-compile'", " action: 'c++-header-parsing'", 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 0de697be74..8c09e6c8cb 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 @@ -123,6 +123,9 @@ public class CppCompileAction extends AbstractAction /** A string constant for the strip action name. */ public static final String STRIP_ACTION_NAME = "strip"; + /** A string constant for the linkstamp-compile action. */ + public static final String LINKSTAMP_COMPILE = "linkstamp-compile"; + /** * A string constant for the c compilation action. */ 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 2df7719e49..633adc98a0 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 @@ -186,7 +186,7 @@ public class CppLinkActionBuilder { private NestedSet<Artifact> runtimeInputs = NestedSetBuilder.emptySet(Order.STABLE_ORDER); private final NestedSetBuilder<Artifact> compilationInputs = NestedSetBuilder.stableOrder(); private final Set<Artifact> linkstamps = new LinkedHashSet<>(); - private List<String> linkstampOptions = new ArrayList<>(); + private ImmutableList<String> additionalLinkstampDefines = ImmutableList.of(); private final List<String> linkopts = new ArrayList<>(); private LinkTargetType linkType = LinkTargetType.STATIC_LIBRARY; private LinkStaticness linkStaticness = LinkStaticness.FULLY_STATIC; @@ -398,12 +398,6 @@ public class CppLinkActionBuilder { public final Set<Artifact> getLinkstamps() { return this.linkstamps; } - /** - * Returns linkstamp options for this link action. - */ - public List<String> getLinkstampOptions() { - return this.linkstampOptions; - } /** * Returns command line options for this link action. @@ -799,7 +793,7 @@ public class CppLinkActionBuilder { .setBuildInfoHeaderArtifacts(buildInfoHeaderArtifacts) .setLinkstamps(linkstampMap) .setLinkopts(ImmutableList.copyOf(linkopts)) - .addLinkstampCompileOptions(linkstampOptions); + .setAdditionalLinkstampDefines(additionalLinkstampDefines); } else { List<String> opts = new ArrayList<>(linkopts); opts.addAll(featureConfiguration.getCommandLine("lto-indexing", buildVariables)); @@ -1236,8 +1230,9 @@ public class CppLinkActionBuilder { return this; } - public CppLinkActionBuilder addLinkstampCompilerOptions(ImmutableList<String> linkstampOptions) { - this.linkstampOptions = linkstampOptions; + public CppLinkActionBuilder setAdditionalLinkstampDefines( + ImmutableList<String> additionalLinkstampDefines) { + this.additionalLinkstampDefines = Preconditions.checkNotNull(additionalLinkstampDefines); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index b7a8e6062f..2d59c5a754 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -110,6 +110,12 @@ public final class CppModel { public static final String MODULE_FILES_VARIABLE_NAME = "module_files"; /** + * 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 the collection of include paths. * * @see CppCompilationContext#getIncludeDirs(). 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 97ad0f391e..190e935252 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 @@ -273,6 +273,9 @@ public class CppRuleClasses { */ public static final String PREPROCESSOR_DEFINES = "preprocessor_defines"; + /** A string constant for the includes feature. */ + public static final String INCLUDES = "includes"; + /** * A string constant for the include_paths feature. */ @@ -349,6 +352,6 @@ public class CppRuleClasses { /** Produce artifacts for coverage in gcc coverage mapping format. */ public static final String GCC_COVERAGE_MAP_FORMAT = "gcc_coverage_map_format"; - /** A string constant for the match-clif feature. */ + /** A string constant for the match-clif action. */ public static final String MATCH_CLIF = "match_clif"; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index c171c9fe01..186c4f742a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java @@ -69,7 +69,7 @@ public final class LinkCommandLine extends CommandLine { private final ImmutableList<String> linkopts; private final ImmutableSet<String> features; private final ImmutableMap<Artifact, Artifact> linkstamps; - private final ImmutableList<String> linkstampCompileOptions; + private final ImmutableList<String> additionalLinkstampDefines; @Nullable private final String fdoBuildStamp; @Nullable private final PathFragment runtimeSolibDir; private final boolean nativeDeps; @@ -92,7 +92,7 @@ public final class LinkCommandLine extends CommandLine { ImmutableList<String> linkopts, ImmutableSet<String> features, ImmutableMap<Artifact, Artifact> linkstamps, - ImmutableList<String> linkstampCompileOptions, + ImmutableList<String> additionalLinkstampDefines, @Nullable String fdoBuildStamp, @Nullable PathFragment runtimeSolibDir, boolean nativeDeps, @@ -116,12 +116,13 @@ public final class LinkCommandLine extends CommandLine { this.linkTargetType = Preconditions.checkNotNull(linkTargetType); this.linkStaticness = Preconditions.checkNotNull(linkStaticness); // For now, silently ignore linkopts if this is a static library link. - this.linkopts = linkTargetType.staticness() == Staticness.STATIC - ? ImmutableList.<String>of() - : Preconditions.checkNotNull(linkopts); + this.linkopts = + linkTargetType.staticness() == Staticness.STATIC + ? ImmutableList.of() + : Preconditions.checkNotNull(linkopts); this.features = Preconditions.checkNotNull(features); this.linkstamps = Preconditions.checkNotNull(linkstamps); - this.linkstampCompileOptions = linkstampCompileOptions; + this.additionalLinkstampDefines = additionalLinkstampDefines; this.fdoBuildStamp = fdoBuildStamp; this.runtimeSolibDir = runtimeSolibDir; this.nativeDeps = nativeDeps; @@ -518,74 +519,90 @@ public final class LinkCommandLine extends CommandLine { return ImmutableList.of(); } - String compilerCommand = cppConfiguration.getCppExecutable().getPathString(); List<String> commands = Lists.newArrayListWithCapacity(linkstamps.size()); for (Map.Entry<Artifact, Artifact> linkstamp : linkstamps.entrySet()) { - List<String> optionList = new ArrayList<>(); + Artifact sourceFile = linkstamp.getKey(); + Artifact outputFile = linkstamp.getValue(); + Variables linkstampVariables = collectLinkstampVariables(sourceFile, outputFile); - // Defines related to the build info are read from generated headers. - for (Artifact header : buildInfoHeaderArtifacts) { - optionList.add("-include"); - optionList.add(header.getExecPathString()); - } - - String labelReplacement = Matcher.quoteReplacement( - isSharedNativeLibrary() ? output.getExecPathString() : Label.print(owner.getLabel())); - String outputPathReplacement = Matcher.quoteReplacement( - output.getExecPathString()); - for (String option : linkstampCompileOptions) { - optionList.add(option - .replaceAll(Pattern.quote("${LABEL}"), labelReplacement) - .replaceAll(Pattern.quote("${OUTPUT_PATH}"), outputPathReplacement)); - } - - optionList.add("-DGPLATFORM=\"" + cppConfiguration + "\""); - optionList.add("-DBUILD_COVERAGE_ENABLED=" + (codeCoverageEnabled ? "1" : "0")); - - // Needed to find headers included from linkstamps. - optionList.add("-I."); - - // Add toolchain compiler options. - optionList.addAll(cppConfiguration.getCompilerOptions(features)); - optionList.addAll(cppConfiguration.getCOptions()); - optionList.addAll(ccProvider.getUnfilteredCompilerOptionsWithSysroot(features)); - if (CppFileTypes.CPP_SOURCE.matches(linkstamp.getKey().getExecPath())) { - optionList.addAll(cppConfiguration.getCxxOptions(features)); - } - - // For dynamic libraries, produce position independent code. - if (linkTargetType == LinkTargetType.DYNAMIC_LIBRARY - && cppConfiguration.toolchainNeedsPic()) { - optionList.add("-fPIC"); - } - - // Stamp FDO builds with FDO subtype string - if (fdoBuildStamp != null) { - optionList.add("-D" + CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\""); - } - - // Add the compilation target. - optionList.add("-c"); - optionList.add(linkstamp.getKey().getExecPathString()); - - // Assemble the final command, exempting outputPrefix from shell escaping. - commands.add(compilerCommand + " " - + ShellEscaper.escapeJoinAll(optionList) - + " -o " - + outputPrefix - + ShellEscaper.escapeString(linkstamp.getValue().getExecPathString())); + ImmutableList.Builder<String> linkstampCompileCommandLine = ImmutableList.builder(); + linkstampCompileCommandLine.add( + featureConfiguration + .getToolForAction(CppCompileAction.LINKSTAMP_COMPILE) + .getToolPath(cppConfiguration.getCrosstoolTopPathFragment()) + .getPathString()); + linkstampCompileCommandLine.addAll( + featureConfiguration.getCommandLine( + CppCompileAction.LINKSTAMP_COMPILE, linkstampVariables)); + // TODO(b/28946988): Remove -c/-o hardcoded flags from bazel + linkstampCompileCommandLine.add("-c"); + linkstampCompileCommandLine.add(sourceFile.getExecPathString()); + linkstampCompileCommandLine.add("-o"); + linkstampCompileCommandLine.add(outputFile.getExecPathString()); + commands.add(ShellEscaper.escapeJoinAll(linkstampCompileCommandLine.build())); } return commands; } - /** - * A builder for a {@link LinkCommandLine}. - */ - public static final class Builder { - // TODO(bazel-team): Pass this in instead of having it here. Maybe move to cc_toolchain. - private static final ImmutableList<String> DEFAULT_LINKSTAMP_OPTIONS = ImmutableList.of( + private Variables collectLinkstampVariables(Artifact sourceFile, Artifact outputFile) { + // TODO(b/34761650): Remove all this hardcoding by separating a full blown compile action. + Preconditions.checkArgument( + featureConfiguration.actionIsConfigured(CppCompileAction.LINKSTAMP_COMPILE)); + + Variables.Builder linkstampVariables = new Variables.Builder(ccProvider.getBuildVariables()); + // We need to force inclusion of build_info headers + linkstampVariables.addStringSequenceVariable( + CppModel.INCLUDES_VARIABLE_NAME, + buildInfoHeaderArtifacts + .stream() + .map(Artifact::getExecPathString) + .collect(ImmutableList.toImmutableList())); + // Input/Output files. + linkstampVariables.addStringVariable( + CppModel.SOURCE_FILE_VARIABLE_NAME, sourceFile.getExecPathString()); + linkstampVariables.addStringVariable( + CppModel.OUTPUT_OBJECT_FILE_VARIABLE_NAME, outputFile.getExecPathString()); + // Include directories for (normal includes with ".", empty quote- and system- includes). + linkstampVariables.addStringSequenceVariable( + CppModel.INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of(".")); + linkstampVariables.addStringSequenceVariable( + CppModel.QUOTE_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); + linkstampVariables.addStringSequenceVariable( + CppModel.SYSTEM_INCLUDE_PATHS_VARIABLE_NAME, ImmutableList.of()); + // Legacy flags coming from fields such as compiler_flag + linkstampVariables.addLazyStringSequenceVariable( + CppModel.LEGACY_COMPILE_FLAGS_VARIABLE_NAME, + CppModel.getLegacyCompileFlagsSupplier( + cppConfiguration, sourceFile.getExecPathString(), features)); + // Unfilterable flags coming from unfiltered_cxx_flag fields + linkstampVariables.addLazyStringSequenceVariable( + CppModel.UNFILTERED_COMPILE_FLAGS_VARIABLE_NAME, + CppModel.getUnfilteredCompileFlagsSupplier(ccProvider, features)); + // Collect all preprocessor defines, and in each replace ${LABEL} by labelReplacements, and + // ${OUTPUT_PATH} with outputPathReplacement. + linkstampVariables.addStringSequenceVariable( + CppModel.PREPROCESSOR_DEFINES_VARIABLE_NAME, + computeAllLinkstampDefines()); + // For dynamic libraries, produce position independent code. + if (linkTargetType == LinkTargetType.DYNAMIC_LIBRARY && cppConfiguration.toolchainNeedsPic()) { + linkstampVariables.addStringVariable(CppModel.PIC_VARIABLE_NAME, ""); + } + return linkstampVariables.build(); + } + + private ImmutableList<String> computeAllLinkstampDefines() { + String labelReplacement = + Matcher.quoteReplacement( + isSharedNativeLibrary() ? output.getExecPathString() : Label.print(owner.getLabel())); + String outputPathReplacement = Matcher.quoteReplacement(output.getExecPathString()); + String labelPattern = Pattern.quote("${LABEL}"); + String outputPathPattern = Pattern.quote("${OUTPUT_PATH}"); + ImmutableList.Builder<String> defines = ImmutableList.builder(); + defines + .add("GPLATFORM=\"" + cppConfiguration + "\"") + .add("BUILD_COVERAGE_ENABLED=" + (codeCoverageEnabled ? "1" : "0")) // G3_VERSION_INFO and G3_TARGET_NAME are C string literals that normally // contain the label of the target being linked. However, they are set // differently when using shared native deps. In that case, a single .so file @@ -593,13 +610,31 @@ public final class LinkCommandLine extends CommandLine { // target(s) were specified on the command line. So in that case we have // to use the (obscure) name of the .so file instead, or more precisely // the path of the .so file relative to the workspace root. - "-DG3_VERSION_INFO=\"${LABEL}\"", - "-DG3_TARGET_NAME=\"${LABEL}\"", - + .add("G3_VERSION_INFO=\"${LABEL}\"") + .add("G3_TARGET_NAME=\"${LABEL}\"") // G3_BUILD_TARGET is a C string literal containing the output of this // link. (An undocumented and untested invariant is that G3_BUILD_TARGET is the location of // the executable, either absolutely, or relative to the directory part of BUILD_INFO.) - "-DG3_BUILD_TARGET=\"${OUTPUT_PATH}\""); + .add("G3_BUILD_TARGET=\"${OUTPUT_PATH}\"") + .addAll(additionalLinkstampDefines); + + if (fdoBuildStamp != null) { + defines.add(CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\""); + } + + return defines + .build() + .stream() + .map( + define -> + define + .replaceAll(labelPattern, labelReplacement) + .replaceAll(outputPathPattern, outputPathReplacement)) + .collect(ImmutableList.toImmutableList()); + } + + /** A builder for a {@link LinkCommandLine}. */ + public static final class Builder { private final BuildConfiguration configuration; private final ActionOwner owner; @@ -615,7 +650,7 @@ public final class LinkCommandLine extends CommandLine { private ImmutableList<String> linkopts = ImmutableList.of(); private ImmutableSet<String> features = ImmutableSet.of(); private ImmutableMap<Artifact, Artifact> linkstamps = ImmutableMap.of(); - private List<String> linkstampCompileOptions = new ArrayList<>(); + private ImmutableList<String> additionalLinkstampDefines = ImmutableList.of(); @Nullable private PathFragment runtimeSolibDir; private boolean nativeDeps; private boolean useTestOnlyFlags; @@ -649,14 +684,6 @@ public final class LinkCommandLine extends CommandLine { "build info headers may only be present on dynamic library or executable links"); } - ImmutableList<String> actualLinkstampCompileOptions; - if (linkstampCompileOptions.isEmpty()) { - actualLinkstampCompileOptions = DEFAULT_LINKSTAMP_OPTIONS; - } else { - actualLinkstampCompileOptions = ImmutableList.copyOf( - Iterables.concat(DEFAULT_LINKSTAMP_OPTIONS, linkstampCompileOptions)); - } - if (toolchain == null) { toolchain = Preconditions.checkNotNull( @@ -693,7 +720,7 @@ public final class LinkCommandLine extends CommandLine { linkopts, features, linkstamps, - actualLinkstampCompileOptions, + additionalLinkstampDefines, CppHelper.getFdoBuildStamp(ruleContext, fdoSupport), runtimeSolibDir, nativeDeps, @@ -797,12 +824,9 @@ public final class LinkCommandLine extends CommandLine { return this; } - /** - * Adds the given C++ compiler options to the list of options passed to the linkstamp - * compilation. - */ - public Builder addLinkstampCompileOptions(List<String> linkstampCompileOptions) { - this.linkstampCompileOptions.addAll(linkstampCompileOptions); + /** Adds the given list of preprocessor defines to the linkstamp compilation. */ + public Builder setAdditionalLinkstampDefines(ImmutableList<String> additionalLinkstampDefines) { + this.additionalLinkstampDefines = Preconditions.checkNotNull(additionalLinkstampDefines); return this; } |