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
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 2e50c12..65009a3 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 @@
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 @@
}
/**
+ * 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 78d7282..de646ac 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 @@
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 7ce177a..6ac746f 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 @@
}
}
- 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 aba401f..c615e3a 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 @@
.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 6d1655e..007bc85 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 @@
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 ed1594e..91b3437 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 @@
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 24ae5dc..3d2d555 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 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);
}
// =================================================================
