Fixes issue with ObjcCompileAction where extra action funcitonality is unable

to generate extra action proto file.

--
PiperOrigin-RevId: 148479293
MOS_MIGRATED_REVID=148479293

3 files changed, 49 insertions, 12 deletions

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 05920e1bf5..1aa3ea07e7 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
@@ -382,9 +382,7 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
   public ExtraActionInfo.Builder getExtraActionInfo() {
     ExtraActionInfo.Builder builder = super.getExtraActionInfo();
     if (extraActionInfoSupplier == null) {
-      Spawn spawn = getSpawn();
-      SpawnInfo spawnInfo = getExtraActionInfo(spawn);
-
+      SpawnInfo spawnInfo = getExtraActionSpawnInfo();
       return builder
           .setExtension(SpawnInfo.spawnInfo, spawnInfo);
     } else {
@@ -393,9 +391,15 @@ public class SpawnAction extends AbstractAction implements ExecutionInfoSpecifie
     }
   }
 
-  private static SpawnInfo getExtraActionInfo(Spawn spawn) {
+  /**
+   * Returns information about this spawn action for use by the extra action mechanism.
+   *
+   * <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.
+   */
+  protected SpawnInfo getExtraActionSpawnInfo() {
     SpawnInfo.Builder info = SpawnInfo.newBuilder();
-
+    Spawn spawn = getSpawn();
     info.addAllArgument(spawn.getArguments());
     for (Map.Entry<String, String> variable : spawn.getEnvironment().entrySet()) {
       info.addVariable(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD b/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
index 73396b2d9b..a44e3afb9f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
@@ -27,6 +27,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:bundlemerge_java_proto",
+        "//src/main/protobuf:extra_actions_base_java_proto",
         "//src/main/protobuf:plmerge_java_proto",
         "//src/main/protobuf:xcodegen_java_proto",
         "//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
index 8e578b6a65..f3d185cc01 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompileAction.java
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.actions.Spawn;
+import com.google.devtools.build.lib.actions.extra.SpawnInfo;
 import com.google.devtools.build.lib.analysis.actions.CommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -90,10 +91,14 @@ public class ObjcCompileAction extends SpawnAction {
 
     @Override
     public Iterable<? extends ActionInput> getInputFiles() {
-      return ImmutableList.<ActionInput>builder()
-          .addAll(super.getInputFiles())
-          .addAll(discoveredInputs)
-          .build();
+      ImmutableList.Builder<ActionInput> listBuilder =
+          ImmutableList.<ActionInput>builder().addAll(super.getInputFiles());
+      // Normally discoveredInputs should not be null when this is called, however that may occur if
+      // the extra action feature is used
+      if (discoveredInputs != null) {
+        listBuilder.addAll(discoveredInputs);
+      }
+      return listBuilder.build();
     }
   }
 
@@ -135,7 +140,7 @@ public class ObjcCompileAction extends SpawnAction {
     super(
         owner,
         tools,
-        inputs,
+        headersListFile == null ? inputs : mandatoryInputs,
         outputs,
         resourceSet,
         argv,
@@ -161,9 +166,11 @@ public class ObjcCompileAction extends SpawnAction {
     ImmutableList.Builder<Artifact> inputs = ImmutableList.<Artifact>builder();
 
     for (Artifact headerArtifact : headers) {
-      if (CppFileTypes.OBJC_HEADER.matches(headerArtifact.getFilename())) {
+      if (CppFileTypes.OBJC_HEADER.matches(headerArtifact.getFilename())
+          // C++ headers can be extensionless
+          || (!headerArtifact.isFileset() && headerArtifact.getExtension().isEmpty())) {
           inputs.add(headerArtifact);
-        }
+      }
     }
     return inputs.build();
   }
@@ -380,6 +387,31 @@ public class ObjcCompileAction extends SpawnAction {
   }
 
   @Override
+  public Iterable<Artifact> getInputFilesForExtraAction(
+      ActionExecutionContext actionExecutionContext)
+      throws ActionExecutionException, InterruptedException {
+    // Use a set to eliminate duplicates.
+    return ImmutableSet.<Artifact>builder()
+        .addAll(getInputs())
+        .addAll(discoverInputs(actionExecutionContext))
+        .build();
+  }
+
+  @Override
+  protected SpawnInfo getExtraActionSpawnInfo() {
+    SpawnInfo.Builder info = SpawnInfo.newBuilder(super.getExtraActionSpawnInfo());
+    if (!inputsKnown()) {
+      for (Artifact headerArtifact : filterHeaderFiles()) {
+        // As in SpawnAction#getExtraActionSpawnInfo explicitly ignore middleman artifacts here.
+        if (!headerArtifact.isMiddlemanArtifact()) {
+          info.addInputFile(headerArtifact.getExecPathString());
+        }
+      }
+    }
+    return info.build();
+  }
+
+  @Override
   public String computeKey() {
     Fingerprint f = new Fingerprint();
     f.addString(GUID);