Use a NestedSet for proto_library check deps sources

--
PiperOrigin-RevId: 145122327
MOS_MIGRATED_REVID=145122327

3 files changed, 19 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
index c524829ce9..c97a746b06 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java
@@ -39,7 +39,7 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory {
       throws InterruptedException, RuleErrorException {
     ImmutableList<Artifact> protoSources =
         ruleContext.getPrerequisiteArtifacts("srcs", TARGET).list();
-    ImmutableList<Artifact> checkDepsProtoSources =
+    NestedSet<Artifact> checkDepsProtoSources =
         ProtoCommon.getCheckDepsProtoSources(ruleContext, protoSources);
     ProtoCommon.checkSourceFilesAreInSamePackage(ruleContext);
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
index 715a73382a..6fc37c6676 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.proto;
 
 import static com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode.TARGET;
+import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
 import static com.google.devtools.build.lib.packages.BuildType.TRISTATE;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -45,30 +46,29 @@ public class ProtoCommon {
   }
 
   /**
-   * Gets the direct sources of a proto library. If protoSources is not empty,
-   * the value is just protoSources. Otherwise, it's the combined sources of all direct dependencies
-   * of the given RuleContext.
+   * Gets the direct sources of a proto library. If protoSources is not empty, the value is just
+   * protoSources. Otherwise, it's the combined sources of all direct dependencies of the given
+   * RuleContext.
+   *
    * @param ruleContext the proto library rule context.
    * @param protoSources the direct proto sources.
    * @return the direct sources of a proto library.
    */
-  // TODO(bazel-team): Proto sources should probably be a NestedSet.
-  public static ImmutableList<Artifact> getCheckDepsProtoSources(
+  public static NestedSet<Artifact> getCheckDepsProtoSources(
       RuleContext ruleContext, ImmutableList<Artifact> protoSources) {
 
     if (protoSources.isEmpty()) {
       /* a proxy/alias library, return the sources of the direct deps */
-      ImmutableList.Builder<Artifact> builder = new ImmutableList.Builder<>();
-      for (TransitiveInfoCollection provider : ruleContext
-          .getPrerequisites("deps", Mode.TARGET)) {
+      NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
+      for (TransitiveInfoCollection provider : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
         ProtoSourcesProvider sources = provider.getProvider(ProtoSourcesProvider.class);
         if (sources != null) {
-          builder.addAll(sources.getCheckDepsProtoSources());
+          builder.addTransitive(sources.getCheckDepsProtoSources());
         }
       }
       return builder.build();
     } else {
-      return protoSources;
+      return NestedSetBuilder.wrap(STABLE_ORDER, protoSources);
     }
   }
 
@@ -181,7 +181,7 @@ public class ProtoCommon {
     } else {
       for (ProtoSourcesProvider provider :
           ruleContext.getPrerequisites("deps", TARGET, ProtoSourcesProvider.class)) {
-        result.addAll(provider.getCheckDepsProtoSources());
+        result.addTransitive(provider.getCheckDepsProtoSources());
       }
       result.addAll(srcs);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java
index 412cebd98a..93d9f91f39 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourcesProvider.java
@@ -40,7 +40,7 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider {
       NestedSet<Artifact> transitiveImports,
       NestedSet<Artifact> transitiveProtoSources,
       ImmutableList<Artifact> protoSources,
-      ImmutableList<Artifact> checkDepsProtoSources,
+      NestedSet<Artifact> checkDepsProtoSources,
       @Nullable Artifact descriptorSet) {
     return new AutoValue_ProtoSourcesProvider(
         transitiveImports,
@@ -78,7 +78,6 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider {
   /**
    * Returns the proto sources from the 'srcs' attribute.
    */
-  // TODO(bazel-team): This should be NestedSets.
   @SkylarkCallable(
     name = "direct_sources",
     doc = "Proto sources from the 'srcs' attribute.",
@@ -87,10 +86,12 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider {
   public abstract ImmutableList<Artifact> getDirectProtoSources();
 
   /**
-   * Returns the proto sources from the 'srcs' attribute. If the library is a proxy library
-   * that has no sources, return the sources from the direct deps.
+   * Returns the proto sources from the 'srcs' attribute. If the library is a proxy library that has
+   * no sources, return the sources from the direct deps.
+   *
+   * <p>This must be a set to avoid collecting the same source twice when depending on 2 proxy 
+   * proto_library's that depend on the same proto_library.
    */
-  // TODO(bazel-team): This should be NestedSets.
   @SkylarkCallable(
     name = "check_deps_sources",
     doc =
@@ -99,7 +100,7 @@ public abstract class ProtoSourcesProvider implements TransitiveInfoProvider {
             + "from this library's direct deps.",
     structField = true
   )
-  public abstract ImmutableList<Artifact> getCheckDepsProtoSources();
+  public abstract NestedSet<Artifact> getCheckDepsProtoSources();
 
   /**
    * Be careful while using this artifact - it is the parsing of the transitive set of .proto files.