Clean up *GccStrategy
- use SimpleSpawn in SpawnGccStrategy
- set PWD in CppCompileAction for consistency
--
PiperOrigin-RevId: 149745059
MOS_MIGRATED_REVID=149745059
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 e835609..2afd77d 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
@@ -36,6 +36,7 @@
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.extra.CppCompileInfo;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
+import com.google.devtools.build.lib.analysis.actions.ExecutionRequirements;
import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.CollectionUtils;
@@ -167,7 +168,6 @@
public static final String CLIF_MATCH = "clif-match";
private final ImmutableMap<String, String> localShellEnvironment;
- private final boolean isCodeCoverageEnabled;
protected final Artifact outputFile;
private final Artifact sourceFile;
private final Label sourceLabel;
@@ -294,7 +294,6 @@
@Nullable Artifact dwoFile,
Artifact optionalSourceFile,
ImmutableMap<String, String> localShellEnvironment,
- boolean isCodeCoverageEnabled,
CppConfiguration cppConfiguration,
CppCompilationContext context,
Class<? extends CppCompileActionContext> actionContext,
@@ -315,7 +314,6 @@
CollectionUtils.asListWithoutNulls(
outputFile, (dotdFile == null ? null : dotdFile.artifact()), gcnoFile, dwoFile));
this.localShellEnvironment = localShellEnvironment;
- this.isCodeCoverageEnabled = isCodeCoverageEnabled;
this.sourceLabel = sourceLabel;
this.sourceFile = sourceFile;
this.outputFile = Preconditions.checkNotNull(outputFile);
@@ -697,7 +695,9 @@
@Override
public ImmutableMap<String, String> getEnvironment() {
Map<String, String> environment = new LinkedHashMap<>(localShellEnvironment);
- if (isCodeCoverageEnabled) {
+ if (!getExecutionInfo().containsKey(ExecutionRequirements.REQUIRES_DARWIN)) {
+ // Linux: this prevents gcc/clang from writing the unpredictable (and often irrelevant) value
+ // of getcwd() into the debug info. Not applicable to Darwin or Windows, which have no /proc.
environment.put("PWD", "/proc/self/cwd");
}
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 cc83edd..aa666f1 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
@@ -362,7 +362,6 @@
tempOutputFile,
dotdFile,
localShellEnvironment,
- codeCoverageEnabled,
cppConfiguration,
context,
actionContext,
@@ -393,7 +392,6 @@
dwoFile,
optionalSourceFile,
localShellEnvironment,
- codeCoverageEnabled,
cppConfiguration,
context,
actionContext,
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 7df15c0..26b3fc5 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
@@ -75,7 +75,6 @@
PathFragment tempOutputFile,
DotdFile dotdFile,
ImmutableMap<String, String> localShellEnvironment,
- boolean codeCoverageEnabled,
CppConfiguration cppConfiguration,
CppCompilationContext context,
Class<? extends CppCompileActionContext> actionContext,
@@ -105,7 +104,6 @@
null,
null,
localShellEnvironment,
- codeCoverageEnabled,
cppConfiguration,
// We only allow inclusion of header files explicitly declared in
// "srcs", so we only use declaredIncludeSrcs, not declaredIncludeDirs.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
index 7ec2b85..366bcd4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
@@ -14,18 +14,18 @@
package com.google.devtools.build.lib.rules.cpp;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
-import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.BaseSpawn;
+import com.google.devtools.build.lib.actions.EmptyRunfilesSupplier;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionStrategy;
-import com.google.devtools.build.lib.actions.Executor;
-import com.google.devtools.build.lib.actions.ResourceSet;
+import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
+import com.google.devtools.build.lib.actions.UserExecException;
/**
* A context for C++ compilation that calls into a {@link SpawnActionContext}.
@@ -35,26 +35,6 @@
name = {"spawn"}
)
public class SpawnGccStrategy implements CppCompileActionContext {
-
- /**
- * A {@link Spawn} that wraps a {@link CppCompileAction} and adds its
- * {@code additionalInputs} (potential files included) to its inputs.
- */
- private static class GccSpawn extends BaseSpawn {
- private final Iterable<? extends ActionInput> inputs;
-
- public GccSpawn(CppCompileAction action, ResourceSet resources) {
- super(action.getArgv(), action.getEnvironment(), action.getExecutionInfo(), action,
- resources);
- this.inputs = Iterables.concat(action.getInputs(), action.getAdditionalInputs());
- }
-
- @Override
- public Iterable<? extends ActionInput> getInputFiles() {
- return ImmutableSet.copyOf(inputs);
- }
- }
-
@Override
public Iterable<Artifact> findAdditionalInputs(
CppCompileAction action,
@@ -68,10 +48,27 @@
public CppCompileActionContext.Reply execWithReply(
CppCompileAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
- Executor executor = actionExecutionContext.getExecutor();
- SpawnActionContext spawnActionContext = executor.getSpawnActionContext(action.getMnemonic());
- Spawn spawn = new GccSpawn(action, action.estimateResourceConsumptionLocal());
- spawnActionContext.exec(spawn, actionExecutionContext);
+ if (action.getDotdFile() != null && action.getDotdFile().artifact() == null) {
+ throw new UserExecException("cannot execute remotely or locally: "
+ + action.getPrimaryInput().getExecPathString());
+ }
+ Iterable<Artifact> inputs = Iterables.concat(action.getInputs(), action.getAdditionalInputs());
+ Spawn spawn = new SimpleSpawn(
+ action,
+ ImmutableList.copyOf(action.getArgv()),
+ ImmutableMap.copyOf(action.getEnvironment()),
+ ImmutableMap.copyOf(action.getExecutionInfo()),
+ EmptyRunfilesSupplier.INSTANCE,
+ ImmutableList.<Artifact>copyOf(inputs),
+ /*tools=*/ImmutableList.<Artifact>of(),
+ /*filesetManifests=*/ImmutableList.<Artifact>of(),
+ action.getOutputs().asList(),
+ action.estimateResourceConsumptionLocal());
+
+ actionExecutionContext
+ .getExecutor()
+ .getSpawnActionContext(action.getMnemonic())
+ .exec(spawn, actionExecutionContext);
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
index 12b3fdc..28a0b2a 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SpawnHelpers.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.sandbox;
-import com.google.common.collect.Iterables;
import com.google.common.io.Files;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionInput;
@@ -22,7 +21,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.analysis.AnalysisUtils;
-import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
import com.google.devtools.build.lib.rules.fileset.FilesetActionContext;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.FileSystem;
@@ -169,13 +167,6 @@
ActionInputHelper.expandArtifacts(
spawn.getInputFiles(), actionExecutionContext.getArtifactExpander());
- if (spawn.getResourceOwner() instanceof CppCompileAction) {
- CppCompileAction action = (CppCompileAction) spawn.getResourceOwner();
- if (action.shouldScanIncludes()) {
- Iterables.addAll(inputs, action.getAdditionalInputs());
- }
- }
-
// ActionInputHelper#expandArtifacts above expands empty TreeArtifacts into an empty list.
// However, actions that accept TreeArtifacts as inputs generally expect that the empty
// directory is created. So here we explicitly mount the directories of the TreeArtifacts as
