Enables objc_proto_library to depend on other objc_proto_libraries. This only works when using the new version of objc_proto_library by adding the portable_proto_filters attribute.

--
MOS_MIGRATED_REVID=130952647

3 files changed, 77 insertions, 7 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index 3a48369cb8..394f0e403a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -438,7 +438,7 @@ public class BazelRuleClassProvider {
     builder.addRuleDefinition(new ObjcFrameworkRule());
     builder.addRuleDefinition(new ObjcImportRule());
     builder.addRuleDefinition(new ObjcLibraryRule());
-    builder.addRuleDefinition(new ObjcProtoLibraryRule());
+    builder.addRuleDefinition(new ObjcProtoLibraryRule(objcProtoAspect));
     builder.addRuleDefinition(new ObjcXcodeprojRule());
     builder.addRuleDefinition(new ObjcRuleClasses.CoptsRule());
     builder.addRuleDefinition(new ObjcRuleClasses.BundlingRule());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java
index aad39c923e..326ce49f02 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoLibraryRule.java
@@ -49,6 +49,19 @@ public class ObjcProtoLibraryRule implements RuleDefinition {
 
   static final String XCODE_GEN_ATTR = "$xcodegen";
 
+  private final ObjcProtoAspect objcProtoAspect;
+
+  /**
+   * Returns a newly built rule definition for objc_proto_library.
+   *
+   * @param objcProtoAspect Aspect that traverses the dependency graph through the deps attribute
+   *                        to gather all proto files and portable filters depended by
+   *                        objc_proto_library targets using the new protobuf compiler/library.
+   */
+  public ObjcProtoLibraryRule(ObjcProtoAspect objcProtoAspect) {
+    this.objcProtoAspect = objcProtoAspect;
+  }
+
   @Override
   public RuleClass build(Builder builder, final RuleDefinitionEnvironment env) {
     return builder
@@ -59,7 +72,8 @@ public class ObjcProtoLibraryRule implements RuleDefinition {
         .override(
             attr("deps", LABEL_LIST)
                 // Support for files in deps is for backwards compatibility.
-                .allowedRuleClasses("proto_library", "filegroup")
+                .allowedRuleClasses("proto_library", "filegroup", "objc_proto_library")
+                .aspect(objcProtoAspect)
                 .legacyAllowAnyFileType())
         /* <!-- #BLAZE_RULE(objc_proto_library).ATTRIBUTE(options_file) -->
         Optional options file to apply to protos which affects compilation (e.g. class
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 bc7e8bdc7a..e87febff44 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
@@ -21,9 +21,11 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.AbstractConfiguredTarget;
 import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
@@ -50,6 +52,18 @@ final class ProtoAttributes {
   static final String PORTABLE_PROTO_FILTERS_EMPTY_ERROR =
       "The portable_proto_filters attribute can't be empty";
 
+  @VisibleForTesting
+  static final String OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR =
+      "Protocol Buffers 2 objc_proto_library targets can't depend on other objc_proto_library "
+          + "targets. Please migrate your Protocol Buffers 2 objc_proto_library targets to use the "
+          + "portable_proto_filters attribute.";
+
+  @VisibleForTesting
+  static final String PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR =
+      "Protobuf objc_proto_library targets can't depend on Protocol Buffers 2 objc_proto_library "
+          + "targets. Please migrate your Protocol Buffers 2 objc_proto_library targets to use the "
+          + "portable_proto_filters attribute.";
+
   private final RuleContext ruleContext;
 
   /**
@@ -73,11 +87,6 @@ final class ProtoAttributes {
    * </ul>
    */
   public void validate() throws RuleErrorException {
-
-    if (getProtoFiles().isEmpty()) {
-      ruleContext.throwWithRuleError(NO_PROTOS_ERROR);
-    }
-
     PrerequisiteArtifacts prerequisiteArtifacts =
         ruleContext.getPrerequisiteArtifacts("deps", Mode.TARGET);
     ImmutableList<Artifact> protos = prerequisiteArtifacts.filter(FileType.of(".proto")).list();
@@ -88,6 +97,10 @@ final class ProtoAttributes {
     if (ruleContext
         .attributes()
         .isAttributeValueExplicitlySpecified(ObjcProtoLibraryRule.PORTABLE_PROTO_FILTERS_ATTR)) {
+      if (getProtoFiles().isEmpty() && !hasObjcProtoLibraryDependencies()) {
+        ruleContext.throwWithRuleError(NO_PROTOS_ERROR);
+      }
+
       if (getPortableProtoFilters().isEmpty()) {
         ruleContext.throwWithRuleError(PORTABLE_PROTO_FILTERS_EMPTY_ERROR);
       }
@@ -98,7 +111,15 @@ final class ProtoAttributes {
           || getOptionsFile().isPresent()) {
         ruleContext.throwWithRuleError(PORTABLE_PROTO_FILTERS_NOT_EXCLUSIVE_ERROR);
       }
+      if (hasPB2Dependencies()) {
+        ruleContext.throwWithRuleError(PROTOCOL_BUFFERS2_IN_PROTOBUF_DEPS_ERROR);
+      }
+
     } else {
+      if (getProtoFiles().isEmpty()) {
+        ruleContext.throwWithRuleError(NO_PROTOS_ERROR);
+      }
+
       if (outputsCpp()) {
         ruleContext.ruleWarning(
             "The output_cpp attribute has been deprecated. Please "
@@ -109,6 +130,9 @@ final class ProtoAttributes {
             "As part of the migration process, it is recommended to enable "
                 + "use_objc_header_names. Please refer to b/29368416 for more information.");
       }
+      if (hasObjcProtoLibraryDependencies()) {
+        ruleContext.throwWithRuleError(OBJC_PROTO_LIB_DEP_IN_PROTOCOL_BUFFERS2_DEPS_ERROR);
+      }
     }
   }
 
@@ -217,4 +241,36 @@ final class ProtoAttributes {
         ruleContext.getPrerequisiteArtifacts("deps", Mode.TARGET);
     return prerequisiteArtifacts.filter(FileType.of(".proto")).list();
   }
+
+  private boolean hasPB2Dependencies() {
+    for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
+      if (isObjcProtoLibrary(dep) && !hasProtobufProvider(dep)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private boolean hasObjcProtoLibraryDependencies() {
+    for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
+      if (isObjcProtoLibrary(dep)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  private boolean isObjcProtoLibrary(TransitiveInfoCollection dependency) {
+    try {
+      AbstractConfiguredTarget target = (AbstractConfiguredTarget) dependency;
+      String targetName = target.getTarget().getTargetKind();
+      return targetName.equals("objc_proto_library rule");
+    } catch (Exception e) {
+      return false;
+    }
+  }
+
+  private boolean hasProtobufProvider(TransitiveInfoCollection dependency) {
+    return dependency.getProvider(ObjcProtoProvider.class) != null;
+  }
 }