Delete the flag --incompatible_depset_union
https://github.com/bazelbuild/bazel/issues/5817
RELNOTES: The flag `--incompatible_depset_union` is removed.
PiperOrigin-RevId: 303325924
diff --git a/site/docs/skylark/depsets.md b/site/docs/skylark/depsets.md
index 2d670b6..c29fbbe 100644
--- a/site/docs/skylark/depsets.md
+++ b/site/docs/skylark/depsets.md
@@ -285,13 +285,6 @@
there are no guarantees about the order of any of its elements (except that it
is deterministic).
-For safety, depsets with different orders cannot be merged with the `+` operator
-unless one of them uses the default order; the resulting depset’s order is the
-same as the left operand. Note that when two depsets of different order are
-merged in this way, the child may appear to have had its elements rearranged
-when it is traversed via the parent. **The `+` operator is deprecated, anyway;
-use the `transitive` argument instead.**
-
## Performance
To see the motivation for using depsets, consider what would have happened if we
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 23e1e10..1c7b062 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -291,20 +291,6 @@
public boolean experimentalAllowTagsPropagation;
@Option(
- name = "incompatible_depset_union",
- defaultValue = "true",
- documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
- effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
- metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
- },
- help =
- "If set to true, depset union using `+`, `|` or `.union` are forbidden. "
- + "Use the `depset` constructor instead.")
- public boolean incompatibleDepsetUnion;
-
- @Option(
name = "incompatible_always_check_depset_elements",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -676,7 +662,6 @@
.experimentalSiblingRepositoryLayout(experimentalSiblingRepositoryLayout)
.experimentalExecGroups(experimentalExecGroups)
.incompatibleApplicableLicenses(incompatibleApplicableLicenses)
- .incompatibleDepsetUnion(incompatibleDepsetUnion)
.incompatibleDisableTargetProviderFields(incompatibleDisableTargetProviderFields)
.incompatibleDisableThirdPartyLicenseChecking(
incompatibleDisableThirdPartyLicenseChecking)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Depset.java b/src/main/java/com/google/devtools/build/lib/syntax/Depset.java
index 2987d72..8f89700 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Depset.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Depset.java
@@ -59,9 +59,7 @@
+ " simulate one using a dictionary where all keys map to <code>True</code>."
+ "<p>Depsets are immutable. They should be created using their <a"
+ " href=\"globals.html#depset\">constructor function</a> and merged or augmented with"
- + " other depsets via the <code>transitive</code> argument. There are other deprecated"
- + " methods (<code>|</code> and <code>+</code> operators)"
- + " that will eventually go away."
+ + " other depsets via the <code>transitive</code> argument. "
+ "<p>The <code>order</code> parameter determines the"
+ " kind of traversal that is done to convert the depset to an iterable. There are"
+ " four possible values:"
@@ -92,7 +90,7 @@
public final class Depset implements StarlarkValue {
private final SkylarkType contentType;
private final NestedSet<?> set;
- @Nullable private final ImmutableList<Object> items;
+ @Nullable private final ImmutableList<Object> items; // TODO(laurentlb): Delete field.
@Nullable private final ImmutableList<NestedSet<?>> transitiveItems;
@AutoCodec.VisibleForSerialization
@@ -107,6 +105,7 @@
this.transitiveItems = transitiveItems;
}
+ // TODO(laurentlb): Remove the left argument once `unionOf` is deleted.
private static Depset create(
Order order, SkylarkType contentType, Object item, @Nullable Depset left)
throws EvalException {
@@ -191,7 +190,7 @@
return create(order, SkylarkType.TOP, items, null);
}
- // implementation of deprecated depset+x, depset|x
+ // TODO(laurentlb): Delete the method. It's used only in tests.
static Depset unionOf(Depset left, Object right) throws EvalException {
return create(left.set.getOrder(), left.contentType, right, left);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index d9ee4ae..0fca831 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -405,17 +405,6 @@
return StarlarkList.concat((StarlarkList<?>) x, (StarlarkList<?>) y, mu);
}
- } else if (x instanceof Depset) {
- // depset + any
- // TODO(bazel-team): Remove deprecated operator.
- if (semantics.incompatibleDepsetUnion()) {
- throw Starlark.errorf(
- "`+` operator on a depset is forbidden. See "
- + "https://docs.bazel.build/versions/master/skylark/depsets.html for "
- + "recommendations. Use --incompatible_depset_union=false "
- + "to temporarily disable this check.");
- }
- return Depset.unionOf((Depset) x, y);
}
break;
@@ -425,16 +414,6 @@
// int | int
return ((Integer) x) | (Integer) y;
}
- } else if (x instanceof Depset) {
- // depset | any
- if (semantics.incompatibleDepsetUnion()) {
- throw Starlark.errorf(
- "`|` operator on a depset is forbidden. See "
- + "https://docs.bazel.build/versions/master/skylark/depsets.html for "
- + "recommendations. Use --incompatible_depset_union=false "
- + "to temporarily disable this check.");
- }
- return Depset.unionOf((Depset) x, y);
}
break;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index c1a375e..60ec41a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -243,8 +243,6 @@
public abstract boolean incompatibleApplicableLicenses();
- public abstract boolean incompatibleDepsetUnion();
-
public abstract boolean incompatibleDisableTargetProviderFields();
public abstract boolean incompatibleDisableThirdPartyLicenseChecking();
@@ -346,7 +344,6 @@
.experimentalExecGroups(false)
.incompatibleAlwaysCheckDepsetElements(true)
.incompatibleApplicableLicenses(false)
- .incompatibleDepsetUnion(true)
.incompatibleDisableTargetProviderFields(false)
.incompatibleDisableThirdPartyLicenseChecking(true)
.incompatibleDisableDeprecatedAttrParams(true)
@@ -417,8 +414,6 @@
public abstract Builder incompatibleApplicableLicenses(boolean value);
- public abstract Builder incompatibleDepsetUnion(boolean value);
-
public abstract Builder incompatibleDisableTargetProviderFields(boolean value);
public abstract Builder incompatibleDisableThirdPartyLicenseChecking(boolean value);
diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
index c1f9f94..d56cc82 100644
--- a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
+++ b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
@@ -190,7 +190,6 @@
.build();
parser.parseAndExitUponError(args);
StarlarkSemanticsOptions semanticsOptions = parser.getOptions(StarlarkSemanticsOptions.class);
- semanticsOptions.incompatibleDepsetUnion = false;
semanticsOptions.incompatibleDisableDeprecatedAttrParams = false;
semanticsOptions.incompatibleNewActionsApi = false;
SkydocOptions skydocOptions = parser.getOptions(SkydocOptions.class);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 7011d0927..84266d2 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -143,7 +143,6 @@
"--incompatible_always_check_depset_elements=" + rand.nextBoolean(),
"--incompatible_applicable_licenses=" + rand.nextBoolean(),
"--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
- "--incompatible_depset_union=" + rand.nextBoolean(),
"--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
"--incompatible_disable_deprecated_attr_params=" + rand.nextBoolean(),
"--incompatible_disable_depset_items=" + rand.nextBoolean(),
@@ -197,7 +196,6 @@
.incompatibleAlwaysCheckDepsetElements(rand.nextBoolean())
.incompatibleApplicableLicenses(rand.nextBoolean())
.incompatibleDepsetForLibrariesToLinkGetter(rand.nextBoolean())
- .incompatibleDepsetUnion(rand.nextBoolean())
.incompatibleDisableTargetProviderFields(rand.nextBoolean())
.incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())
.incompatibleDisableDepsetItems(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
index c9e633c..26e3a7f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/DepsetTest.java
@@ -275,11 +275,11 @@
@Test
public void testIncompatibleUnion() throws Exception {
- new Scenario("--incompatible_depset_union=true")
- .testIfErrorContains("`+` operator on a depset is forbidden", "depset([]) + ['a']");
+ new Scenario()
+ .testIfErrorContains("unsupported binary operation: depset + list", "depset([]) + ['a']");
- new Scenario("--incompatible_depset_union=true")
- .testIfErrorContains("`|` operator on a depset is forbidden", "depset([]) | ['a']");
+ new Scenario()
+ .testIfErrorContains("unsupported binary operation: depset | list", "depset([]) | ['a']");
}
private void assertContainsInOrder(String statement, Object... expectedElements)
@@ -290,127 +290,18 @@
}
@Test
- public void testUnionOrder() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func():",
- " s1 = depset()",
- " s2 = depset()",
- " s1 += ['a']",
- " s2 += ['b']",
- " s1 += s2",
- " return s1",
- "s = func()");
- assertThat(get("s").toCollection()).containsExactly("b", "a").inOrder();
- }
-
- @Test
- public void testUnionIncompatibleOrder() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- checkEvalError(
- "Order mismatch: topological != postorder",
- "depset(['a', 'b'], order='postorder') + depset(['c', 'd'], order='topological')");
- }
-
- @Test
- public void testFunctionReturnsDepset() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func():", //
- " t = depset()",
- " t += ['a']",
- " return t",
- "s = func()");
- assertThat(get("s")).isInstanceOf(Depset.class);
- assertThat(get("s").toCollection()).containsExactly("a");
- }
-
- @Test
- public void testPlusEqualsWithList() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func():", //
- " t = depset()",
- " t += ['a', 'b']",
- " return t",
- "s = func()");
- assertThat(get("s").toCollection()).containsExactly("a", "b").inOrder();
- }
-
- @Test
- public void testPlusEqualsNoSideEffects() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func():",
- " s1 = depset()",
- " s1 += ['a']",
- " s2 = s1",
- " s2 += ['b']",
- " return s1",
- "s = func()");
- assertThat(get("s").toCollection()).containsExactly("a");
- }
-
- @Test
- public void testFuncParamNoSideEffects() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func1(t):",
- " t += ['b']",
- "def func2():",
- " u = depset()",
- " u += ['a']",
- " func1(u)",
- " return u",
- "s = func2()");
- assertThat(get("s").toCollection()).containsExactly("a");
- }
-
- @Test
- public void testTransitiveOrdering() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func():",
- " sa = depset(['a'], order='postorder')",
- " sb = depset(['b'], order='postorder')",
- " sc = depset(['c'], order='postorder') + sa",
- " return depset() + sb + sc",
- "s = func()");
- // The iterator lists the Transitive sets first
- assertThat(get("s").toCollection()).containsExactly("b", "a", "c").inOrder();
- }
-
- @Test
- public void testLeftRightDirectOrdering() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "def func():",
- " t = depset()",
- " t += [4]",
- " t += [2, 4]",
- " t += [3, 4, 5]",
- " return t",
- "s = func()");
- // All elements are direct. The iterator lists them left-to-right.
- assertThat(get("s").toCollection()).containsExactly(4, 2, 3, 5).inOrder();
- }
-
- @Test
public void testToString() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "s = depset() + [2, 4, 6] + [3, 4, 5]", //
- "x = str(s)");
+ exec("s = depset([3, 4, 5], transitive = [depset([2, 4, 6])])", "x = str(s)");
assertThat(lookup("x")).isEqualTo("depset([2, 4, 6, 3, 5])");
}
@Test
public void testToStringWithOrder() throws Exception {
- setSemantics("--incompatible_depset_union=false");
exec(
- "s = depset(order = 'topological') + [2, 4, 6] + [3, 4, 5]", //
+ "s = depset([3, 4, 5], transitive = [depset([2, 4, 6])], ",
+ " order = 'topological')",
"x = str(s)");
- assertThat(lookup("x")).isEqualTo("depset([2, 4, 6, 3, 5], order = \"topological\")");
+ assertThat(lookup("x")).isEqualTo("depset([3, 5, 6, 4, 2], order = \"topological\")");
}
private Depset get(String varname) throws Exception {
@@ -419,10 +310,8 @@
@Test
public void testToList() throws Exception {
- setSemantics("--incompatible_depset_union=false");
- exec(
- "s = depset() + [2, 4, 6] + [3, 4, 5]", //
- "x = s.to_list()");
+ setSemantics();
+ exec("s = depset([3, 4, 5], transitive = [depset([2, 4, 6])])", "x = s.to_list()");
Object value = lookup("x");
assertThat(value).isInstanceOf(StarlarkList.class);
assertThat((Iterable<?>) value).containsExactly(2, 4, 6, 3, 5).inOrder();
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index e53dcea..c17c206 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -1449,14 +1449,6 @@
}
@Test
- public void testUnionSet() throws Exception {
- new Scenario("--incompatible_depset_union=false")
- .testExpression("str(depset([1, 3]) | depset([1, 2]))", "depset([1, 2, 3])")
- .testExpression("str(depset([1, 2]) | [1, 3])", "depset([1, 2, 3])")
- .testIfExactError("unsupported binary operation: int | bool", "2 | False");
- }
-
- @Test
public void testSetIsNotIterable() throws Exception {
new Scenario()
.testIfErrorContains("not iterable", "list(depset(['a', 'b']))")
