Automated rollback of commit 4a2e51b3d7b7a27d243e8cc233d60fbbaeeb6190.
*** Reason for rollback ***
Breaks targets on the nightly TGP.
Reproduction:
blaze build //third_party/bazel_rules/rules_kotlin/tests/android/java/com/google/jni:AndroidJniTest --compilation_mode=opt --flaky_test_attempts=2 --fat_apk_cpu=x86 --android_platforms=//buildenv/platforms/android:x86 --incompatible_enable_android_toolchain_resolution=1 --collect_code_coverage=1 --instrumentation_filter=//java/com/google/android/samples/apps/topeka[/:],//third_party/bazel_rules/rules_kotlin[/:],//tools/build_defs/kotlin[/:]
TGP link: []
*** Original change description ***
Make `getPrimaryOutput()` always return the first artifact in `getOutputs()`.
This is already the case everywhere except `CppCompileAction` but was not documented, leading to `SpawnAction` unnecessarily storing a field for the primary output when it's already just the first element in its outputs. This change saves 8 bytes per `SpawnAction` and `GenRuleAction`, and moves other `SpawnAction` subclasses closer to an 8-byte threshold.
`CppCompileAction` had been using the coverage artifact (if pr
***
RELNOTES: None.
PiperOrigin-RevId: 523634597
Change-Id: I0aa70851fe4d403afabc56e808546d6638a9f2b7
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index aa60baf..ab1d50f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -309,8 +309,10 @@
}
@Override
- public final Artifact getPrimaryOutput() {
- return Iterables.get(outputs, 0);
+ public Artifact getPrimaryOutput() {
+ // Default behavior is to return the first output artifact.
+ // Use the method rather than field in case of overriding in subclasses.
+ return Iterables.getFirst(getOutputs(), null);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
index b7df603..8a51350 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java
@@ -186,8 +186,7 @@
Artifact getPrimaryInput();
/**
- * Returns the "primary" output of this action, which is the same as the first artifact in {@link
- * #getOutputs}.
+ * Returns the "primary" output of this action.
*
* <p>For example, the linked library would be the primary output of a LinkAction.
*
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
index df9172d..4d21092 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionMetadata.java
@@ -48,9 +48,9 @@
}
/**
- * Returns a human-readable description of the inputs to {@link #getKey}. Used in the output from
- * '--explain', and in error messages for '--check_up_to_date' and '--check_tests_up_to_date'. May
- * return null, meaning no extra information is available.
+ * Returns a human-readable description of the inputs to {@link #getKey(ActionKeyContext)}. Used
+ * in the output from '--explain', and in error messages for '--check_up_to_date' and
+ * '--check_tests_up_to_date'. May return null, meaning no extra information is available.
*
* <p>If the return value is non-null, for consistency it should be a multiline message of the
* form:
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index b9332e5..666489a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis.actions;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.devtools.build.lib.actions.ActionAnalysisMetadata.mergeMaps;
import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
@@ -104,7 +103,7 @@
private static final String GUID = "ebd6fce3-093e-45ee-adb6-bf513b602f0d";
- private static final Interner<ImmutableSortedMap<String, String>> executionInfoInterner =
+ public static final Interner<ImmutableSortedMap<String, String>> executionInfoInterner =
BlazeInterners.newWeakInterner();
private final CommandLines commandLines;
@@ -119,6 +118,7 @@
private final ImmutableMap<String, String> executionInfo;
private final ExtraActionInfoSupplier extraActionInfoSupplier;
+ private final Artifact primaryOutput;
private final Consumer<Pair<ActionExecutionContext, List<SpawnResult>>> resultConsumer;
private final boolean stripOutputPaths;
@@ -132,6 +132,7 @@
* @param inputs the set of all files potentially read by this action; must not be subsequently
* modified.
* @param outputs the set of all files written by this action; must not be subsequently modified.
+ * @param primaryOutput the primary output of this action
* @param resourceSetOrBuilder the resources consumed by executing this Action.
* @param env the action environment
* @param commandLines the command lines to execute. This includes the main argv vector and any
@@ -147,6 +148,7 @@
NestedSet<Artifact> tools,
NestedSet<Artifact> inputs,
Iterable<Artifact> outputs,
+ Artifact primaryOutput,
ResourceSetOrBuilder resourceSetOrBuilder,
CommandLines commandLines,
CommandLineLimits commandLineLimits,
@@ -159,6 +161,7 @@
tools,
inputs,
outputs,
+ primaryOutput,
resourceSetOrBuilder,
commandLines,
commandLineLimits,
@@ -185,6 +188,7 @@
* @param inputs the set of all files potentially read by this action; must not be subsequently
* modified
* @param outputs the set of all files written by this action; must not be subsequently modified.
+ * @param primaryOutput the primary output of this action
* @param resourceSetOrBuilder the resources consumed by executing this Action.
* @param env the action's environment
* @param executionInfo out-of-band information for scheduling the spawn
@@ -202,6 +206,7 @@
NestedSet<Artifact> tools,
NestedSet<Artifact> inputs,
Iterable<? extends Artifact> outputs,
+ Artifact primaryOutput,
ResourceSetOrBuilder resourceSetOrBuilder,
CommandLines commandLines,
CommandLineLimits commandLineLimits,
@@ -216,6 +221,7 @@
Consumer<Pair<ActionExecutionContext, List<SpawnResult>>> resultConsumer,
boolean stripOutputPaths) {
super(owner, tools, inputs, runfilesSupplier, outputs, env);
+ this.primaryOutput = primaryOutput;
this.resourceSetOrBuilder = resourceSetOrBuilder;
this.executionInfo =
executionInfo.isEmpty()
@@ -232,6 +238,11 @@
this.stripOutputPaths = stripOutputPaths;
}
+ @Override
+ public Artifact getPrimaryOutput() {
+ return primaryOutput;
+ }
+
@VisibleForTesting
public CommandLines getCommandLines() {
return commandLines;
@@ -250,10 +261,12 @@
}
@Override
- public Sequence<CommandLineArgsApi> getStarlarkArgs() {
+ public Sequence<CommandLineArgsApi> getStarlarkArgs() throws EvalException {
ImmutableList.Builder<CommandLineArgsApi> result = ImmutableList.builder();
ImmutableSet<Artifact> directoryInputs =
- getInputs().toList().stream().filter(Artifact::isDirectory).collect(toImmutableSet());
+ getInputs().toList().stream()
+ .filter(artifact -> artifact.isDirectory())
+ .collect(ImmutableSet.toImmutableSet());
for (CommandLineAndParamFileInfo commandLine : commandLines.getCommandLines()) {
result.add(Args.forRegisteredAction(commandLine, directoryInputs));
@@ -523,7 +536,7 @@
* <p>Subclasses of SpawnAction may override this in order to provide action-specific behaviour.
* This can be necessary, for example, when the action discovers inputs.
*/
- private SpawnInfo getExtraActionSpawnInfo()
+ protected SpawnInfo getExtraActionSpawnInfo()
throws CommandLineExpansionException, InterruptedException {
SpawnInfo.Builder info = SpawnInfo.newBuilder();
Spawn spawn = getSpawnForExtraAction();
@@ -588,7 +601,7 @@
throws CommandLineExpansionException {
super(
arguments,
- ImmutableMap.of(),
+ ImmutableMap.<String, String>of(),
parent.getExecutionInfo(),
parent.getRunfilesSupplier(),
parent,
@@ -792,7 +805,8 @@
owner,
tools,
inputsAndTools,
- ImmutableSet.copyOf(outputs),
+ ImmutableList.copyOf(outputs),
+ outputs.get(0),
resourceSetOrBuilder,
commandLines,
commandLineLimits,
@@ -812,7 +826,8 @@
ActionOwner owner,
NestedSet<Artifact> tools,
NestedSet<Artifact> inputsAndTools,
- ImmutableSet<Artifact> outputs,
+ ImmutableList<Artifact> outputs,
+ Artifact primaryOutput,
ResourceSetOrBuilder resourceSetOrBuilder,
CommandLines commandLines,
CommandLineLimits commandLineLimits,
@@ -828,6 +843,7 @@
tools,
inputsAndTools,
outputs,
+ primaryOutput,
resourceSetOrBuilder,
commandLines,
commandLineLimits,
@@ -934,6 +950,13 @@
}
/**
+ * Checks whether the action produces any outputs
+ */
+ public boolean hasOutputs() {
+ return !outputs.isEmpty();
+ }
+
+ /**
* Sets RecourceSet for builder. If ResourceSetBuilder set, then ResourceSetBuilder will
* override setResources.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java
index e44d64b..69eb744 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/StarlarkAction.java
@@ -18,8 +18,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.Action;
@@ -83,6 +81,7 @@
* @param inputs the set of all files potentially read by this action; must not be subsequently
* modified
* @param outputs the set of all files written by this action; must not be subsequently modified.
+ * @param primaryOutput the primary output of this action
* @param resourceSetOrBuilder the resources consumed by executing this Action
* @param commandLines the command lines to execute. This includes the main argv vector and any
* param file-backed command lines.
@@ -104,6 +103,7 @@
NestedSet<Artifact> tools,
NestedSet<Artifact> inputs,
Iterable<Artifact> outputs,
+ Artifact primaryOutput,
ResourceSetOrBuilder resourceSetOrBuilder,
CommandLines commandLines,
CommandLineLimits commandLineLimits,
@@ -123,6 +123,7 @@
? createInputs(shadowedAction.get().getInputs(), inputs)
: inputs,
outputs,
+ primaryOutput,
resourceSetOrBuilder,
commandLines,
commandLineLimits,
@@ -375,7 +376,8 @@
ActionOwner owner,
NestedSet<Artifact> tools,
NestedSet<Artifact> inputsAndTools,
- ImmutableSet<Artifact> outputs,
+ ImmutableList<Artifact> outputs,
+ Artifact primaryOutput,
ResourceSetOrBuilder resourceSetOrBuilder,
CommandLines commandLines,
CommandLineLimits commandLineLimits,
@@ -401,6 +403,7 @@
tools,
inputsAndTools,
outputs,
+ primaryOutput,
resourceSetOrBuilder,
commandLines,
commandLineLimits,
@@ -412,7 +415,7 @@
mnemonic,
unusedInputsList,
shadowedAction,
- stripOutputPaths(mnemonic, inputsAndTools, outputs, configuration));
+ stripOutputPaths(mnemonic, inputsAndTools, primaryOutput, configuration));
}
/**
@@ -432,7 +435,7 @@
private static boolean stripOutputPaths(
String mnemonic,
NestedSet<Artifact> inputs,
- ImmutableSet<Artifact> outputs,
+ Artifact primaryOutput,
BuildConfigurationValue configuration) {
ImmutableList<String> qualifyingMnemonics =
ImmutableList.of(
@@ -453,8 +456,7 @@
CoreOptions coreOptions = configuration.getOptions().get(CoreOptions.class);
return coreOptions.outputPathsMode == OutputPathsMode.STRIP
&& qualifyingMnemonics.contains(mnemonic)
- && PathStripper.isPathStrippable(
- inputs, Iterables.get(outputs, 0).getExecPath().subFragment(0, 1));
+ && PathStripper.isPathStrippable(inputs, primaryOutput.getExecPath().subFragment(0, 1));
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java
index a10bb4e..28ef8c3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraAction.java
@@ -14,8 +14,10 @@
package com.google.devtools.build.lib.analysis.extra;
+import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.Action;
@@ -54,6 +56,9 @@
private final boolean createDummyOutput;
private final NestedSet<Artifact> extraActionInputs;
+ public static final Function<ExtraAction, Action> GET_SHADOWED_ACTION =
+ e -> e != null ? e.getShadowedAction() : null;
+
ExtraAction(
NestedSet<Artifact> extraActionInputs,
RunfilesSupplier runfilesSupplier,
@@ -73,6 +78,7 @@
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
extraActionInputs),
outputs,
+ Iterables.getFirst(outputs, null),
AbstractAction.DEFAULT_RESOURCE_SET,
CommandLines.of(argv),
CommandLineLimits.UNLIMITED,
@@ -122,7 +128,7 @@
updateInputs(
createInputs(shadowedAction.getInputs(), inputFilesForExtraAction, extraActionInputs));
return NestedSetBuilder.wrap(
- Order.STABLE_ORDER, Sets.difference(getInputs().toSet(), oldInputs.toSet()));
+ Order.STABLE_ORDER, Sets.<Artifact>difference(getInputs().toSet(), oldInputs.toSet()));
}
private static NestedSet<Artifact> createInputs(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index 2206435..8e628c3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -676,7 +676,7 @@
env.put("TEST_RANDOM_SEED", Integer.toString(getRunNumber() + 1));
}
// TODO(b/184206260): Actually set TEST_RANDOM_SEED with random seed.
- // The above TEST_RANDOM_SEED has historically been set with the run number, but we should
+ // The above TEST_RANDOM_SEED has histroically been set with the run number, but we should
// explicitly set TEST_RUN_NUMBER to indicate the run number and actually set TEST_RANDOM_SEED
// with a random seed. However, much code has come to depend on it being set to the run number
// and this is an externally documented behavior. Modifying TEST_RANDOM_SEED should be done
@@ -923,6 +923,11 @@
return workspaceName;
}
+ @Override
+ public Artifact getPrimaryOutput() {
+ return testLog;
+ }
+
public PackageSpecificationProvider getNetworkAllowlist() {
return networkAllowlist;
}
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 caa067d..2a182d5 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
@@ -1732,7 +1732,7 @@
configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
- Artifact objectFile = compileAction.getPrimaryOutput();
+ Artifact objectFile = compileAction.getOutputFile();
if (pic) {
result.addPicObjectFile(objectFile);
} else {
@@ -1775,7 +1775,7 @@
configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
- Artifact tokenFile = compileAction.getPrimaryOutput();
+ Artifact tokenFile = compileAction.getOutputFile();
result.addHeaderTokenFile(tokenFile);
}
@@ -1868,13 +1868,13 @@
configuration, featureConfiguration, picBuilder, ruleErrorConsumer);
CppCompileAction picAction = picBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(picAction);
- directOutputs.add(picAction.getPrimaryOutput());
+ directOutputs.add(picAction.getOutputFile());
if (addObject) {
- result.addPicObjectFile(picAction.getPrimaryOutput());
+ result.addPicObjectFile(picAction.getOutputFile());
if (bitcodeOutput) {
result.addLtoBitcodeFile(
- picAction.getPrimaryOutput(), ltoIndexingFile, getCopts(sourceArtifact, sourceLabel));
+ picAction.getOutputFile(), ltoIndexingFile, getCopts(sourceArtifact, sourceLabel));
}
}
if (dwoFile != null) {
@@ -1939,7 +1939,7 @@
configuration, featureConfiguration, builder, ruleErrorConsumer);
CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(compileAction);
- Artifact objectFile = compileAction.getPrimaryOutput();
+ Artifact objectFile = compileAction.getOutputFile();
directOutputs.add(objectFile);
if (addObject) {
result.addObjectFile(objectFile);
@@ -2118,7 +2118,7 @@
CppCompileAction sdAction = sdBuilder.buildOrThrowRuleError(ruleErrorConsumer);
actionRegistry.registerAction(sdAction);
- return ImmutableList.of(dAction.getPrimaryOutput(), sdAction.getPrimaryOutput());
+ return ImmutableList.of(dAction.getOutputFile(), sdAction.getOutputFile());
}
/**
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 82d2010..3d09372 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,10 +123,10 @@
private static final boolean VALIDATION_DEBUG_WARN = false;
- @VisibleForTesting static final String CPP_COMPILE_MNEMONIC = "CppCompile";
- @VisibleForTesting static final String OBJC_COMPILE_MNEMONIC = "ObjcCompile";
+ @VisibleForTesting public static final String CPP_COMPILE_MNEMONIC = "CppCompile";
+ @VisibleForTesting public static final String OBJC_COMPILE_MNEMONIC = "ObjcCompile";
- @Nullable private final Artifact gcnoFile;
+ final Artifact outputFile;
private final Artifact sourceFile;
private final CppConfiguration cppConfiguration;
private final NestedSet<Artifact> mandatoryInputs;
@@ -144,7 +144,8 @@
private final boolean shouldScanIncludes;
private final boolean usePic;
private final boolean useHeaderModules;
- private final boolean needsIncludeValidation;
+ final boolean needsIncludeValidation;
+ private final boolean hasCoverageArtifact;
private final CcCompilationContext ccCompilationContext;
private final ImmutableList<Artifact> builtinIncludeFiles;
@@ -272,7 +273,7 @@
.addTransitive(inputsForInvalidation)
.build(),
collectOutputs(
- Preconditions.checkNotNull(outputFile, "outputFile"),
+ outputFile,
dotdFile,
diagnosticsFile,
gcnoFile,
@@ -280,7 +281,8 @@
ltoIndexingFile,
additionalOutputs),
env);
- this.gcnoFile = gcnoFile;
+ Preconditions.checkNotNull(outputFile);
+ this.outputFile = outputFile;
this.sourceFile = sourceFile;
this.shareable = shareable;
this.cppConfiguration = cppConfiguration;
@@ -302,6 +304,7 @@
coptsFilter,
actionName,
dotdFile,
+ diagnosticsFile,
featureConfiguration,
variables);
this.executionInfo = executionInfo;
@@ -321,10 +324,11 @@
.getParentDirectory()
.getChild(outputFile.getFilename() + ".params");
}
+ this.hasCoverageArtifact = gcnoFile != null;
}
private static ImmutableSet<Artifact> collectOutputs(
- Artifact outputFile,
+ @Nullable Artifact outputFile,
@Nullable Artifact dotdFile,
@Nullable Artifact diagnosticsFile,
@Nullable Artifact gcnoFile,
@@ -332,11 +336,14 @@
@Nullable Artifact ltoIndexingFile,
ImmutableList<Artifact> additionalOutputs) {
ImmutableSet.Builder<Artifact> outputs = ImmutableSet.builder();
- outputs.add(outputFile);
+ // gcnoFile comes first because easy access to it is occasionally useful.
if (gcnoFile != null) {
outputs.add(gcnoFile);
}
outputs.addAll(additionalOutputs);
+ if (outputFile != null) {
+ outputs.add(outputFile);
+ }
if (dotdFile != null) {
outputs.add(dotdFile);
}
@@ -357,6 +364,7 @@
CoptsFilter coptsFilter,
String actionName,
Artifact dotdFile,
+ Artifact diagnosticsFile,
FeatureConfiguration featureConfiguration,
CcToolchainVariables variables) {
return CompileCommandLine.builder(sourceFile, coptsFilter, actionName, dotdFile)
@@ -380,11 +388,11 @@
return shouldScanIncludes;
}
- boolean useInMemoryDotdFiles() {
+ public boolean useInMemoryDotdFiles() {
return cppConfiguration.getInmemoryDotdFiles();
}
- boolean enabledCppCompileResourcesEstimation() {
+ public boolean enabledCppCompileResourcesEstimation() {
return cppConfiguration.getExperimentalCppCompileResourcesEstimation();
}
@@ -412,7 +420,6 @@
// discarded as orphans.
// This is strictly better than marking all transitive modules as inputs, which would also
// effectively disable orphan detection for .pcm files.
- Artifact outputFile = getPrimaryOutput();
if (outputFile.isFileType(CppFileTypes.CPP_MODULE)) {
return ImmutableSet.of(outputFile);
}
@@ -429,7 +436,7 @@
}
/** Clears the discovered {@link #additionalInputs}. */
- private void clearAdditionalInputs() {
+ public void clearAdditionalInputs() {
additionalInputs = null;
}
@@ -507,7 +514,7 @@
throw new ActionExecutionException(message, this, /*catastrophe=*/ false, code);
}
commandLineKey = computeCommandLineKey(options);
- ImmutableList<PathFragment> systemIncludeDirs = getSystemIncludeDirs(options);
+ List<PathFragment> systemIncludeDirs = getSystemIncludeDirs(options);
boolean siblingLayout =
actionExecutionContext
.getOptions()
@@ -552,8 +559,7 @@
}
if (useHeaderModules) {
- boolean separate =
- getPrimaryOutput().equals(ccCompilationContext.getSeparateHeaderModule(usePic));
+ boolean separate = outputFile.equals(ccCompilationContext.getSeparateHeaderModule(usePic));
usedModules =
ccCompilationContext.computeUsedModules(usePic, additionalInputs.toSet(), separate);
}
@@ -593,7 +599,7 @@
additionalInputs =
NestedSetBuilder.fromNestedSet(additionalInputs).addTransitive(discoveredModules).build();
- if (getPrimaryOutput().isFileType(CppFileTypes.CPP_MODULE)) {
+ if (outputFile.isFileType(CppFileTypes.CPP_MODULE)) {
this.discoveredModules = discoveredModules;
}
usedModules = null;
@@ -632,11 +638,21 @@
return getSourceFile();
}
+ @Override
+ public Artifact getPrimaryOutput() {
+ return getOutputFile();
+ }
+
/** Returns the path of the c/cc source for gcc. */
public final Artifact getSourceFile() {
return compileCommandLine.getSourceFile();
}
+ /** Returns the path where gcc should put its result. */
+ public Artifact getOutputFile() {
+ return outputFile;
+ }
+
@Override
@Nullable
public Artifact getGrepIncludes() {
@@ -713,7 +729,7 @@
return getSystemIncludeDirs(getCompilerOptions());
}
- private ImmutableList<PathFragment> getSystemIncludeDirs(List<String> compilerOptions) {
+ private List<PathFragment> getSystemIncludeDirs(List<String> compilerOptions) {
// TODO(bazel-team): parsing the command line flags here couples us to gcc- and clang-cl-style
// compiler command lines; use a different way to specify system includes (for example through a
// system_includes attribute in cc_toolchain); note that that would disallow users from
@@ -803,7 +819,6 @@
@Override
public ImmutableList<Artifact> getIncludeScannerSources() {
if (getSourceFile().isFileType(CppFileTypes.CPP_MODULE_MAP)) {
- Artifact outputFile = getPrimaryOutput();
boolean isSeparate = outputFile.equals(ccCompilationContext.getSeparateHeaderModule(usePic));
// Expected 0 args, but got 1.
Preconditions.checkState(
@@ -868,7 +883,7 @@
}
@Override
- public Sequence<String> getStarlarkArgv() throws EvalException {
+ public Sequence<String> getStarlarkArgv() throws EvalException, InterruptedException {
try {
return StarlarkList.immutableCopyOf(
compileCommandLine.getArguments(
@@ -880,7 +895,7 @@
}
@Override
- public Sequence<CommandLineArgsApi> getStarlarkArgs() {
+ public Sequence<CommandLineArgsApi> getStarlarkArgs() throws EvalException {
ImmutableSet<Artifact> directoryInputs =
getInputs().toList().stream().filter(Artifact::isDirectory).collect(toImmutableSet());
@@ -901,6 +916,10 @@
return StarlarkList.immutableCopyOf(ImmutableList.of(args));
}
+ public ParamFileActionInput getParamFileActionInput() {
+ return paramFileActionInput;
+ }
+
@Override
public ExtraActionInfo.Builder getExtraActionInfo(ActionKeyContext actionKeyContext)
throws CommandLineExpansionException, InterruptedException {
@@ -912,7 +931,7 @@
for (String option : options) {
info.addCompilerOption(option);
}
- info.setOutputFile(getPrimaryOutput().getExecPathString());
+ info.setOutputFile(outputFile.getExecPathString());
info.setSourceFile(getSourceFile().getExecPathString());
if (inputsDiscovered()) {
info.addAllSourcesAndHeaders(Artifact.toExecPaths(getInputs().toList()));
@@ -935,7 +954,7 @@
return super.getExtraActionInfo(actionKeyContext)
.setExtension(CppCompileInfo.cppCompileInfo, info.build());
} catch (CommandLineExpansionException e) {
- throw new AssertionError("CppCompileAction command line expansion cannot fail", e);
+ throw new AssertionError("CppCompileAction command line expansion cannot fail.");
}
}
@@ -1016,7 +1035,7 @@
// Copy the nested sets to hash sets for fast contains checking, but do so lazily.
// Avoid immutable sets here to limit memory churn.
- ImmutableSet<PathFragment> looseHdrsDirs = ccCompilationContext.getLooseHdrsDirs().toSet();
+ Set<PathFragment> looseHdrsDirs = ccCompilationContext.getLooseHdrsDirs().toSet();
for (Artifact input : inputsForValidation.toList()) {
if (!validateInclude(
actionExecutionContext, allowedIncludes, looseHdrsDirs, ignoreDirs, input)) {
@@ -1217,7 +1236,7 @@
return variableBuilder.build();
}
- CcToolchainVariables getOverwrittenVariables() {
+ public CcToolchainVariables getOverwrittenVariables() {
if (useHeaderModules) {
// TODO(cmita): Avoid keeping state in CppCompileAction.
// There are two cases for when this method might be called:
@@ -1249,7 +1268,7 @@
// The separate module is an allowed input to all compiles of this context except for its own
// compile.
Artifact separateModule = ccCompilationContext.getSeparateHeaderModule(usePic);
- if (separateModule != null && !separateModule.equals(getPrimaryOutput())) {
+ if (separateModule != null && !separateModule.equals(outputFile)) {
builder.add(separateModule);
}
return builder.build();
@@ -1265,7 +1284,7 @@
@Override
public synchronized void updateInputs(NestedSet<Artifact> inputs) {
super.updateInputs(inputs);
- if (getPrimaryOutput().isFileType(CppFileTypes.CPP_MODULE)) {
+ if (outputFile.isFileType(CppFileTypes.CPP_MODULE)) {
discoveredModules =
NestedSetBuilder.wrap(
Order.STABLE_ORDER,
@@ -1302,7 +1321,7 @@
* estimation we are using form C + K * inputs, where C and K selected in such way, that more than
* 95% of actions used less than C + K * inputs MB of memory during execution.
*/
- static ResourceSet estimateResourceConsumptionLocal(
+ public static ResourceSet estimateResourceConsumptionLocal(
boolean enabled, String mnemonic, OS os, int inputs) {
if (!enabled) {
return AbstractAction.DEFAULT_RESOURCE_SET;
@@ -1356,7 +1375,7 @@
executionInfo,
getCommandLineKey(),
ccCompilationContext.getDeclaredIncludeSrcs(),
- mandatoryInputs,
+ getMandatoryInputs(),
additionalPrunableHeaders,
ccCompilationContext.getLooseHdrsDirs(),
builtInIncludeDirectories,
@@ -1586,7 +1605,7 @@
}
@Nullable
- private byte[] getDotDContents(SpawnResult spawnResult) throws EnvironmentalExecException {
+ protected byte[] getDotDContents(SpawnResult spawnResult) throws EnvironmentalExecException {
if (getDotdFile() != null) {
InputStream in = spawnResult.getInMemoryOutput(getDotdFile());
if (in != null) {
@@ -1601,11 +1620,12 @@
return null;
}
- boolean shouldParseShowIncludes() {
+ protected boolean shouldParseShowIncludes() {
return featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
}
- Spawn createSpawn(Path execRoot, Map<String, String> clientEnv) throws ActionExecutionException {
+ protected Spawn createSpawn(Path execRoot, Map<String, String> clientEnv)
+ throws ActionExecutionException {
// Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed
// for execution.
NestedSetBuilder<ActionInput> inputsBuilder =
@@ -1613,8 +1633,8 @@
if (discoversInputs()) {
inputsBuilder.addTransitive(getAdditionalInputs());
}
- if (paramFileActionInput != null) {
- inputsBuilder.add(paramFileActionInput);
+ if (getParamFileActionInput() != null) {
+ inputsBuilder.add(getParamFileActionInput());
}
NestedSet<ActionInput> inputs = inputsBuilder.build();
@@ -1649,13 +1669,15 @@
ImmutableList.copyOf(getArguments()),
getEffectiveEnvironment(clientEnv),
executionInfo.buildOrThrow(),
- /* runfilesSupplier= */ null,
- /* filesetMappings= */ ImmutableMap.of(),
+ /*runfilesSupplier=*/ null,
+ /*filesetMappings=*/ ImmutableMap.of(),
inputs,
- /* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+ /*tools=*/ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
getOutputs(),
// In coverage mode, .gcno file not produced for an empty translation unit.
- /* mandatoryOutputs= */ gcnoFile != null ? ImmutableSet.of(gcnoFile) : null,
+ /*mandatoryOutputs=*/ hasCoverageArtifact
+ ? ImmutableSet.copyOf(getOutputs().asList().subList(1, getOutputs().size()))
+ : null,
() ->
estimateResourceConsumptionLocal(
enabledCppCompileResourcesEstimation(),
@@ -1717,7 +1739,7 @@
siblingRepositoryLayout);
}
- private DependencySet processDepset(
+ public DependencySet processDepset(
ActionExecutionContext actionExecutionContext, Path execRoot, byte[] dotDContents)
throws ActionExecutionException {
try {
@@ -1734,7 +1756,7 @@
}
}
- private List<Path> getPermittedSystemIncludePrefixes(Path execRoot) {
+ public List<Path> getPermittedSystemIncludePrefixes(Path execRoot) {
List<Path> systemIncludePrefixes = new ArrayList<>();
for (PathFragment includePath : getBuiltInIncludeDirectories()) {
if (includePath.isAbsolute()) {
@@ -1751,16 +1773,20 @@
*/
private void ensureCoverageNotesFileExists(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException {
- if (gcnoFile == null) {
+ if (!hasCoverageArtifact) {
return;
}
- if (!gcnoFile.isFileType(CppFileTypes.COVERAGE_NOTES)) {
+ Artifact gcnoArtifact = getOutputs().iterator().next();
+ if (!gcnoArtifact.isFileType(CppFileTypes.COVERAGE_NOTES)) {
BugReport.sendBugReport(
new IllegalStateException(
- "In coverage mode but gcno artifact is not correct type: " + gcnoFile + ", " + this));
+ "In coverage mode but gcno artifact is not first output: "
+ + gcnoArtifact
+ + ", "
+ + this));
return;
}
- Path outputPath = actionExecutionContext.getInputPath(gcnoFile);
+ Path outputPath = actionExecutionContext.getInputPath(gcnoArtifact);
if (outputPath.exists()) {
return;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
index ff84c65..28acc9a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -55,7 +55,7 @@
private final NestedSet<Artifact> allInputs;
/**
- * Creates a CppCompileActionTemplate.
+ * Creates an CppCompileActionTemplate.
*
* @param sourceTreeArtifact the TreeArtifact that contains source files to compile.
* @param outputTreeArtifact the TreeArtifact that contains compilation outputs.
@@ -170,6 +170,7 @@
cppCompileActionBuilder.getCoptsFilter(),
CppActionNames.CPP_COMPILE,
dotdTreeArtifact,
+ diagnosticsTreeArtifact,
cppCompileActionBuilder.getFeatureConfiguration(),
cppCompileActionBuilder.getVariables());
CppCompileAction.computeKey(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
index 93577a7..f30a84c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java
@@ -17,7 +17,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionEnvironment;
@@ -45,6 +44,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.IOException;
+import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -77,7 +77,8 @@
NestedSet<Artifact> inputs,
@Nullable BitcodeFiles allBitcodeFiles,
@Nullable Artifact importsFile,
- ImmutableSet<Artifact> outputs,
+ Collection<Artifact> outputs,
+ Artifact primaryOutput,
ActionOwner owner,
CommandLines argv,
CommandLineLimits commandLineLimits,
@@ -92,6 +93,7 @@
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
inputs,
outputs,
+ primaryOutput,
AbstractAction.DEFAULT_RESOURCE_SET,
argv,
commandLineLimits,
@@ -259,7 +261,8 @@
ActionOwner owner,
NestedSet<Artifact> tools,
NestedSet<Artifact> inputsAndTools,
- ImmutableSet<Artifact> outputs,
+ ImmutableList<Artifact> outputs,
+ Artifact primaryOutput,
ResourceSetOrBuilder resourceSetOrBuilder,
CommandLines commandLines,
CommandLineLimits commandLineLimits,
@@ -275,6 +278,7 @@
bitcodeFiles,
imports,
outputs,
+ primaryOutput,
owner,
commandLines,
commandLineLimits,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
index 42d7f85..c6998db 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleAction.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.AbstractAction;
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
@@ -54,6 +55,7 @@
tools,
inputs,
outputs,
+ Iterables.getFirst(outputs, null),
AbstractAction.DEFAULT_RESOURCE_SET,
commandLines,
CommandLineLimits.UNLIMITED,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java
index f37c6a6..492db60 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java
@@ -433,6 +433,7 @@
/* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
/* inputs= */ allInputs,
/* outputs= */ outputs.build(),
+ /* primaryOutput= */ outputJar,
/* resourceSetOrBuilder= */ AbstractAction.DEFAULT_RESOURCE_SET,
/* commandLines= */ CommandLines.builder()
.addCommandLine(executableLine)
