Clean up *GccStrategy

- use SimpleSpawn in SpawnGccStrategy
- set PWD in CppCompileAction for consistency

--
PiperOrigin-RevId: 149745059
MOS_MIGRATED_REVID=149745059

5 files changed, 30 insertions, 46 deletions

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 e8356092ce..2afd77d857 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.Executor;
 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 class CppCompileAction extends AbstractAction
   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 @@ public class CppCompileAction extends AbstractAction
       @Nullable Artifact dwoFile,
       Artifact optionalSourceFile,
       ImmutableMap<String, String> localShellEnvironment,
-      boolean isCodeCoverageEnabled,
       CppConfiguration cppConfiguration,
       CppCompilationContext context,
       Class<? extends CppCompileActionContext> actionContext,
@@ -315,7 +314,6 @@ public class CppCompileAction extends AbstractAction
         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 @@ public class CppCompileAction extends AbstractAction
   @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 cc83edd40f..aa666f1827 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 @@ public class CppCompileActionBuilder {
           tempOutputFile,
           dotdFile,
           localShellEnvironment,
-          codeCoverageEnabled,
           cppConfiguration,
           context,
           actionContext,
@@ -393,7 +392,6 @@ public class CppCompileActionBuilder {
           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 7df15c067d..26b3fc5a48 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 @@ public class FakeCppCompileAction extends CppCompileAction {
       PathFragment tempOutputFile,
       DotdFile dotdFile,
       ImmutableMap<String, String> localShellEnvironment,
-      boolean codeCoverageEnabled,
       CppConfiguration cppConfiguration,
       CppCompilationContext context,
       Class<? extends CppCompileActionContext> actionContext,
@@ -105,7 +104,6 @@ public class FakeCppCompileAction extends CppCompileAction {
         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 7ec2b85121..366bcd47ef 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 @@ import com.google.devtools.build.lib.actions.SpawnActionContext;
   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 class SpawnGccStrategy implements CppCompileActionContext {
   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 12b3fdc3e6..28a0b2a4dc 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.ActionInputHelper;
 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 @@ public final class SpawnHelpers {
         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