Enforce Starlark nested set depth limit in runfiles.merge() and merge_all()
We want to prevent overly deep nested sets which can cause a stack overflow.
Since we are now accessing --nested_set_depth_limit from additional places
(Runfiles as well as Depset), we want to move the option from
NestedSetOptionsModule to BuildLanguageOptions (where, arguably, it ought to
have been from the start). As a nice side effect, since the depth limit no
longer lives in global static state, depth limit tests are now thread-safe
regardless of sharding.
RELNOTES: runfiles.merge() and merge_all() now respect --nested_set_depth_limit.
If you hit the depth limit because you were calling merge() in a loop, use
merge_all() on a sequence of runfiles objects instead.
PiperOrigin-RevId: 364631293
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 d8afce2..eca8b41 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
@@ -33,6 +33,7 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.starlarkbuildapi.RunfilesApi;
@@ -51,6 +52,9 @@
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.syntax.Location;
/**
@@ -1064,8 +1068,34 @@
}
}
+ private static void verifyNestedSetDepthLimitHelper(
+ NestedSet<?> nestedSet, String name, int limit) throws EvalException {
+ if (nestedSet.getApproxDepth() > limit) {
+ throw Starlark.errorf(
+ "%s depset depth %d exceeds limit (%d)", name, nestedSet.getApproxDepth(), limit);
+ }
+ }
+
+ /**
+ * Checks that the depth of a Runfiles object's nested sets (artifacts, symlinks, root symlinks,
+ * extra middlemen) does not exceed Starlark's depset depth limit, as specified by {@code
+ * --nested_set_depth_limit}.
+ *
+ * @param semantics Starlark semantics providing {@code --nested_set_depth_limit}
+ * @return this object, in the fluent style
+ * @throws EvalException if a nested set in the Runfiles object exceeds the depth limit
+ */
+ private Runfiles verifyNestedSetDepthLimit(StarlarkSemantics semantics) throws EvalException {
+ int limit = semantics.get(BuildLanguageOptions.NESTED_SET_DEPTH_LIMIT);
+ verifyNestedSetDepthLimitHelper(artifacts, "artifacts", limit);
+ verifyNestedSetDepthLimitHelper(symlinks, "symlinks", limit);
+ verifyNestedSetDepthLimitHelper(rootSymlinks, "root symlinks", limit);
+ verifyNestedSetDepthLimitHelper(extraMiddlemen, "extra middlemen", limit);
+ return this;
+ }
+
@Override
- public Runfiles merge(RunfilesApi other) {
+ public Runfiles merge(RunfilesApi other, StarlarkThread thread) throws EvalException {
Runfiles o = (Runfiles) other;
if (isEmpty()) {
// This is not just a memory / performance optimization. The Builder requires a valid suffix,
@@ -1075,11 +1105,15 @@
} else if (o.isEmpty()) {
return this;
}
- return new Runfiles.Builder(suffix, false).merge(this).merge(o).build();
+ return new Runfiles.Builder(suffix, false)
+ .merge(this)
+ .merge(o)
+ .build()
+ .verifyNestedSetDepthLimit(thread.getSemantics());
}
@Override
- public Runfiles mergeAll(Sequence<?> sequence) throws EvalException {
+ public Runfiles mergeAll(Sequence<?> sequence, StarlarkThread thread) throws EvalException {
// The delayed initialization of the Builder is not just a memory / performance optimization.
// The Builder requires a valid suffix, but the {@code Runfiles.EMPTY} singleton has an invalid
// one, which must not be used to construct a Runfiles.Builder.
@@ -1109,7 +1143,7 @@
if (uniqueNonEmptyMergee != null) {
return uniqueNonEmptyMergee;
} else if (builder != null) {
- return builder.build();
+ return builder.build().verifyNestedSetDepthLimit(thread.getSemantics());
} else {
return EMPTY;
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD
index 4cd99a6..d3255ff 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD
@@ -135,7 +135,6 @@
"//src/main/java/com/google/devtools/build/lib/bazel/rules",
"//src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions",
"//src/main/java/com/google/devtools/build/lib/buildeventservice",
- "//src/main/java/com/google/devtools/build/lib/collect/nestedset:options",
"//src/main/java/com/google/devtools/build/lib/dynamic",
"//src/main/java/com/google/devtools/build/lib/metrics:memory-use-recorder",
"//src/main/java/com/google/devtools/build/lib/metrics:metrics_module",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
index 807d74d..f64dd97 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
@@ -53,7 +53,6 @@
StarlarkRepositoryDebugModule.class,
com.google.devtools.build.lib.bazel.debug.WorkspaceRuleModule.class,
com.google.devtools.build.lib.bazel.coverage.BazelCoverageReportModule.class,
- com.google.devtools.build.lib.collect.nestedset.NestedSetOptionsModule.class,
com.google.devtools.build.lib.starlarkdebug.module.StarlarkDebuggerModule.class,
com.google.devtools.build.lib.bazel.repository.RepositoryResolvedModule.class,
com.google.devtools.build.lib.bazel.repository.CacheHitReportingModule.class,
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
index 1a40e07..6b02d36 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
@@ -43,17 +43,6 @@
)
java_library(
- name = "options",
- srcs = ["NestedSetOptionsModule.java"],
- deps = [
- ":nestedset",
- "//src/main/java/com/google/devtools/build/lib:runtime",
- "//src/main/java/com/google/devtools/common/options",
- "//third_party:guava",
- ],
-)
-
-java_library(
name = "testutils",
testonly = 1,
srcs = ["NestedSetCodecTestUtils.java"],
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java
index 7bd10c9..f940ebc 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/Depset.java
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
import javax.annotation.Nullable;
import net.starlark.java.annot.StarlarkAnnotations;
import net.starlark.java.annot.StarlarkBuiltin;
@@ -565,7 +564,7 @@
// check depth limit
int depth = result.getSet().getApproxDepth();
- int limit = depthLimit.get();
+ int limit = semantics.get(BuildLanguageOptions.NESTED_SET_DEPTH_LIMIT);
if (depth > limit) {
throw Starlark.errorf("depset depth %d exceeds limit (%d)", depth, limit);
}
@@ -606,21 +605,6 @@
return o instanceof Sequence && ((Sequence) o).isEmpty();
}
- /**
- * Sets the maximum depth for nested sets constructed by the Starlark {@code depset} function (as
- * set by {@code --nested_set_depth_limit}).
- *
- * @return whether the new limit differs from the old
- */
- public static boolean setDepthLimit(int newLimit) {
- int oldValue = depthLimit.getAndSet(newLimit);
- return oldValue != newLimit;
- }
-
- // The effective default value comes from the --nested_set_depth_limit
- // flag in NestedSetOptionsModule, which overrides this.
- private static final AtomicInteger depthLimit = new AtomicInteger(3500);
-
// Delegate equality to the underlying NestedSet. Otherwise, it's possible to create multiple
// Depset instances wrapping the same NestedSet that aren't equal to each other.
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
index d128dac..06ae545 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
@@ -369,7 +369,7 @@
* <p>This function may return an overapproximation of the true depth if the NestedSet was derived
* from the result of calling {@link #getNonLeaves} or {@link #splitIfExceedsMaximumSize}.
*/
- int getApproxDepth() {
+ public int getApproxDepth() {
return this.depthAndOrder >>> 2;
}
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetOptionsModule.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetOptionsModule.java
deleted file mode 100644
index 851d0f4..0000000
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetOptionsModule.java
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright 2019 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.collect.nestedset;
-
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.runtime.BlazeModule;
-import com.google.devtools.build.lib.runtime.CommandEnvironment;
-import com.google.devtools.common.options.Option;
-import com.google.devtools.common.options.OptionDocumentationCategory;
-import com.google.devtools.common.options.OptionEffectTag;
-import com.google.devtools.common.options.OptionsBase;
-
-/** A {@link BlazeModule} handling options pertaining to {@link NestedSet}. */
-public class NestedSetOptionsModule extends BlazeModule {
-
- /** Command line options controlling the behavior of {@link NestedSet}. */
- public static final class Options extends OptionsBase {
- @Option(
- name = "nested_set_depth_limit",
- defaultValue = "3500",
- documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
- effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
- help =
- "The maximum depth of the graph internal to a depset (also known as NestedSet), above"
- + " which the depset() constructor will fail.")
- public int nestedSetDepthLimit;
- }
-
- @Override
- public void beforeCommand(CommandEnvironment env) {
- Options options = env.getOptions().getOptions(Options.class);
- boolean changed = Depset.setDepthLimit(options.nestedSetDepthLimit);
- if (changed) {
- env.getSkyframeExecutor().resetEvaluator();
- }
- }
-
- @Override
- public Iterable<Class<? extends OptionsBase>> getCommonCommandOptions() {
- return ImmutableList.of(Options.class);
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
index c5316e8..8941b1ad 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
@@ -605,6 +605,16 @@
public long maxComputationSteps;
@Option(
+ name = "nested_set_depth_limit",
+ defaultValue = "3500",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ help =
+ "The maximum depth of the graph internal to a depset (also known as NestedSet), above"
+ + " which the depset() constructor will fail.")
+ public int nestedSetDepthLimit;
+
+ @Option(
name = "experimental_shadowed_action",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -685,6 +695,7 @@
.setBool(INCOMPATIBLE_RESTRICT_STRING_ESCAPES, incompatibleRestrictStringEscapes)
.setBool(INCOMPATIBLE_LINKOPTS_TO_LINKLIBS, incompatibleLinkoptsToLinklibs)
.set(MAX_COMPUTATION_STEPS, maxComputationSteps)
+ .set(NESTED_SET_DEPTH_LIMIT, nestedSetDepthLimit)
.setBool(EXPERIMENTAL_SHADOWED_ACTION, experimentalShadowedAction)
.build();
return INTERNER.intern(semantics);
@@ -771,4 +782,6 @@
"experimental_cc_starlark_api_enabled_packages", ImmutableList.of());
public static final StarlarkSemantics.Key<Long> MAX_COMPUTATION_STEPS =
new StarlarkSemantics.Key<>("max_computation_steps", 0L);
+ public static final StarlarkSemantics.Key<Integer> NESTED_SET_DEPTH_LIMIT =
+ new StarlarkSemantics.Key<>("nested_set_depth_limit", 3500);
}
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunfilesApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunfilesApi.java
index df92dea..4d43026 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunfilesApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunfilesApi.java
@@ -22,6 +22,7 @@
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.StarlarkValue;
/** An interface for a set of runfiles. */
@@ -69,8 +70,9 @@
positional = true,
named = false,
doc = "The runfiles object to merge into this."),
- })
- RunfilesApi merge(RunfilesApi other);
+ },
+ useStarlarkThread = true)
+ RunfilesApi merge(RunfilesApi other, StarlarkThread thread) throws EvalException;
@StarlarkMethod(
name = "merge_all",
@@ -86,6 +88,7 @@
positional = true,
named = false,
doc = "The sequence of runfiles objects to merge into this."),
- })
- RunfilesApi mergeAll(Sequence<?> sequence) throws EvalException;
+ },
+ useStarlarkThread = true)
+ RunfilesApi mergeAll(Sequence<?> sequence, StarlarkThread thread) throws EvalException;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
index 555f696..e031705 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
@@ -118,6 +118,7 @@
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:bin_tools",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/rules/android",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
index 69d6620..840a16d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
@@ -15,6 +15,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -28,15 +29,21 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.events.EventKind;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.common.options.Options;
+import com.google.devtools.common.options.OptionsParsingException;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import javax.annotation.Nullable;
+import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Mutability;
import net.starlark.java.eval.StarlarkList;
+import net.starlark.java.eval.StarlarkThread;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -55,6 +62,13 @@
assertThat(Iterables.getOnlyElement(eventCollector).getKind()).isEqualTo(EventKind.WARNING);
}
+ private static StarlarkThread newStarlarkThread(String... options)
+ throws OptionsParsingException {
+ return new StarlarkThread(
+ Mutability.create("test"),
+ Options.parse(BuildLanguageOptions.class, options).getOptions().toStarlarkSemantics());
+ }
+
@Test
public void testFilterListForObscuringSymlinksCatchesBadObscurer() throws Exception {
Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
@@ -463,7 +477,7 @@
}
@Test
- public void testMergeWithSymlinks() {
+ public void testMergeWithSymlinks() throws Exception {
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Artifact artifactA = ActionsTestUtil.createArtifact(root, "a/target");
Artifact artifactB = ActionsTestUtil.createArtifact(root, "b/target");
@@ -475,8 +489,9 @@
Runfiles runfilesB = new Runfiles.Builder("TESTING")
.addSymlink(sympathB, artifactB)
.build();
+ StarlarkThread thread = newStarlarkThread();
- Runfiles runfilesC = runfilesA.merge(runfilesB);
+ Runfiles runfilesC = runfilesA.merge(runfilesB, thread);
assertThat(runfilesC.getSymlinksAsMap(null).get(sympathA)).isEqualTo(artifactA);
assertThat(runfilesC.getSymlinksAsMap(null).get(sympathB)).isEqualTo(artifactB);
}
@@ -493,19 +508,23 @@
Runfiles runfilesA = new Runfiles.Builder("TESTING").addSymlink(sympathA, artifactA).build();
Runfiles runfilesB = new Runfiles.Builder("TESTING").addSymlink(sympathB, artifactB).build();
Runfiles runfilesC = new Runfiles.Builder("TESTING").addSymlink(sympathC, artifactC).build();
+ StarlarkThread thread = newStarlarkThread();
- Runfiles runfilesMerged = runfilesA.mergeAll(StarlarkList.immutableOf(runfilesB, runfilesC));
+ Runfiles runfilesMerged =
+ runfilesA.mergeAll(StarlarkList.immutableOf(runfilesB, runfilesC), thread);
assertThat(runfilesMerged.getSymlinksAsMap(null))
.containsExactly(sympathA, artifactA, sympathB, artifactB, sympathC, artifactC);
}
@Test
- public void testMergeEmptyWithNonEmpty() {
+ public void testMergeEmptyWithNonEmpty() throws Exception {
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Artifact artifactA = ActionsTestUtil.createArtifact(root, "a/target");
Runfiles runfilesB = new Runfiles.Builder("TESTING").addArtifact(artifactA).build();
- assertThat(Runfiles.EMPTY.merge(runfilesB)).isSameInstanceAs(runfilesB);
- assertThat(runfilesB.merge(Runfiles.EMPTY)).isSameInstanceAs(runfilesB);
+ StarlarkThread thread = newStarlarkThread();
+
+ assertThat(Runfiles.EMPTY.merge(runfilesB, thread)).isSameInstanceAs(runfilesB);
+ assertThat(runfilesB.merge(Runfiles.EMPTY, thread)).isSameInstanceAs(runfilesB);
}
@Test
@@ -513,27 +532,70 @@
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Artifact artifact = ActionsTestUtil.createArtifact(root, "target");
Runfiles nonEmpty = new Runfiles.Builder("TESTING").addArtifact(artifact).build();
+ StarlarkThread thread = newStarlarkThread();
- assertThat(Runfiles.EMPTY.mergeAll(StarlarkList.immutableOf(nonEmpty)))
+ assertThat(Runfiles.EMPTY.mergeAll(StarlarkList.immutableOf(nonEmpty), thread))
.isSameInstanceAs(nonEmpty);
assertThat(
Runfiles.EMPTY.mergeAll(
- StarlarkList.immutableOf(Runfiles.EMPTY, nonEmpty, Runfiles.EMPTY)))
+ StarlarkList.immutableOf(Runfiles.EMPTY, nonEmpty, Runfiles.EMPTY), thread))
.isSameInstanceAs(nonEmpty);
- assertThat(nonEmpty.mergeAll(StarlarkList.immutableOf(Runfiles.EMPTY, Runfiles.EMPTY)))
+ assertThat(nonEmpty.mergeAll(StarlarkList.immutableOf(Runfiles.EMPTY, Runfiles.EMPTY), thread))
.isSameInstanceAs(nonEmpty);
- assertThat(nonEmpty.mergeAll(StarlarkList.immutableOf())).isSameInstanceAs(nonEmpty);
+ assertThat(nonEmpty.mergeAll(StarlarkList.immutableOf(), thread)).isSameInstanceAs(nonEmpty);
}
@Test
public void mergeAll_emptyWithEmpty() throws Exception {
- assertThat(Runfiles.EMPTY.mergeAll(StarlarkList.immutableOf()))
+ StarlarkThread thread = newStarlarkThread();
+ assertThat(Runfiles.EMPTY.mergeAll(StarlarkList.immutableOf(), thread))
.isSameInstanceAs(Runfiles.EMPTY);
- assertThat(Runfiles.EMPTY.mergeAll(StarlarkList.immutableOf(Runfiles.EMPTY, Runfiles.EMPTY)))
+ assertThat(
+ Runfiles.EMPTY.mergeAll(
+ StarlarkList.immutableOf(Runfiles.EMPTY, Runfiles.EMPTY), thread))
.isSameInstanceAs(Runfiles.EMPTY);
}
@Test
+ public void merge_exceedsDepthLimit_throwsException() throws Exception {
+ ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
+ Artifact artifactA = ActionsTestUtil.createArtifact(root, "a/target");
+ Artifact artifactB = ActionsTestUtil.createArtifact(root, "b/target");
+ Artifact artifactC = ActionsTestUtil.createArtifact(root, "c/target");
+ Runfiles runfilesA = new Runfiles.Builder("TESTING").addArtifact(artifactA).build();
+ Runfiles runfilesB = new Runfiles.Builder("TESTING").addArtifact(artifactB).build();
+ Runfiles runfilesC = new Runfiles.Builder("TESTING").addArtifact(artifactC).build();
+ StarlarkThread thread = newStarlarkThread("--nested_set_depth_limit=2");
+
+ Runfiles mergeAB = runfilesA.merge(runfilesB, thread);
+ EvalException expected =
+ assertThrows(EvalException.class, () -> mergeAB.merge(runfilesC, thread));
+ assertThat(expected).hasMessageThat().contains("artifacts depset depth 3 exceeds limit (2)");
+ }
+
+ @Test
+ public void mergeAll_exceedsDepthLimit_throwsException() throws Exception {
+ ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
+ Artifact artifactA = ActionsTestUtil.createArtifact(root, "a/target");
+ Artifact artifactB = ActionsTestUtil.createArtifact(root, "b/target");
+ Artifact artifactC = ActionsTestUtil.createArtifact(root, "c/target");
+ PathFragment sympathA = PathFragment.create("a/symlink");
+ PathFragment sympathB = PathFragment.create("b/symlink");
+ PathFragment sympathC = PathFragment.create("c/symlink");
+ Runfiles runfilesA = new Runfiles.Builder("TESTING").addSymlink(sympathA, artifactA).build();
+ Runfiles runfilesB = new Runfiles.Builder("TESTING").addSymlink(sympathB, artifactB).build();
+ Runfiles runfilesC = new Runfiles.Builder("TESTING").addSymlink(sympathC, artifactC).build();
+ StarlarkThread thread = newStarlarkThread("--nested_set_depth_limit=2");
+
+ Runfiles mergeAllAB = runfilesA.mergeAll(StarlarkList.immutableOf(runfilesB), thread);
+ EvalException expected =
+ assertThrows(
+ EvalException.class,
+ () -> mergeAllAB.mergeAll(StarlarkList.immutableOf(runfilesC), thread));
+ assertThat(expected).hasMessageThat().contains("symlinks depset depth 3 exceeds limit (2)");
+ }
+
+ @Test
public void testOnlyExtraMiddlemenNotConsideredEmpty() {
ArtifactRoot root =
ArtifactRoot.asDerivedRoot(
diff --git a/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java b/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java
index b39db0a..92637fe 100644
--- a/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/collect/nestedset/DepsetTest.java
@@ -461,8 +461,7 @@
.testEval("create_depset(3000)", "None") // succeeds
.testIfErrorContains("depset depth 3501 exceeds limit (3500)", "create_depset(4000)");
- Depset.setDepthLimit(100);
- ev.new Scenario()
+ ev.new Scenario("--nested_set_depth_limit=100")
.setUp(
"def create_depset(depth):",
" x = depset([0])",
