Wrap runtime jars and proto sources going into runfiles in a stable order nested set.

This saves memory, as the nested set would otherwise become flattened. Assuming the jars don't have duplicates files with the same root relative path in the same runfiles tree this won't make any semantic difference.

PiperOrigin-RevId: 169234428

7 files changed, 36 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
index 2e50c123e6..65009a3254 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
@@ -828,9 +828,9 @@ public final class Runfiles {
       return this;
     }
 
-
     /**
      * @deprecated Use {@link #addTransitiveArtifacts} instead, to prevent increased memory use.
+     *     <p>See alse {@link Builder#addTransitiveArtifactsWrappedInStableOrder}
      */
     @Deprecated
     public Builder addArtifacts(NestedSet<Artifact> artifacts) {
@@ -850,6 +850,20 @@ public final class Runfiles {
     }
 
     /**
+     * Adds a nested set to the internal collection.
+     *
+     * <p>The nested set will become wrapped in stable order. Only use this when the set of
+     * artifacts will not have conflicting root relative paths, or the wrong artifact will end up in
+     * the runfiles tree.
+     */
+    public Builder addTransitiveArtifactsWrappedInStableOrder(NestedSet<Artifact> artifacts) {
+      NestedSet<Artifact> wrappedArtifacts =
+          NestedSetBuilder.<Artifact>stableOrder().addTransitive(artifacts).build();
+      artifactsBuilder.addTransitive(wrappedArtifacts);
+      return this;
+    }
+
+    /**
      * Adds a symlink.
      */
     public Builder addSymlink(PathFragment link, Artifact target) {
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index 78d7282aea..de646ac546 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -535,8 +535,10 @@ public class BazelJavaSemantics implements JavaSemantics {
       Runfiles.Builder runfilesBuilder) {
     TransitiveInfoCollection testSupport = getTestSupport(ruleContext);
     if (testSupport != null) {
-      // Not using addTransitiveArtifacts() due to the mismatch in NestedSet ordering.
-      runfilesBuilder.addArtifacts(getRuntimeJarsForTargets(testSupport));
+      // We assume that the runtime jars will not have conflicting artifacts
+      // with the same root relative path
+      runfilesBuilder.addTransitiveArtifactsWrappedInStableOrder(
+          getRuntimeJarsForTargets(testSupport));
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index 7ce177ae8d..6ac746f904 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -442,7 +442,9 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
       }
     }
 
-    builder.addArtifacts(javaCommon.getRuntimeClasspath());
+    // We assume that the runtime jars will not have conflicting artifacts
+    // with the same root relative path
+    builder.addTransitiveArtifactsWrappedInStableOrder(javaCommon.getRuntimeClasspath());
 
     // Add the JDK files if it comes from P4 (see java_stub_template.txt).
     TransitiveInfoCollection javabaseTarget = ruleContext.getPrerequisite(":jvm", Mode.TARGET);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
index aba401f91a..c615e3a4eb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
@@ -443,8 +443,11 @@ public class JavaSkylarkCommon {
         .build();
     JavaCompilationArgsProvider javaCompilationArgsProvider =
         helper.buildCompilationArgsProvider(artifacts, true);
-    Runfiles runfiles = new Runfiles.Builder(skylarkRuleContext.getWorkspaceName()).addArtifacts(
-        javaCompilationArgsProvider.getRecursiveJavaCompilationArgs().getRuntimeJars()).build();
+    Runfiles runfiles =
+        new Runfiles.Builder(skylarkRuleContext.getWorkspaceName())
+            .addTransitiveArtifactsWrappedInStableOrder(
+                javaCompilationArgsProvider.getRecursiveJavaCompilationArgs().getRuntimeJars())
+            .build();
 
     JavaPluginInfoProvider transitivePluginsProvider =
         JavaPluginInfoProvider.merge(Iterables.concat(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoLibrary.java
index 6d1655e8ad..007bc85511 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoLibrary.java
@@ -60,9 +60,11 @@ public class JavaLiteProtoLibrary implements RuleConfiguredTargetFactory {
     JavaCompilationArgsProvider dependencyArgsProviders =
         constructJcapFromAspectDeps(ruleContext, javaProtoLibraryAspectProviders);
 
+    // We assume that the runtime jars will not have conflicting artifacts
+    // with the same root relative path
     Runfiles runfiles =
         new Runfiles.Builder(ruleContext.getWorkspaceName())
-            .addArtifacts(
+            .addTransitiveArtifactsWrappedInStableOrder(
                 dependencyArgsProviders.getRecursiveJavaCompilationArgs().getRuntimeJars())
             .build();
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoLibrary.java
index ed1594ed5d..91b34375a6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoLibrary.java
@@ -52,9 +52,11 @@ public class JavaProtoLibrary implements RuleConfiguredTargetFactory {
     JavaCompilationArgsProvider dependencyArgsProviders =
         constructJcapFromAspectDeps(ruleContext, javaProtoLibraryAspectProviders);
 
+    // We assume that the runtime jars will not have conflicting artifacts
+    // with the same root relative path
     Runfiles runfiles =
         new Runfiles.Builder(ruleContext.getWorkspaceName())
-            .addArtifacts(
+            .addTransitiveArtifactsWrappedInStableOrder(
                 dependencyArgsProviders.getRecursiveJavaCompilationArgs().getRuntimeJars())
             .build();
 
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 24ae5dc47c..3d2d55567e 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
@@ -120,11 +120,11 @@ public class ProtoCommon {
 
   public static Runfiles.Builder createDataRunfilesProvider(
       final NestedSet<Artifact> transitiveProtoSources, RuleContext ruleContext) {
+    // We assume that the proto sources will not have conflicting artifacts
+    // with the same root relative path
     return new Runfiles.Builder(
             ruleContext.getWorkspaceName(), ruleContext.getConfiguration().legacyExternalRunfiles())
-        // TODO(bazel-team): addArtifacts is deprecated, but addTransitive fails
-        // due to nested set ordering restrictions. Figure this out.
-        .addArtifacts(transitiveProtoSources);
+        .addTransitiveArtifactsWrappedInStableOrder(transitiveProtoSources);
   }
 
   // =================================================================