Allow for outputs to be both an Artifact and read via an in-memory stream. Use
this from both Java and C++.
RELNOTES: None.
PiperOrigin-RevId: 236785802
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 7f639ad..ee09a1f 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
@@ -1420,7 +1420,7 @@
Artifact sourceFile = builder.getSourceFile();
String dotdFileExecPath = null;
if (builder.getDotdFile() != null) {
- dotdFileExecPath = builder.getDotdFile().getSafeExecPath().getPathString();
+ dotdFileExecPath = builder.getDotdFile().getExecPathString();
}
ImmutableMap.Builder<String, String> allAdditionalBuildVariables = ImmutableMap.builder();
allAdditionalBuildVariables.putAll(additionalBuildVariables);
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 e5f9018..6840cd1 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
@@ -19,7 +19,6 @@
import com.google.devtools.build.lib.actions.Artifact;
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.CppCompileAction.DotdFile;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.util.Pair;
@@ -36,7 +35,7 @@
private final FeatureConfiguration featureConfiguration;
private final CcToolchainVariables variables;
private final String actionName;
- private final DotdFile dotdFile;
+ private final Artifact dotdFile;
@AutoCodec.Instantiator
@VisibleForSerialization
@@ -46,7 +45,7 @@
FeatureConfiguration featureConfiguration,
CcToolchainVariables variables,
String actionName,
- DotdFile dotdFile) {
+ Artifact dotdFile) {
this.sourceFile = Preconditions.checkNotNull(sourceFile);
this.coptsFilter = coptsFilter;
this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
@@ -128,7 +127,7 @@
return sourceFile;
}
- public DotdFile getDotdFile() {
+ public Artifact getDotdFile() {
return dotdFile;
}
@@ -153,10 +152,7 @@
}
public static Builder builder(
- Artifact sourceFile,
- CoptsFilter coptsFilter,
- String actionName,
- DotdFile dotdFile) {
+ Artifact sourceFile, CoptsFilter coptsFilter, String actionName, Artifact dotdFile) {
return new Builder(sourceFile, coptsFilter, actionName, dotdFile);
}
@@ -167,7 +163,7 @@
private FeatureConfiguration featureConfiguration;
private CcToolchainVariables variables = CcToolchainVariables.EMPTY;
private final String actionName;
- @Nullable private final DotdFile dotdFile;
+ @Nullable private final Artifact dotdFile;
public CompileCommandLine build() {
return new CompileCommandLine(
@@ -180,10 +176,7 @@
}
private Builder(
- Artifact sourceFile,
- CoptsFilter coptsFilter,
- String actionName,
- DotdFile dotdFile) {
+ Artifact sourceFile, CoptsFilter coptsFilter, String actionName, Artifact dotdFile) {
this.sourceFile = sourceFile;
this.coptsFilter = coptsFilter;
this.actionName = actionName;
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 e0ba276..39c7266 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
@@ -220,7 +220,7 @@
ImmutableList<Artifact> builtinIncludeFiles,
NestedSet<Artifact> additionalPrunableHeaders,
Artifact outputFile,
- DotdFile dotdFile,
+ Artifact dotdFile,
@Nullable Artifact gcnoFile,
@Nullable Artifact dwoFile,
@Nullable Artifact ltoIndexingFile,
@@ -237,12 +237,7 @@
super(
owner,
createInputsBuilder(mandatoryInputs, inputsForInvalidation).build(),
- CollectionUtils.asSetWithoutNulls(
- outputFile,
- dotdFile == null ? null : dotdFile.artifact(),
- gcnoFile,
- dwoFile,
- ltoIndexingFile),
+ CollectionUtils.asSetWithoutNulls(outputFile, dotdFile, gcnoFile, dwoFile, ltoIndexingFile),
env);
Preconditions.checkArgument(!shouldPruneModules || shouldScanIncludes);
this.outputFile = Preconditions.checkNotNull(outputFile);
@@ -300,6 +295,10 @@
return !useHeaderModules || !shouldPruneModules;
}
+ public boolean useInMemoryDotdFiles() {
+ return cppConfiguration.getInmemoryDotdFiles();
+ }
+
@Override
public List<PathFragment> getBuiltInIncludeDirectories() {
return builtInIncludeDirectories;
@@ -566,11 +565,8 @@
return discoveredModules;
}
- /**
- * Returns the path where gcc should put the discovered dependency
- * information.
- */
- public DotdFile getDotdFile() {
+ /** Returns the path where the compiler should put the discovered dependency information. */
+ public Artifact getDotdFile() {
return compileCommandLine.getDotdFile();
}
@@ -1286,24 +1282,11 @@
ActionExecutionContext actionExecutionContext, Path execRoot, Reply reply)
throws ActionExecutionException {
try {
- DotdFile dotdFile = getDotdFile();
- Preconditions.checkNotNull(dotdFile);
DependencySet depSet = new DependencySet(execRoot);
- // artifact() is null if we are using in-memory .d files. We also want to prepare for the
- // case where we expected an in-memory .d file, but we did not get an appropriate response.
- // Perhaps we produced the file locally.
- if (dotdFile.artifact() != null || reply == null) {
- Path dotdPath;
- if (dotdFile.artifact() != null) {
- dotdPath = dotdFile.getPath(actionExecutionContext);
- } else {
- dotdPath = execRoot.getRelative(dotdFile.getSafeExecPath());
- }
- return depSet.read(dotdPath);
- } else {
- // This is an in-memory .d file.
+ if (reply != null && cppConfiguration.getInmemoryDotdFiles()) {
return depSet.process(reply.getContents());
}
+ return depSet.read(actionExecutionContext.getInputPath(getDotdFile()));
} catch (IOException e) {
// Some kind of IO or parse exception--wrap & rethrow it to stop the build.
throw new ActionExecutionException("error while parsing .d file", e, this, false);
@@ -1499,46 +1482,4 @@
.addTransitive(mandatoryInputs)
.addAll(inputsForInvalidation);
}
-
- /**
- * A reference to a .d file. There are two modes:
- *
- * <ol>
- * <li>an Artifact that represents a real on-disk file
- * <li>just an execPath that refers to a virtual .d file that is not written to disk
- * </ol>
- */
- public static class DotdFile {
- private final Artifact artifact;
- private final PathFragment execPath;
-
- public DotdFile(Artifact artifact) {
- this.artifact = artifact;
- this.execPath = null;
- }
-
- public DotdFile(PathFragment execPath) {
- this.artifact = null;
- this.execPath = execPath;
- }
-
- /**
- * @return the Artifact or null
- */
- public Artifact artifact() {
- return artifact;
- }
-
- /**
- * @return Gets the execPath regardless of whether this is a real Artifact
- */
- public PathFragment getSafeExecPath() {
- return execPath == null ? artifact.getExecPath() : execPath;
- }
-
- /** @return the on-disk location of the .d file or null */
- public Path getPath(ActionExecutionContext actionExecutionContext) {
- return actionExecutionContext.getInputPath(artifact);
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index cde8d01..ecd0b78 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -33,7 +33,6 @@
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
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.CppCompileAction.DotdFile;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.LinkedHashMap;
@@ -60,7 +59,7 @@
private Artifact dwoFile;
private Artifact ltoIndexingFile;
private PathFragment tempOutputFile;
- private DotdFile dotdFile;
+ private Artifact dotdFile;
private Artifact gcnoFile;
private CcCompilationContext ccCompilationContext = CcCompilationContext.EMPTY;
private final List<String> pluginOpts = new ArrayList<>();
@@ -464,7 +463,7 @@
public CppCompileActionBuilder setOutputs(Artifact outputFile, Artifact dotdFile) {
this.outputFile = outputFile;
- this.dotdFile = dotdFile == null ? null : new DotdFile(dotdFile);
+ this.dotdFile = dotdFile;
return this;
}
@@ -486,21 +485,9 @@
if (generateDotd && !useHeaderModules()) {
String dotdFileName =
CppHelper.getDotdFileName(ruleErrorConsumer, ccToolchain, outputCategory, outputName);
- if (cppConfiguration.getInmemoryDotdFiles()) {
- // Just set the path, no artifact is constructed
- dotdFile =
- new DotdFile(
- configuration
- .getBinDirectory(label.getPackageIdentifier().getRepository())
- .getExecPath()
- .getRelative(CppHelper.getObjDirectory(label))
- .getRelative(dotdFileName));
- } else {
- dotdFile =
- new DotdFile(
- CppHelper.getCompileOutputArtifact(
- actionConstructionContext, label, dotdFileName, configuration));
- }
+ dotdFile =
+ CppHelper.getCompileOutputArtifact(
+ actionConstructionContext, label, dotdFileName, configuration);
} else {
dotdFile = null;
}
@@ -539,7 +526,7 @@
return this;
}
- public DotdFile getDotdFile() {
+ public Artifact getDotdFile() {
return this.dotdFile;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index 5b3acb9..502ac6d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -74,7 +74,7 @@
NestedSet<Artifact> prunableHeaders,
Artifact outputFile,
PathFragment tempOutputFile,
- DotdFile dotdFile,
+ Artifact dotdFile,
ActionEnvironment env,
CcCompilationContext ccCompilationContext,
CoptsFilter nocopts,
@@ -216,8 +216,7 @@
// line to write to $TEST_TMPDIR instead.
final String outputPrefix = "$TEST_TMPDIR/";
String argv =
- getArguments()
- .stream()
+ getArguments().stream()
.map(
input -> {
String result = ShellEscaper.escapeString(input);
@@ -228,7 +227,7 @@
}
if (input.equals(outputFile.getExecPathString())
|| (getDotdFile() != null
- && input.equals(getDotdFile().getSafeExecPath().getPathString()))) {
+ && input.equals(getDotdFile().getExecPathString()))) {
result = outputPrefix + ShellEscaper.escapeString(input);
}
return result;
@@ -246,7 +245,7 @@
|| outputFile
.getExecPath()
.getParentDirectory()
- .equals(getDotdFile().getSafeExecPath().getParentDirectory()));
+ .equals(getDotdFile().getExecPath().getParentDirectory()));
FileSystemUtils.writeContent(
actionExecutionContext.getInputPath(outputFile),
ISO_8859_1,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index fa39881..8357aeb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -95,12 +95,11 @@
private final ImmutableList<Artifact> sourceJars;
private final JavaPluginInfo plugins;
- private final ImmutableSet<? extends ActionInput> outputFiles;
private final NestedSet<Artifact> directJars;
private final NestedSet<Artifact> mandatoryInputs;
private final NestedSet<Artifact> transitiveInputs;
private final NestedSet<Artifact> dependencyArtifacts;
- private final ActionInput outputDepsProto;
+ private final Artifact outputDepsProto;
private final JavaClasspathMode classpathMode;
private final JavaCompileExtraActionInfoSupplier extraActionInfoSupplier;
@@ -123,7 +122,7 @@
CommandLine flagLine,
BuildConfiguration configuration,
NestedSet<Artifact> dependencyArtifacts,
- ActionInput outputDepsProto,
+ Artifact outputDepsProto,
JavaClasspathMode classpathMode) {
super(
owner,
@@ -148,15 +147,6 @@
this.dependencyArtifacts = dependencyArtifacts;
this.outputDepsProto = outputDepsProto;
this.classpathMode = classpathMode;
- ImmutableSet.Builder<ActionInput> outputsBuilder = ImmutableSet.builder();
- outputsBuilder.addAll(outputs);
- if (outputDepsProto != null) {
- // If the outputDepsProto is a proper Artifact, it is already in outputs and has thus been
- // declared as an output of the action above. Adding it again won't hurt as this is a set.
- // If we are reading deps protos in memory, add the virtual action output here.
- outputsBuilder.add(outputDepsProto);
- }
- outputFiles = outputsBuilder.build();
}
@Override
@@ -309,10 +299,11 @@
actionExecutionContext);
SpawnResult spawnResult = Iterables.getOnlyElement(results);
+ InputStream inMemoryOutput = spawnResult.getInMemoryOutput(outputDepsProto);
try (InputStream input =
- (outputDepsProto instanceof Artifact)
- ? ((Artifact) outputDepsProto).getPath().getInputStream()
- : spawnResult.getInMemoryOutput(outputDepsProto)) {
+ inMemoryOutput == null
+ ? outputDepsProto.getPath().getInputStream()
+ : inMemoryOutput) {
if (!Deps.Dependencies.parseFrom(input).getRequiresReducedClasspathFallback()) {
return ActionResult.create(results);
}
@@ -425,11 +416,6 @@
public Iterable<? extends ActionInput> getInputFiles() {
return inputs;
}
-
- @Override
- public Collection<? extends ActionInput> getOutputFiles() {
- return outputFiles;
- }
}
@VisibleForTesting
@@ -489,6 +475,6 @@
}
public Artifact getOutputDepsProto() {
- return (outputDepsProto instanceof Artifact) ? (Artifact) outputDepsProto : null;
+ return outputDepsProto;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java
index e1f4562..8be7ef7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java
@@ -24,8 +24,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionEnvironment;
-import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
@@ -287,27 +285,25 @@
classpathMode = JavaClasspathMode.OFF;
}
- ActionInput outputDepsProtoInput = null;
- if (outputDepsProto != null) {
+ Artifact outputDepsProto = null;
+ if (this.outputDepsProto != null) {
+ outputDepsProto =
+ ruleContext.getDerivedArtifact(
+ FileSystemUtils.replaceExtension(
+ this.outputDepsProto.relativeTo(outputJar.getRoot().getExecPath()), ".jdeps"),
+ outputJar.getRoot());
+ outputs.add(outputDepsProto);
if (javaConfiguration.inmemoryJdepsFiles()) {
- outputDepsProtoInput = ActionInputHelper.fromPath(outputDepsProto.getSafePathString());
executionInfo =
ImmutableMap.<String, String>builderWithExpectedSize(this.executionInfo.size() + 1)
.putAll(this.executionInfo)
.put(
ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
- outputDepsProtoInput.getExecPathString())
+ outputDepsProto.getExecPathString())
.build();
- } else {
- Artifact outputDepsProtoArtifact =
- ruleContext.getDerivedArtifact(
- FileSystemUtils.replaceExtension(
- outputDepsProto.relativeTo(outputJar.getRoot().getExecPath()), ".jdeps"),
- outputJar.getRoot());
- outputDepsProtoInput = outputDepsProtoArtifact;
- outputs.add(outputDepsProtoArtifact);
}
}
+
NestedSet<Artifact> tools = toolsBuilder.build();
mandatoryInputs.addTransitive(tools);
JavaCompileAction javaCompileAction =
@@ -329,7 +325,7 @@
/* flagLine= */ buildParamFileContents(ruleContext.getConfiguration(), internedJcopts),
/* configuration= */ ruleContext.getConfiguration(),
/* dependencyArtifacts= */ compileTimeDependencyArtifacts,
- /* outputDepsProto= */ outputDepsProtoInput,
+ /* outputDepsProto= */ outputDepsProto,
/* classpathMode= */ classpathMode);
ruleContext.getAnalysisEnvironment().registerAction(javaCompileAction);
return javaCompileAction;
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java
index 58e700d..b9dbcc3 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
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.CppCompileAction.DotdFile;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;
import java.util.List;
@@ -151,6 +150,6 @@
scratchArtifact("a/FakeInput"),
CoptsFilter.alwaysPasses(),
"c++-compile",
- new DotdFile(scratchArtifact("a/dotD")));
+ scratchArtifact("a/dotD"));
}
}
