Removes the generation of modulemaps per proto group, and only adds one per proto target. Removes module map generation for binary targets.

Filter well known protos from ProtocolBuffers2 as there may be some proto_library targets including them.

--
MOS_MIGRATED_REVID=131093115

3 files changed, 79 insertions, 53 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java
index e87febff44..0824d2c107 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtoAttributes.java
@@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.AbstractConfiguredTarget;
 import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
@@ -32,6 +33,7 @@ import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.
 import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 /** Common rule attributes used by an objc_proto_library. */
 final class ProtoAttributes {
@@ -64,6 +66,8 @@ final class ProtoAttributes {
           + "targets. Please migrate your Protocol Buffers 2 objc_proto_library targets to use the "
           + "portable_proto_filters attribute.";
 
+  private static final PathFragment BAZEL_TOOLS_PREFIX = new PathFragment("external/bazel_tools/");
+
   private final RuleContext ruleContext;
 
   /**
@@ -221,6 +225,46 @@ final class ProtoAttributes {
         .list();
   }
 
+  /**
+   * Filters the well known protos from the given list of proto files. This should be used to
+   * prevent the well known protos from being generated as they are already generated in the runtime
+   * library.
+   */
+  ImmutableSet<Artifact> filterWellKnownProtos(Iterable<Artifact> protoFiles) {
+    // Since well known protos are already linked in the runtime library, we have to filter them
+    // so they don't get generated again.
+    ImmutableSet.Builder<Artifact> filteredProtos = new ImmutableSet.Builder<Artifact>();
+    for (Artifact protoFile : protoFiles) {
+      if (!isProtoWellKnown(protoFile)) {
+        filteredProtos.add(protoFile);
+      }
+    }
+    return filteredProtos.build();
+  }
+
+  /** Returns whether the given proto is a well known proto or not. */
+  boolean isProtoWellKnown(Artifact protoFile) {
+    return getWellKnownProtoPaths().contains(protoFile.getExecPath());
+  }
+
+  private ImmutableSet<PathFragment> getWellKnownProtoPaths() {
+    ImmutableSet.Builder<PathFragment> wellKnownProtoPathsBuilder = new ImmutableSet.Builder<>();
+    Iterable<Artifact> wellKnownProtoFiles =
+        ruleContext
+            .getPrerequisiteArtifacts(ObjcRuleClasses.PROTOBUF_WELL_KNOWN_TYPES, Mode.HOST)
+            .list();
+    for (Artifact wellKnownProtoFile : wellKnownProtoFiles) {
+      PathFragment execPath = wellKnownProtoFile.getExecPath();
+      if (execPath.startsWith(BAZEL_TOOLS_PREFIX)) {
+        wellKnownProtoPathsBuilder.add(execPath.relativeTo(BAZEL_TOOLS_PREFIX));
+      } else {
+        wellKnownProtoPathsBuilder.add(execPath);
+      }
+    }
+    return wellKnownProtoPathsBuilder.build();
+  }
+
+
   /** Returns the sets of proto files that were added using proto_library dependencies. */
   private NestedSet<Artifact> getProtoDepsSources() {
     NestedSetBuilder<Artifact> artifacts = NestedSetBuilder.stableOrder();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
index f4a926759f..0af84d64d1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
@@ -60,8 +60,6 @@ import java.util.Set;
  */
 final class ProtobufSupport {
 
-  private static final PathFragment BAZEL_TOOLS_PREFIX = new PathFragment("external/bazel_tools/");
-
   private static final String BUNDLED_PROTOS_IDENTIFIER = "BundledProtos";
 
   /**
@@ -129,24 +127,36 @@ final class ProtobufSupport {
    */
   public ProtobufSupport registerGenerationActions() {
     int actionId = 0;
+
     for (ImmutableSet<Artifact> inputProtos : inputsToOutputsMap.keySet()) {
       Iterable<Artifact> outputProtos = inputsToOutputsMap.get(inputProtos);
-
       registerGenerationAction(outputProtos, inputProtos, getUniqueBundledProtosSuffix(actionId));
+      actionId++;
+    }
 
-      IntermediateArtifacts intermediateArtifacts = getUniqueIntermediateArtifacts(actionId);
+    if (!isLinkingTarget()) {
+      registerModuleMapGenerationAction();
+    }
 
-      CompilationArtifacts compilationArtifacts =
-          getCompilationArtifacts(intermediateArtifacts, inputProtos, outputProtos);
+    return this;
+  }
 
-      ObjcCommon common = getCommon(intermediateArtifacts, compilationArtifacts);
+  private void registerModuleMapGenerationAction() {
+    IntermediateArtifacts moduleMapIntermediateArtifacts =
+        ObjcRuleClasses.intermediateArtifacts(ruleContext);
+    CompilationArtifacts.Builder moduleMapCompilationArtifacts =
+        new CompilationArtifacts.Builder()
+            .setIntermediateArtifacts(moduleMapIntermediateArtifacts)
+            .setPchFile(Optional.<Artifact>absent())
+            .addAdditionalHdrs(getProtobufHeaders())
+            .addAdditionalHdrs(
+                getGeneratedProtoOutputs(inputsToOutputsMap.values(), ".pbobjc.h"));
 
-      new CompilationSupport(
-              ruleContext, intermediateArtifacts, new CompilationAttributes.Builder().build())
-          .registerGenerateModuleMapAction(common.getCompilationArtifacts());
-      actionId++;
-    }
-    return this;
+    new CompilationSupport(
+            ruleContext,
+            ObjcRuleClasses.intermediateArtifacts(ruleContext),
+            new CompilationAttributes.Builder().build())
+        .registerGenerateModuleMapAction(Optional.of(moduleMapCompilationArtifacts.build()));
   }
 
   /** Registers the actions that will compile the generated code. */
@@ -210,7 +220,12 @@ final class ProtobufSupport {
 
     Iterable<PathFragment> userHeaderSearchPaths =
         ImmutableList.of(getWorkspaceRelativeOutputDir());
-    ObjcCommon.Builder commonBuilder = new ObjcCommon.Builder(ruleContext);
+    IntermediateArtifacts moduleMapIntermediateArtifacts =
+        ObjcRuleClasses.intermediateArtifacts(ruleContext);
+    ObjcCommon.Builder commonBuilder =
+        new ObjcCommon.Builder(ruleContext)
+            .setIntermediateArtifacts(moduleMapIntermediateArtifacts)
+            .setHasModuleMap();
 
     int actionId = 0;
     for (ImmutableSet<Artifact> inputProtos : inputsToOutputsMap.keySet()) {
@@ -326,7 +341,7 @@ final class ProtobufSupport {
       for (Artifact proto : protoSet) {
         // If the proto is well known, don't store it as we don't need to generate it; it comes
         // generated with the runtime library.
-        if (isProtoWellKnown(proto)) {
+        if (attributes.isProtoWellKnown(proto)) {
           continue;
         }
         if (!protoToGroupMap.containsKey(proto)) {
@@ -415,7 +430,6 @@ final class ProtobufSupport {
     ObjcCommon.Builder commonBuilder =
         new ObjcCommon.Builder(ruleContext)
             .setIntermediateArtifacts(intermediateArtifacts)
-            .setHasModuleMap()
             .setCompilationArtifacts(compilationArtifacts);
     if (isLinkingTarget()) {
       commonBuilder.addUserHeaderSearchPaths(getProtobufHeaderSearchPaths());
@@ -433,7 +447,7 @@ final class ProtobufSupport {
       Iterable<Artifact> outputProtoFiles) {
     // Filter the well known protos from the set of headers. We don't generate the headers for them
     // as they are part of the runtime library.
-    Iterable<Artifact> filteredInputProtos = filterWellKnownProtos(inputProtoFiles);
+    Iterable<Artifact> filteredInputProtos = attributes.filterWellKnownProtos(inputProtoFiles);
 
     CompilationArtifacts.Builder compilationArtifacts =
         new CompilationArtifacts.Builder()
@@ -627,39 +641,6 @@ final class ProtobufSupport {
     return casedSegments.toString();
   }
 
-  private ImmutableSet<Artifact> filterWellKnownProtos(Iterable<Artifact> protoFiles) {
-    // Since well known protos are already linked in the runtime library, we have to filter them
-    // so they don't get generated again.
-    ImmutableSet.Builder<Artifact> filteredProtos = new ImmutableSet.Builder<Artifact>();
-    for (Artifact protoFile : protoFiles) {
-      if (!isProtoWellKnown(protoFile)) {
-        filteredProtos.add(protoFile);
-      }
-    }
-    return filteredProtos.build();
-  }
-
-  private ImmutableSet<PathFragment> getWellKnownProtoPaths(RuleContext ruleContext) {
-    ImmutableSet.Builder<PathFragment> wellKnownProtoPathsBuilder = new ImmutableSet.Builder<>();
-    Iterable<Artifact> wellKnownProtoFiles =
-        ruleContext
-            .getPrerequisiteArtifacts(ObjcRuleClasses.PROTOBUF_WELL_KNOWN_TYPES, Mode.HOST)
-            .list();
-    for (Artifact wellKnownProtoFile : wellKnownProtoFiles) {
-      PathFragment execPath = wellKnownProtoFile.getExecPath();
-      if (execPath.startsWith(BAZEL_TOOLS_PREFIX)) {
-        wellKnownProtoPathsBuilder.add(execPath.relativeTo(BAZEL_TOOLS_PREFIX));
-      } else {
-        wellKnownProtoPathsBuilder.add(execPath);
-      }
-    }
-    return wellKnownProtoPathsBuilder.build();
-  }
-
-  private boolean isProtoWellKnown(Artifact protoFile) {
-    return getWellKnownProtoPaths(ruleContext).contains(protoFile.getExecPath());
-  }
-
   private boolean isLinkingTarget() {
     return !ruleContext
         .attributes()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
index f67ebf7b91..87f22d946d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
@@ -73,7 +73,8 @@ final class ProtocolBuffers2Support {
         new FileWriteAction(
             ruleContext.getActionOwner(),
             getProtoInputsFile(),
-            getProtoInputsFileContents(attributes.getProtoFiles()),
+            getProtoInputsFileContents(
+                attributes.filterWellKnownProtos(attributes.getProtoFiles())),
             false));
 
     ruleContext.registerAction(
@@ -82,7 +83,7 @@ final class ProtocolBuffers2Support {
             .addInput(attributes.getProtoCompiler())
             .addInputs(attributes.getProtoCompilerSupport())
             .addInput(getProtoInputsFile())
-            .addInputs(attributes.getProtoFiles())
+            .addTransitiveInputs(attributes.getProtoFiles())
             .addInputs(attributes.getOptionsFile().asSet())
             .addOutputs(getGeneratedProtoOutputs(getHeaderExtension()))
             .addOutputs(getGeneratedProtoOutputs(getSourceExtension()))
@@ -231,7 +232,7 @@ final class ProtocolBuffers2Support {
 
   private Iterable<Artifact> getGeneratedProtoOutputs(String extension) {
     ImmutableList.Builder<Artifact> builder = new ImmutableList.Builder<>();
-    for (Artifact protoFile : attributes.getProtoFiles()) {
+    for (Artifact protoFile : attributes.filterWellKnownProtos(attributes.getProtoFiles())) {
       String protoFileName = FileSystemUtils.removeExtension(protoFile.getFilename());
       String generatedOutputName;
       if (attributes.outputsCpp()) {