Automated rollback of commit c4fc6201fdfa71993e2e9e295a946150e6990c75.
*** Reason for rollback ***
Caused a memory regression
*** Original change description ***
Expose cc_common.create_compile_build_variables
This cl enabled skylark rules to create build variables used for C++ compile
actions (in a limited form, e.g. build variables for modules are not exposed
etc).
Working towards #4571.
RELNOTES: None
PiperOrigin-RevId: 196566686
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 981152c..b64778d 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
@@ -1517,16 +1517,16 @@
ruleContext,
featureConfiguration,
ccToolchain,
- toPathString(sourceFile),
- toPathString(builder.getOutputFile()),
- toPathString(gcnoFile),
- toPathString(dwoFile),
- toPathString(ltoIndexingFile),
+ sourceFile,
+ builder.getOutputFile(),
+ gcnoFile,
+ dwoFile,
+ ltoIndexingFile,
ImmutableList.of(),
userCompileFlags.build(),
cppModuleMap,
usePic,
- builder.getTempOutputFile(),
+ builder.getRealOutputFilePath(),
CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()),
dotdFileExecPath,
ImmutableList.copyOf(variablesExtensions),
@@ -1538,10 +1538,6 @@
ccCompilationContext.getDefines());
}
- private static String toPathString(Artifact a) {
- return a == null ? null : a.getExecPathString();
- }
-
/**
* Returns a {@code CppCompileActionBuilder} with the common fields for a C++ compile action being
* initialized.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
index 3aa90dd..2c246f7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Collection;
/** Enum covering all build variables we create for all various {@link CppCompileAction}. */
public enum CompileBuildVariables {
@@ -107,24 +108,24 @@
RuleContext ruleContext,
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchainProvider,
- String sourceFile,
- String outputFile,
- String gcnoFile,
- String dwoFile,
- String ltoIndexingFile,
+ Artifact sourceFile,
+ Artifact outputFile,
+ Artifact gcnoFile,
+ Artifact dwoFile,
+ Artifact ltoIndexingFile,
ImmutableList<String> includes,
- Iterable<String> userCompileFlags,
+ ImmutableList<String> userCompileFlags,
CppModuleMap cppModuleMap,
boolean usePic,
- PathFragment fakeOutputFile,
+ PathFragment realOutputFilePath,
String fdoStamp,
String dotdFileExecPath,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
- Iterable<PathFragment> includeDirs,
- Iterable<PathFragment> quoteIncludeDirs,
- Iterable<PathFragment> systemIncludeDirs,
+ Collection<PathFragment> includeDirs,
+ Collection<PathFragment> quoteIncludeDirs,
+ Collection<PathFragment> systemIncludeDirs,
Iterable<String> defines) {
try {
return setupVariablesOrThrowEvalException(
@@ -139,20 +140,16 @@
userCompileFlags,
cppModuleMap,
usePic,
- toPathString(fakeOutputFile),
+ realOutputFilePath,
fdoStamp,
dotdFileExecPath,
variablesExtensions,
additionalBuildVariables,
directModuleMaps,
- getSafePathStrings(includeDirs),
- getSafePathStrings(quoteIncludeDirs),
- getSafePathStrings(systemIncludeDirs),
- defines,
- /* addLegacyCxxOptions= */ CppFileTypes.CPP_SOURCE.matches(sourceFile)
- || CppFileTypes.CPP_HEADER.matches(sourceFile)
- || CppFileTypes.CPP_MODULE_MAP.matches(sourceFile)
- || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFile));
+ includeDirs,
+ quoteIncludeDirs,
+ systemIncludeDirs,
+ defines);
} catch (EvalException e) {
ruleContext.ruleError(e.getMessage());
return CcToolchainVariables.EMPTY;
@@ -162,28 +159,25 @@
public static CcToolchainVariables setupVariablesOrThrowEvalException(
FeatureConfiguration featureConfiguration,
CcToolchainProvider ccToolchainProvider,
- String sourceFile,
- // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are
- // updated.
- String outputFile,
- String gcnoFile,
- String dwoFile,
- String ltoIndexingFile,
+ Artifact sourceFile,
+ Artifact outputFile,
+ Artifact gcnoFile,
+ Artifact dwoFile,
+ Artifact ltoIndexingFile,
ImmutableList<String> includes,
- Iterable<String> userCompileFlags,
+ ImmutableList<String> userCompileFlags,
CppModuleMap cppModuleMap,
boolean usePic,
- String fakeOutputFile,
+ PathFragment realOutputFilePath,
String fdoStamp,
String dotdFileExecPath,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
Iterable<Artifact> directModuleMaps,
- Iterable<String> includeDirs,
- Iterable<String> quoteIncludeDirs,
- Iterable<String> systemIncludeDirs,
- Iterable<String> defines,
- boolean addLegacyCxxOptions)
+ Iterable<PathFragment> includeDirs,
+ Iterable<PathFragment> quoteIncludeDirs,
+ Iterable<PathFragment> systemIncludeDirs,
+ Iterable<String> defines)
throws EvalException {
Preconditions.checkNotNull(directModuleMaps);
Preconditions.checkNotNull(includeDirs);
@@ -196,42 +190,34 @@
buildVariables.addStringSequenceVariable(
USER_COMPILE_FLAGS.getVariableName(), userCompileFlags);
+ buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile.getExecPathString());
+
+ String sourceFilename = sourceFile.getExecPathString();
buildVariables.addLazyStringSequenceVariable(
LEGACY_COMPILE_FLAGS.getVariableName(),
- getLegacyCompileFlagsSupplier(ccToolchainProvider, addLegacyCxxOptions));
+ getLegacyCompileFlagsSupplier(ccToolchainProvider, sourceFilename));
- if (sourceFile != null) {
- buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile);
- }
-
- if (sourceFile == null
- || (!CppFileTypes.OBJC_SOURCE.matches(sourceFile)
- && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFile))) {
+ if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename)
+ && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) {
buildVariables.addLazyStringSequenceVariable(
UNFILTERED_COMPILE_FLAGS.getVariableName(),
getUnfilteredCompileFlagsSupplier(ccToolchainProvider));
}
- String fakeOutputFileOrRealOutputFile = fakeOutputFile != null ? fakeOutputFile : outputFile;
-
- if (outputFile != null) {
- // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are
- // updated.
- if (!FileType.contains(
- PathFragment.create(outputFile),
- CppFileTypes.ASSEMBLER,
- CppFileTypes.PIC_ASSEMBLER,
- CppFileTypes.PREPROCESSED_C,
- CppFileTypes.PREPROCESSED_CPP,
- CppFileTypes.PIC_PREPROCESSED_C,
- CppFileTypes.PIC_PREPROCESSED_CPP)) {
- buildVariables.addStringVariable(
- OUTPUT_OBJECT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile);
- }
-
+ // 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(
- OUTPUT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile);
+ OUTPUT_OBJECT_FILE.getVariableName(), realOutputFilePath.getSafePathString());
}
+ buildVariables.addStringVariable(
+ OUTPUT_FILE.getVariableName(), realOutputFilePath.getSafePathString());
// Set dependency_file to enable <object>.d file generation.
if (dotdFileExecPath != null) {
@@ -255,11 +241,12 @@
buildVariables.addStringSequenceVariable(MODULE_FILES.getVariableName(), ImmutableSet.of());
}
if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) {
- buildVariables.addStringSequenceVariable(INCLUDE_PATHS.getVariableName(), includeDirs);
buildVariables.addStringSequenceVariable(
- QUOTE_INCLUDE_PATHS.getVariableName(), quoteIncludeDirs);
+ INCLUDE_PATHS.getVariableName(), getSafePathStrings(includeDirs));
buildVariables.addStringSequenceVariable(
- SYSTEM_INCLUDE_PATHS.getVariableName(), systemIncludeDirs);
+ QUOTE_INCLUDE_PATHS.getVariableName(), getSafePathStrings(quoteIncludeDirs));
+ buildVariables.addStringSequenceVariable(
+ SYSTEM_INCLUDE_PATHS.getVariableName(), getSafePathStrings(systemIncludeDirs));
}
if (!includes.isEmpty()) {
@@ -290,16 +277,18 @@
}
if (gcnoFile != null) {
- buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile);
+ buildVariables.addStringVariable(
+ GCOV_GCNO_FILE.getVariableName(), gcnoFile.getExecPathString());
}
if (dwoFile != null) {
- buildVariables.addStringVariable(PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile);
+ buildVariables.addStringVariable(
+ PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile.getExecPathString());
}
if (ltoIndexingFile != null) {
buildVariables.addStringVariable(
- LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile);
+ LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile.getExecPathString());
}
buildVariables.addAllStringVariables(additionalBuildVariables);
@@ -326,11 +315,14 @@
* to arguments (to prevent accidental capture of enclosing instance which could regress memory).
*/
private static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier(
- CcToolchainProvider toolchain, boolean addLegacyCxxOptions) {
+ CcToolchainProvider toolchain, String sourceFilename) {
return () -> {
ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder();
legacyCompileFlags.addAll(toolchain.getLegacyCompileOptions());
- if (addLegacyCxxOptions) {
+ 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(toolchain.getLegacyCxxOptions());
}
return legacyCompileFlags.build();
@@ -348,14 +340,6 @@
return () -> ccToolchain.getUnfilteredCompilerOptions();
}
- private static String toPathString(PathFragment a) {
- if (a == null) {
- return null;
- }
-
- return a.getSafePathString();
- }
-
public String getVariableName() {
return variableName;
}
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 626f343..0339e13 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
@@ -254,7 +254,6 @@
" action: 'c++-module-codegen'",
" action: 'c++-module-compile'",
" flag_group {",
- " expand_if_all_available: 'output_file'",
" flag: '-frandom-seed=%{output_file}'",
" }",
" }",
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 67d020f..7f92471 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
@@ -146,8 +146,8 @@
ruleContext,
featureConfiguration,
ccToolchainProvider,
- sourceFile.getExecPathString(),
- outputFile.getExecPathString(),
+ sourceFile,
+ outputFile,
/* gcnoFile= */ null,
/* dwoFile= */ null,
/* ltoIndexingFile= */ null,
@@ -158,7 +158,7 @@
CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()),
/* cppModuleMap= */ null,
needsPic,
- /* fakeOutputFile= */ null,
+ outputFile.getExecPath(),
fdoBuildStamp,
/* dotdFileExecPath= */ null,
/* variablesExtensions= */ ImmutableList.of(),
