Allow native.existing_rule and native.existing_rules to return a lightweight view.
Experimental implementation of
https://github.com/bazelbuild/proposals/blob/main/designs/2021-06-15-improving-native.existing_rules.md
Gated behind the --experimental_existing_rules_immutable_view flag. See
https://github.com/bazelbuild/bazel/issues/13907
To check the performance impact, I queried a toy package that in a loop, creates
a genrule and then on each loop iteration performs one of the following tasks:
* checks if rule with a given name existed in `existing_rules()` ("in")
* for each rule in `existing_rules()`, checks if an attribute with a given name exists ("shallow iteration")
* for 50% of rules in `existing_rules()`, checks if a particular attribute value exists under any name ("50% deep iteration");
* iterates every rule name, attribute name, and attribute value ("deep iteration");
Query time in seconds without --experimental_existing_rules_immutable_view:
```
n in shallow 50%deep deep
1000 2.73 2.84 3.36 3.61
2000 8.42 9.07 10.97 11.62
4000 30.95 33.81 40.88 44.17
```
With --experimental_existing_rules_immutable_view:
```
n in shallow 50%deep deep
1000 0.40 0.64 2.58 4.19
2000 0.43 1.08 7.96 13.64
4000 0.51 2.59 28.83 52.99
```
In other words, in the unusual case where we examine every attribute value
of every existing rule, the overhead of managing immutable views would cause
a 15-20% slowdown. In any other situation, where we don't need to materialize
every rule's attribute values, the immutable view approach yields a
substantial performance improvement.
We'd want to see if we can reduce the 15-20% worst-case penalty in followup
work.
RELNOTES: Adds --experimental_existing_rules_immutable_view flag to make the
native.existing_rule and native.existing_rules functions more efficient by
returning immutable, lightweight dict-like view objects instead of mutable
dicts.
PiperOrigin-RevId: 397805096
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 6f919b5..77b7298 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -19,7 +19,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.BiMap;
-import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -978,7 +977,10 @@
private License defaultLicense = License.NO_LICENSE;
private Set<License.DistributionType> defaultDistributionSet = License.DEFAULT_DISTRIB;
- private BiMap<String, Target> targets = HashBiMap.create();
+ // All targets added to the package. We use SnapshottableBiMap to help track insertion order of
+ // Rule targets, for use by native.existing_rules().
+ private BiMap<String, Target> targets =
+ new SnapshottableBiMap<>(target -> target instanceof Rule);
private final Map<Label, EnvironmentGroup> environmentGroups = new HashMap<>();
/**
@@ -1479,14 +1481,22 @@
}
/**
- * Removes a target from the {@link Package} under construction. Intended to be used only by
- * {@link com.google.devtools.build.lib.skyframe.PackageFunction} to remove targets whose labels
- * cross subpackage boundaries.
+ * Replaces a target in the {@link Package} under construction with a new target with the same
+ * name and belonging to the same package.
+ *
+ * <p>A hack needed for {@link WorkspaceFactoryHelper}.
*/
- void removeTarget(Target target) {
- if (target.getPackage() == pkg) {
- this.targets.remove(target.getName());
- }
+ void replaceTarget(Target newTarget) {
+ Preconditions.checkArgument(
+ targets.containsKey(newTarget.getName()),
+ "No existing target with name '%s' in the targets map",
+ newTarget.getName());
+ Preconditions.checkArgument(
+ newTarget.getPackage() == pkg, // pointer comparison since we're constructing `pkg`
+ "Replacement target belongs to package '%s', expected '%s'",
+ newTarget.getPackage(),
+ pkg);
+ targets.put(newTarget.getName(), newTarget);
}
public Set<Target> getTargets() {
@@ -1494,6 +1504,24 @@
}
/**
+ * Returns a lightweight snapshot view of the names of all rule targets belonging to this
+ * package at the time of this call.
+ *
+ * @throws IllegalStateException if this method is called after {@link beforeBuild} has been
+ * called.
+ */
+ Map<String, Rule> getRulesSnapshotView() {
+ if (targets instanceof SnapshottableBiMap<?, ?>) {
+ return Maps.transformValues(
+ ((SnapshottableBiMap<String, Target>) targets).getTrackedSnapshot(),
+ target -> (Rule) target);
+ } else {
+ throw new IllegalStateException(
+ "getRulesSnapshotView() cannot be used after beforeBuild() has been called");
+ }
+ }
+
+ /**
* Returns an (immutable, unordered) view of all the targets belonging to this package which are
* instances of the specified class.
*/
@@ -1556,8 +1584,10 @@
if (!((InputFile) cacheInstance).isVisibilitySpecified()
|| cacheInstance.getVisibility() != visibility
|| !Objects.equals(cacheInstance.getLicense(), license)) {
- targets.put(filename, new InputFile(
- pkg, cacheInstance.getLabel(), cacheInstance.getLocation(), visibility, license));
+ targets.put(
+ filename,
+ new InputFile(
+ pkg, cacheInstance.getLabel(), cacheInstance.getLocation(), visibility, license));
}
}
@@ -1718,6 +1748,17 @@
getPackageIdentifier(), ioExceptionMessage, ioException, ioExceptionDetailedExitCode);
}
+ // SnapshottableBiMap does not allow removing targets (in order to efficiently track rule
+ // insertion order). However, we *do* need to support removal of targets in
+ // PackageFunction.handleLabelsCrossingSubpackagesAndPropagateInconsistentFilesystemExceptions
+ // which is called *between* calls to beforeBuild and finishBuild. We thus repoint the targets
+ // map to the SnapshottableBiMap's underlying bimap and thus stop tracking insertion order.
+ // After this point, snapshots of targets should no longer be used, and any further
+ // getRulesSnapshotView calls will throw.
+ if (targets instanceof SnapshottableBiMap<?, ?>) {
+ targets = ((SnapshottableBiMap<String, Target>) targets).getUnderlyingBiMap();
+ }
+
// We create the original BUILD InputFile when the package filename is set; however, the
// visibility may be overridden with an exports_files directive, so we need to obtain the
// current instance here.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SnapshottableBiMap.java b/src/main/java/com/google/devtools/build/lib/packages/SnapshottableBiMap.java
new file mode 100644
index 0000000..22f4027
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/packages/SnapshottableBiMap.java
@@ -0,0 +1,499 @@
+// Copyright 2021 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.packages;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.Maps;
+import com.google.common.collect.UnmodifiableIterator;
+import java.util.AbstractMap;
+import java.util.AbstractSet;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Set;
+import java.util.function.Predicate;
+
+/**
+ * A bimap with the following features and restrictions:
+ *
+ * <ul>
+ * <li>it (lazily) tracks the order in which keys were inserted;
+ * <li>... but only for entries whose values satisfy a predicate;
+ * <li>it's append-only, i.e. it supports addition of new key-value pairs, or replacement of the
+ * value of an existing key, but not deletion of key-value pairs;
+ * <li>... with the restriction that replacement is not allowed to make a previously tracked entry
+ * become untracked.
+ * </ul>
+ *
+ * <p>Tracking the insertion order and prohibiting key deletion allows this bimap to provide a
+ * lightweight snapshot view for iterating (in key insertion order) over entries which existed at a
+ * given point in time.
+ *
+ * <p>Intended to be used by {@code native.existing_rules} in Starlark, which needs to be able to
+ * iterate, at some later point in time, over the rules which existed in a {@link Package.Builder}
+ * at the time of the {@code existing_rules} call. We do not want to track insertion orders of
+ * numerous non-rule targets (e.g. files) - hence, filtering by a predicate. And in the common case
+ * where {@code existing_rules} is never called in a package, we want to avoid the overhead of
+ * keeping track of insertion orders - hence, laziness.
+ *
+ * <p>In packages with a large number of targets, the use of lightweight snapshots instead of
+ * copying results in a noticeable improvement in loading times, e.g. 2.2 times faster loading for a
+ * package with 4400 targets and 300 {@code native.existing_rules} calls.
+ */
+final class SnapshottableBiMap<K, V> implements BiMap<K, V> {
+ private final BiMap<K, V> contents = HashBiMap.create();
+ private final Predicate<V> track;
+
+ // trackedKeys and trackedKeyOrders are initialized lazily by ensureOrderTracking(). In the case
+ // where the order-tracking map represents a package builder's targets, ensureOrderTracking() is
+ // intended to be triggered only by a call to {@code native.existing_rules} in Starlark.
+ //
+ // Holds all keys being tracked, in their relative insertion order.
+ private ArrayList<K> trackedKeys;
+ // Maps all keys being tracked to their index in trackedKeys.
+ private Map<K, Integer> trackedKeyOrders;
+
+ public SnapshottableBiMap(Predicate<V> track) {
+ this.track = track;
+ }
+
+ /**
+ * Returns the underlying contents bimap.
+ *
+ * <p>Mutating the underlying bimap will violate the guarantees of this class and possibly cause
+ * inconsistent behavior in snapshot views. Therefore, the recommended usage pattern is to replace
+ * any references to the {@code SnapshottableBiMap} with the underlying map, and ensure that any
+ * snapshots of the map are no longer in use at that point.
+ *
+ * <p>An optimization hack intended only for use from {@link Package.Builder#beforeBuild}.
+ */
+ BiMap<K, V> getUnderlyingBiMap() {
+ return contents;
+ }
+
+ @Override
+ public int size() {
+ return contents.size();
+ }
+
+ private int sizeTracked() {
+ ensureOrderTracking();
+ return trackedKeyOrders.size();
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return contents.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return contents.containsKey(key);
+ }
+
+ @Override
+ public boolean containsValue(Object value) {
+ return contents.containsValue(value);
+ }
+
+ @Override
+ public V get(Object key) {
+ return contents.get(key);
+ }
+
+ /**
+ * Returns the insertion order of the specified key (relative to other tracked keys), or -1 if the
+ * key was never inserted into the map or corresponds to a key-value pair whose insertion order we
+ * do not track. Replacing a key's value does not change this order if tracking has already begun.
+ */
+ private int getTrackedKeyOrder(Object key) {
+ ensureOrderTracking();
+ Integer order = trackedKeyOrders.get(key);
+ return order == null ? -1 : order;
+ }
+
+ /**
+ * Returns the tracked key with the specified insertion order (as determined by {@link
+ * #getTrackedKeyOrder}).
+ *
+ * @throws IndexOutOfBoundsException if the specified insertion order is out of bounds
+ */
+ private K getTrackedKey(int order) {
+ ensureOrderTracking();
+ return trackedKeys.get(order);
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Note that once key insertion order tracking has started, overriding a key with a different
+ * value will not change the key's insertion order.
+ *
+ * @throws IllegalArgumentException if attempting to replace a key-value pair whose insertion
+ * order was tracked with a key-value pair whose insertion order is not tracked, or if the
+ * given value is already bound to a different key in this map.
+ */
+ @Override
+ public V put(K key, V value) {
+ if (startedOrderTracking()) {
+ boolean oldWasTracked = getTrackedKeyOrder(key) >= 0;
+ boolean newIsTracked = track.test(value);
+ if (oldWasTracked) {
+ Preconditions.checkArgument(
+ newIsTracked,
+ "Cannot replace a key-value pair which is tracked with a key-value pair which is"
+ + " not tracked");
+ } else {
+ if (newIsTracked) {
+ recordKeyOrder(key);
+ }
+ }
+ }
+ return contents.put(key, value);
+ }
+
+ /**
+ * @deprecated Not supported, since it's morally equivalent to preceding a {@link #put} call with
+ * a silent {@code this.values().remove(value)}.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public V forcePut(K key, V value) {
+ throw new UnsupportedOperationException("Append-only data structure");
+ }
+
+ /**
+ * @deprecated Not supported.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public V remove(Object key) {
+ throw new UnsupportedOperationException("Append-only data structure");
+ }
+
+ @Override
+ public void putAll(Map<? extends K, ? extends V> map) {
+ for (Map.Entry<? extends K, ? extends V> entry : map.entrySet()) {
+ put(entry.getKey(), entry.getValue());
+ }
+ }
+
+ /**
+ * @deprecated Not supported.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public void clear() {
+ throw new UnsupportedOperationException("Append-only data structure");
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Removing a key from the set does not change the key's order if it was tracked prior to
+ * removal. Removal is supported only for consistency with {@link values}.
+ */
+ @Override
+ public Set<K> keySet() {
+ return Collections.unmodifiableSet(contents.keySet());
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Removing a value from the set does not change the key's order if it was tracked prior to
+ * removal. Ideally, we would not want to support removal, but it is required for {@link
+ * PackageFunction#handleLabelsCrossingSubpackagesAndPropagateInconsistentFilesystemExceptions}.
+ */
+ @Override
+ public Set<V> values() {
+ return Collections.unmodifiableSet(contents.values());
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Removing an entry from the set does not change the key's order if it was tracked prior to
+ * removal. Removal is supported only for consistency with {@link values}.
+ */
+ @Override
+ public Set<Map.Entry<K, V>> entrySet() {
+ return Collections.unmodifiableSet(contents.entrySet());
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * <p>The returned map is unmodifiable (all modifications will throw an {@link
+ * UnsupportedOperationException}.
+ */
+ @Override
+ public BiMap<V, K> inverse() {
+ return Maps.unmodifiableBiMap(contents.inverse());
+ }
+
+ private boolean startedOrderTracking() {
+ Preconditions.checkState((trackedKeys == null) == (trackedKeyOrders == null));
+ return trackedKeys != null;
+ }
+
+ private void ensureOrderTracking() {
+ if (!startedOrderTracking()) {
+ trackedKeys = new ArrayList<>();
+ trackedKeyOrders = new HashMap<>();
+
+ contents.forEach(
+ (key, value) -> {
+ if (track.test(value)) {
+ recordKeyOrder(key);
+ }
+ });
+ }
+ }
+
+ private void recordKeyOrder(K key) {
+ int order = trackedKeys.size();
+ trackedKeys.add(key);
+ trackedKeyOrders.put(key, order);
+ }
+
+ /**
+ * Returns a lightweight snapshot view of the tracked entries existing in the bimap at the time
+ * this method is called.
+ *
+ * <p>Most method calls on the view returned by this method will start insertion order tracking if
+ * it has not been started already. In particular, that implies that after this method had been
+ * called, a value whose insertion order was tracked may no longer be replaceable with a value
+ * whose insertion order is not tracked. See {@link #put} for details.
+ */
+ public Map<K, V> getTrackedSnapshot() {
+ return new TrackedSnapshot<>(this);
+ }
+
+ /**
+ * A view of a {@link SnapshottableBiMap}'s contents existing at a certain point in time.
+ *
+ * <p>Iterators over the view's {@link #keySet}, {@link #entrySet}, or {@link #values} iterate in
+ * key insertion order.
+ */
+ static final class TrackedSnapshot<K, V> extends AbstractMap<K, V> {
+ private final SnapshottableBiMap<K, V> underlying;
+ // The number of initial elements from `underlying`'s `trackedKeys` list that should be
+ // considered to be present in this view. Note that we don't snapshot values, so we'll use
+ // whatever the most recent value in `underlying` is even if it changed after this snapshot
+ // was created.
+ private final int sizeTracked;
+
+ private TrackedSnapshot(SnapshottableBiMap<K, V> underlying) {
+ this.underlying = underlying;
+ this.sizeTracked = underlying.sizeTracked();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ int order = underlying.getTrackedKeyOrder(key);
+ return order >= 0 && order < sizeTracked;
+ }
+
+ @Override
+ public boolean containsValue(Object value) {
+ Object key = underlying.inverse().get(value);
+ if (key != null) {
+ int order = underlying.getTrackedKeyOrder(key);
+ return order >= 0 && order < sizeTracked;
+ } else {
+ return false;
+ }
+ }
+
+ @Override
+ public V get(Object key) {
+ if (containsKey(key)) {
+ return underlying.get(key);
+ } else {
+ return null;
+ }
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public V put(K key, V value) {
+ throw new UnsupportedOperationException("Read-only snapshot");
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public V remove(Object key) {
+ throw new UnsupportedOperationException("Read-only snapshot");
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public void putAll(Map<? extends K, ? extends V> m) {
+ throw new UnsupportedOperationException("Read-only snapshot");
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public void clear() {
+ throw new UnsupportedOperationException("Read-only snapshot");
+ }
+
+ @Override
+ public Set<Map.Entry<K, V>> entrySet() {
+ return new UnmodifiableSet<Map.Entry<K, V>>() {
+ @Override
+ public int size() {
+ return sizeTracked;
+ }
+
+ @Override
+ public boolean isEmpty() {
+ return sizeTracked == 0;
+ }
+
+ @Override
+ public boolean contains(Object object) {
+ if (!(object instanceof Map.Entry<?, ?>)) {
+ return false;
+ }
+ Map.Entry<?, ?> entry = (Map.Entry<?, ?>) object;
+ return TrackedSnapshot.this.containsKey(entry.getKey())
+ && TrackedSnapshot.this.containsValue(entry.getValue());
+ }
+
+ @Override
+ public Iterator<Map.Entry<K, V>> iterator() {
+ return new UnmodifiableIterator<Map.Entry<K, V>>() {
+ private int nextOrder = 0;
+
+ @Override
+ public boolean hasNext() {
+ return nextOrder < TrackedSnapshot.this.sizeTracked;
+ }
+
+ @Override
+ public Map.Entry<K, V> next() {
+ if (!hasNext()) {
+ throw new NoSuchElementException();
+ }
+ K key = TrackedSnapshot.this.underlying.getTrackedKey(nextOrder);
+ V value = TrackedSnapshot.this.underlying.get(key);
+ nextOrder++;
+ return new AbstractMap.SimpleEntry<>(key, value);
+ }
+ };
+ }
+ };
+ }
+
+ private abstract static class UnmodifiableSet<E> extends AbstractSet<E> {
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public boolean add(E entry) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public boolean remove(Object o) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * @deprecated Not implemented due to lack of need.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public boolean containsAll(Collection<?> c) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public boolean addAll(Collection<? extends E> c) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public boolean retainAll(Collection<?> c) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public boolean removeAll(Collection<?> c) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * @deprecated Unsupported operation.
+ * @throws UnsupportedOperationException always.
+ */
+ @Deprecated
+ @Override
+ public void clear() {
+ throw new UnsupportedOperationException();
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
index 438b6b2..709bf0b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java
@@ -15,10 +15,13 @@
package com.google.devtools.build.lib.packages;
import static com.google.devtools.build.lib.packages.PackageFactory.getContext;
+import static java.util.Comparator.naturalOrder;
import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterators;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -36,18 +39,24 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Comparator;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
+import net.starlark.java.annot.Param;
+import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Mutability;
import net.starlark.java.eval.NoneType;
+import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkIndexable;
import net.starlark.java.eval.StarlarkInt;
+import net.starlark.java.eval.StarlarkIterable;
import net.starlark.java.eval.StarlarkList;
+import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.StarlarkValue;
import net.starlark.java.eval.Tuple;
@@ -140,17 +149,234 @@
}
result.add(match);
}
- result.sort(Comparator.naturalOrder());
+ result.sort(naturalOrder());
return StarlarkList.copyOf(thread.mutability(), result);
}
+ // TODO(https://github.com/bazelbuild/bazel/issues/13605): implement StarlarkMapping (after we've
+ // added such an interface) to allow `dict(native.existing_rule(x))`.
+ private static interface DictLikeView extends StarlarkIndexable, StarlarkIterable<String> {
+ @Override
+ public default boolean isImmutable() {
+ return true;
+ }
+
+ @StarlarkMethod(
+ name = "get",
+ doc = "Behaves the same as <a href=\"dict.html#get\"><code>dict.get</code></a>.",
+ parameters = {
+ @Param(name = "key", doc = "The key to look for."),
+ @Param(
+ name = "default",
+ defaultValue = "None",
+ named = true,
+ doc = "The default value to use (instead of None) if the key is not found.")
+ },
+ allowReturnNones = true)
+ @Nullable // Java callers expect a null return when defaultValue is null
+ public Object get(Object key, @Nullable Object defaultValue) throws EvalException;
+
+ @StarlarkMethod(
+ name = "keys",
+ doc =
+ "Behaves like <a href=\"dict.html#keys\"><code>dict.keys</code></a>, but the returned"
+ + " value is an immutable sequence.")
+ public default StarlarkIterable<String> keys() {
+ // TODO(https://github.com/bazelbuild/starlark/issues/203): return a sequence view which
+ // supports efficient membership lookup (`"foo" in existing_rule("bar").keys()`), and
+ // materializes into a list (to allow len() or lookup by integer index) only if needed. Note
+ // that materialization into a list would need to be thread-safe (assuming it's possible for
+ // the sequence view to be used from multiple starlark threads). For now, we return an
+ // immutable list, so that migration to a sequence view is less likely to cause breakage.
+ return StarlarkList.immutableCopyOf(this);
+ }
+
+ @StarlarkMethod(
+ name = "values",
+ doc =
+ "Behaves like <a href=\"dict.html#values\"><code>dict.values</code></a>, but the"
+ + " returned value is an immutable sequence.")
+ public default StarlarkIterable<Object> values() throws EvalException {
+ // TODO(https://github.com/bazelbuild/starlark/issues/203): return a sequence view; see keys()
+ // for implementation concerns.
+ ArrayList<Object> valueList = new ArrayList<>();
+ for (String key : this) {
+ valueList.add(Preconditions.checkNotNull(get(key, null)));
+ }
+ return StarlarkList.immutableCopyOf(valueList);
+ }
+
+ @StarlarkMethod(
+ name = "items",
+ doc =
+ "Behaves like <a href=\"dict.html#items\"><code>dict.items</code></a>, but the returned"
+ + " value is an immutable sequence.")
+ public default StarlarkIterable<Tuple> items() throws EvalException {
+ // TODO(https://github.com/bazelbuild/starlark/issues/203): return a sequence view; see keys()
+ // for implementation concerns.
+ ArrayList<Tuple> itemsList = new ArrayList<>();
+ for (String key : this) {
+ itemsList.add(Tuple.pair(key, Preconditions.checkNotNull(get(key, null))));
+ }
+ return StarlarkList.immutableCopyOf(itemsList);
+ }
+ }
+
+ private static final class ExistingRuleView implements DictLikeView {
+ private final Rule rule;
+
+ ExistingRuleView(Rule rule) {
+ this.rule = rule;
+ }
+
+ @Override
+ public void repr(Printer printer) {
+ printer.append("<native.ExistingRuleView for target '").append(rule.getName()).append("'>");
+ }
+
+ @Override
+ @Nullable // Java callers expect a null return when defaultValue is null
+ public Object get(Object key, @Nullable Object defaultValue) throws EvalException {
+ if (!(key instanceof String)) {
+ return defaultValue;
+ }
+ String attributeName = (String) key;
+ switch (attributeName) {
+ case "name":
+ return rule.getName();
+ case "kind":
+ return rule.getRuleClass();
+ default:
+ if (!isPotentiallyExportableAttribute(rule.getRuleClassObject(), attributeName)) {
+ return defaultValue;
+ }
+ Object v =
+ starlarkifyValue(
+ null /* immutable */, rule.getAttr(attributeName), rule.getPackage());
+ if (v != null) {
+ return v;
+ }
+ }
+ return defaultValue;
+ }
+
+ @Override
+ public Iterator<String> iterator() {
+ return Iterators.concat(
+ ImmutableList.of("name", "kind").iterator(),
+ rule.getAttributes().stream()
+ .map(Attribute::getName)
+ .filter(
+ attributeName -> {
+ switch (attributeName) {
+ case "name":
+ case "kind":
+ // handled specially
+ return false;
+ default:
+ return isPotentiallyExportableAttribute(
+ rule.getRuleClassObject(), attributeName)
+ && isPotentiallyStarlarkifiableValue(rule.getAttr(attributeName));
+ }
+ })
+ .iterator());
+ }
+
+ @Override
+ public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+ Object val = get(key, null);
+ if (val != null) {
+ return val;
+ }
+ throw Starlark.errorf("key %s not found in view", Starlark.repr(key));
+ }
+
+ @Override
+ public boolean containsKey(StarlarkSemantics semantics, Object key) {
+ if (!(key instanceof String)) {
+ return false;
+ }
+ String attributeName = (String) key;
+ switch (attributeName) {
+ case "name":
+ case "kind":
+ return true;
+ default:
+ return isPotentiallyExportableAttribute(rule.getRuleClassObject(), attributeName)
+ && isPotentiallyStarlarkifiableValue(rule.getAttr(attributeName));
+ }
+ }
+ }
+
@Override
public Object existingRule(String name, StarlarkThread thread) throws EvalException {
BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("native.existing_rule");
PackageContext context = getContext(thread);
Target target = context.pkgBuilder.getTarget(name);
- return target instanceof Rule ? getRuleDict((Rule) target, thread.mutability()) : Starlark.NONE;
+ if (target instanceof Rule /* `instanceof` also verifies that target != null */) {
+ Rule rule = (Rule) target;
+ if (thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.EXPERIMENTAL_EXISTING_RULES_IMMUTABLE_VIEW)) {
+ return new ExistingRuleView(rule);
+ } else {
+ return getRuleDict(rule, thread.mutability());
+ }
+ } else {
+ return Starlark.NONE;
+ }
+ }
+
+ private static final class ExistingRulesView implements DictLikeView {
+ // We take a lightweight snapshot of the rules existing in a Package.Builder to avoid exposing
+ // any rules added to Package.Builder after the existing_rules() call which created this view.
+ private final Map<String, Rule> rulesSnapshotView;
+
+ ExistingRulesView(Map<String, Rule> rulesSnapshotView) {
+ this.rulesSnapshotView = rulesSnapshotView;
+ }
+
+ @Override
+ public void repr(Printer printer) {
+ printer.append("<native.ExistingRulesView object>");
+ }
+
+ @Override
+ @Nullable // Java callers expect a null return when defaultValue is null
+ public Object get(Object key, @Nullable Object defaultValue) {
+ if (!(key instanceof String)) {
+ return defaultValue;
+ }
+ Rule rule = rulesSnapshotView.get(key);
+ if (rule != null) {
+ return new ExistingRuleView(rule);
+ } else {
+ return defaultValue;
+ }
+ }
+
+ @Override
+ public Iterator<String> iterator() {
+ return rulesSnapshotView.keySet().iterator();
+ }
+
+ @Override
+ public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+ Object val = get(key, null);
+ if (val != null) {
+ return val;
+ }
+ throw Starlark.errorf("key %s not found in view", Starlark.repr(key));
+ }
+
+ @Override
+ public boolean containsKey(StarlarkSemantics semantics, Object key) {
+ if (!(key instanceof String)) {
+ return false;
+ }
+ return rulesSnapshotView.containsKey(key);
+ }
}
/*
@@ -158,27 +384,29 @@
For now, we ignore this, since users can implement it in Starlark.
*/
@Override
- public Dict<String, Dict<String, Object>> existingRules(StarlarkThread thread)
- throws EvalException {
+ public Object existingRules(StarlarkThread thread) throws EvalException {
BazelStarlarkContext.from(thread).checkLoadingOrWorkspacePhase("native.existing_rules");
PackageContext context = getContext(thread);
- Collection<Target> targets = context.pkgBuilder.getTargets();
- Mutability mu = thread.mutability();
- Dict.Builder<String, Dict<String, Object>> rules = Dict.builder();
- for (Target t : targets) {
- if (t instanceof Rule) {
- rules.put(t.getName(), getRuleDict((Rule) t, mu));
+ if (thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.EXPERIMENTAL_EXISTING_RULES_IMMUTABLE_VIEW)) {
+ return new ExistingRulesView(context.pkgBuilder.getRulesSnapshotView());
+ } else {
+ Collection<Target> targets = context.pkgBuilder.getTargets();
+ Mutability mu = thread.mutability();
+ Dict.Builder<String, Dict<String, Object>> rules = Dict.builder();
+ for (Target t : targets) {
+ if (t instanceof Rule) {
+ rules.put(t.getName(), getRuleDict((Rule) t, mu));
+ }
}
+ return rules.build(mu);
}
- return rules.build(mu);
}
@Override
public NoneType packageGroup(
- String name,
- Sequence<?> packagesO,
- Sequence<?> includesO,
- StarlarkThread thread)
+ String name, Sequence<?> packagesO, Sequence<?> includesO, StarlarkThread thread)
throws EvalException {
BazelStarlarkContext.from(thread).checkLoadingPhase("native.package_group");
PackageContext context = getContext(thread);
@@ -290,13 +518,7 @@
Dict.Builder<String, Object> values = Dict.builder();
for (Attribute attr : rule.getAttributes()) {
- if (!Character.isAlphabetic(attr.getName().charAt(0))) {
- continue;
- }
-
- if (attr.getName().equals("distribs")) {
- // attribute distribs: cannot represent type class java.util.Collections$SingletonSet
- // in Starlark: [INTERNAL].
+ if (!isPotentiallyExportableAttribute(rule.getRuleClassObject(), attr.getName())) {
continue;
}
@@ -319,22 +541,81 @@
}
/**
+ * Returns true if the given attribute of a rule class is generally allowed to be exposed via
+ * {@code native.existing_rule()} and {@code native.existing_rules()}.
+ *
+ * <p>This method makes no attempt to validate that the attribute exists in the rule class.
+ *
+ * <p>Even if this method returns true, the attribute may still be suppressed if it has a
+ * prohibited value (e.g. is of a bad type, or is a select() that cannot be processed).
+ */
+ private static boolean isPotentiallyExportableAttribute(
+ RuleClass ruleClass, String attributeName) {
+ if (attributeName.length() == 0 || !Character.isAlphabetic(attributeName.charAt(0))) {
+ // Do not expose hidden or implicit attributes.
+ return false;
+ }
+ if (attributeName.equals("distribs")) {
+ Attribute attr = ruleClass.getAttributeByName(attributeName);
+ if (attr != null && attr.getType() == BuildType.DISTRIBUTIONS) {
+ // "distribs" attribute (a Set<License.DistributionType> value) is not a StarlarkValue. Note
+ // that we cannot check for a Set<License.DistributionType> directly because generic type
+ // info is erased at runime.
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Returns true if the given value is generally allowed to be exposed via {@code
+ * native.existing_rule()} and or {@code native.existing_rules()}. Returns false for null.
+ *
+ * <p>Even if this method returns true, the value may still be suppressed if it is a select() that
+ * cannot be processed.
+ */
+ private static boolean isPotentiallyStarlarkifiableValue(@Nullable Object val) {
+ if (val == null) {
+ return false;
+ }
+ if (val.getClass().isAnonymousClass()) {
+ // Computed defaults. They will be represented as
+ // "deprecation": com.google.devtools.build.lib.analysis.BaseRuleClasses$2@6960884a,
+ // Filter them until we invent something more clever.
+ return false;
+ }
+
+ if (val instanceof License) {
+ // License is deprecated as a Starlark type, so omit this type from Starlark values
+ // to avoid exposing these objects, even though they are technically StarlarkValue.
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
* Converts a target attribute value to a Starlark value for return in {@code
* native.existing_rule()} or {@code native.existing_rules()}.
*
* <p>Any dict values in the result have mutability {@code mu}.
*
+ * <p>Any label values in the result which are inside {@code pkg} (the current package) are
+ * rewritten using ":foo" shorthand.
+ *
* @return the value, or null if we don't want to export it to the user.
* @throws NotRepresentableException if an unknown type is encountered.
*/
+ // TODO(https://github.com/bazelbuild/bazel/issues/13829): don't throw NotRepresentableException;
+ // perhaps change to an unchecked exception instead?
@Nullable
private static Object starlarkifyValue(Mutability mu, Object val, Package pkg)
throws NotRepresentableException {
// easy cases
- if (val == null
- || val instanceof Boolean
- || val instanceof String
- || val instanceof StarlarkInt) {
+ if (!isPotentiallyStarlarkifiableValue(val)) {
+ return null;
+ }
+ if (val instanceof Boolean || val instanceof String || val instanceof StarlarkInt) {
return val;
}
@@ -352,6 +633,8 @@
if (val instanceof Label) {
Label l = (Label) val;
if (l.getPackageName().equals(pkg.getName())) {
+ // TODO(https://github.com/bazelbuild/bazel/issues/13828): do not ignore the repo component
+ // of the label.
return ":" + l.getName();
}
return l.getCanonicalForm();
@@ -385,19 +668,6 @@
return m.build(mu);
}
- if (val.getClass().isAnonymousClass()) {
- // Computed defaults. They will be represented as
- // "deprecation": com.google.devtools.build.lib.analysis.BaseRuleClasses$2@6960884a,
- // Filter them until we invent something more clever.
- return null;
- }
-
- if (val instanceof License) {
- // License is deprecated as a Starlark type, so omit this type from Starlark values
- // to avoid exposing these objects, even though they are technically StarlarkValue.
- return null;
- }
-
if (val instanceof BuildType.SelectorList) {
List<Object> selectors = new ArrayList<>();
for (BuildType.Selector<?> selector : ((BuildType.SelectorList<?>) val).getSelectors()) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
index a0eaad8..06c1180 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
@@ -152,10 +152,9 @@
if (old instanceof Rule) {
Verify.verify(((Rule) old).getOutputFiles().isEmpty());
}
-
- pkg.removeTarget(rule);
+ pkg.replaceTarget(rule);
+ } else {
+ pkg.addRule(rule);
}
-
- pkg.addRule(rule);
}
}
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 b5070bb..9d86319 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
@@ -156,6 +156,17 @@
public boolean incompatibleEnableExportsProvider;
@Option(
+ name = "experimental_existing_rules_immutable_view",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS, OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+ help =
+ "If set to true, native.existing_rule and native.existing_rules return lightweight"
+ + " immutable view objects instead of mutable dicts.")
+ public boolean experimentalExistingRulesImmutableView;
+
+ @Option(
name = "experimental_google_legacy_api",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -592,6 +603,8 @@
.setBool(
EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis)
.setBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER, incompatibleEnableExportsProvider)
+ .setBool(
+ EXPERIMENTAL_EXISTING_RULES_IMMUTABLE_VIEW, experimentalExistingRulesImmutableView)
.setBool(EXPERIMENTAL_GOOGLE_LEGACY_API, experimentalGoogleLegacyApi)
.setBool(EXPERIMENTAL_NINJA_ACTIONS, experimentalNinjaActions)
.setBool(EXPERIMENTAL_PLATFORMS_API, experimentalPlatformsApi)
@@ -666,6 +679,8 @@
"-experimental_enable_android_migration_apis";
public static final String INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER =
"-incompatible_enable_exports_provider";
+ public static final String EXPERIMENTAL_EXISTING_RULES_IMMUTABLE_VIEW =
+ "-experimental_existing_rules_immutable_view";
public static final String EXPERIMENTAL_GOOGLE_LEGACY_API = "-experimental_google_legacy_api";
public static final String EXPERIMENTAL_NINJA_ACTIONS = "-experimental_ninja_actions";
public static final String EXPERIMENTAL_PLATFORMS_API = "-experimental_platforms_api";
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java
index a0d07a6..381c8e9 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkNativeModuleApi.java
@@ -19,7 +19,6 @@
import net.starlark.java.annot.ParamType;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
-import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.NoneType;
import net.starlark.java.eval.Sequence;
@@ -93,13 +92,20 @@
@StarlarkMethod(
name = "existing_rule",
doc =
- "Returns a new mutable dict that describes the attributes of a rule instantiated in this "
- + "thread's package, or <code>None</code> if no rule instance of that name exists." //
- + "<p>The dict contains an entry for each attribute, except private ones, whose"
- + " names do not start with a letter. In addition, the dict contains entries for the"
- + " rule instance's <code>name</code> and <code>kind</code> (for example,"
+ "Returns a new mutable dict that describes the attributes of a rule instantiated in this"
+ + " thread's package, or <code>None</code> if no rule instance of that name"
+ + " exists." //
+ + "<p>If the <code>--experimental_existing_rules_immutable_view</code> flag is set,"
+ + " instead returns an immutable dict-like object (i.e. supporting dict-like"
+ + " iteration, <code>len(x)</code>, <code>name in x</code>, <code>x[name]</code>,"
+ + " <code>x.get(name)</code>, <code>x.items()</code>, <code>x.keys()</code>, and"
+ + " <code>x.values()</code>) with the same content." //
+ + "<p>The result contains an entry for each attribute, with the exception of private"
+ + " ones (whose names do not start with a letter) and a few unrepresentable legacy"
+ + " attribute types. In addition, the dict contains entries for the rule instance's"
+ + " <code>name</code> and <code>kind</code> (for example,"
+ " <code>'cc_binary'</code>)." //
- + "<p>The values of the dict represent attribute values as follows:" //
+ + "<p>The values of the result represent attribute values as follows:" //
+ "<ul><li>Attributes of type str, int, and bool are represented as is.</li>" //
+ "<li>Labels are converted to strings of the form <code>':foo'</code> for targets"
+ " in the same package or <code>'//pkg:name'</code> for targets in a different"
@@ -125,11 +131,18 @@
doc =
"Returns a new mutable dict describing the rules so far instantiated in this thread's"
+ " package. Each dict entry maps the name of the rule instance to the result that"
- + " would be returned by <code>existing_rule(name)</code>.<p><i>Note: If possible,"
- + " avoid using this function. It makes BUILD files brittle and order-dependent, and"
- + " it may be expensive especially if called within a loop.</i>",
+ + " would be returned by <code>existing_rule(name)</code>." //
+ + "<p>If the <code>--experimental_existing_rules_immutable_view</code> flag is set,"
+ + " instead returns an immutable dict-like object (i.e. supporting dict-like"
+ + " iteration, <code>len(x)</code>, <code>name in x</code>, <code>x[name]</code>,"
+ + " <code>x.get(name)</code>, <code>x.items()</code>, <code>x.keys()</code>, and"
+ + " <code>x.values()</code>) with the same content." //
+ + "<p><i>Note: If possible, avoid using this function. It makes BUILD files brittle"
+ + " and order-dependent. Furthermore, unless the"
+ + " <code>--experimental_existing_rules_immutable_view</code> flag is set, this"
+ + " function may be very expensive, especially if called within a loop.</i>",
useStarlarkThread = true)
- Dict<String, Dict<String, Object>> existingRules(StarlarkThread thread) throws EvalException;
+ Object existingRules(StarlarkThread thread) throws EvalException;
@StarlarkMethod(
name = "package_group",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD
index e4f2249..f256d90 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD
@@ -90,6 +90,7 @@
"//src/test/java/com/google/devtools/build/lib/testutil",
"//src/test/java/com/google/devtools/build/lib/testutil:JunitUtils",
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
+ "//third_party:auto_value",
"//third_party:guava",
"//third_party:guava-testlib",
"//third_party:jsr305",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java b/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java
new file mode 100644
index 0000000..ceb3c6f
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/packages/NativeExistingRulesTest.java
@@ -0,0 +1,452 @@
+// Copyright 2021 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.packages;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
+import java.util.HashMap;
+import java.util.Map;
+import net.starlark.java.annot.Param;
+import net.starlark.java.annot.StarlarkBuiltin;
+import net.starlark.java.annot.StarlarkMethod;
+import net.starlark.java.eval.Dict;
+import net.starlark.java.eval.Starlark;
+import net.starlark.java.eval.StarlarkInt;
+import net.starlark.java.eval.StarlarkValue;
+import net.starlark.java.eval.Tuple;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@code native.existing_rule} and {@code native.existing_rules} functions.
+ *
+ * <p>This class covers the legacy behavior where the {@code
+ * --experimental_existing_rules_immutable_view} flag is disabled. The enabled case is covered by
+ * the subclass, {@link WithImmutableView}.
+ */
+@RunWith(JUnit4.class)
+public class NativeExistingRulesTest extends BuildViewTestCase {
+ private TestStarlarkBuiltin testStarlarkBuiltin; // initialized by createRuleClassProvider()
+
+ // Intended to be overridden by this test case's subclasses. Note that overriding of JUnit's
+ // @Before methods is not recommended.
+ protected void setupOptions() throws Exception {
+ // --noexperimental_existing_rules_immutable_view is the default; set it explicitly for clarity.
+ setBuildLanguageOptions("--noexperimental_existing_rules_immutable_view");
+ }
+
+ @Before
+ public final void setUp() throws Exception {
+ setupOptions();
+ }
+
+ @StarlarkBuiltin(name = "test")
+ private static final class TestStarlarkBuiltin implements StarlarkValue {
+
+ private final Map<String, Object> saved = new HashMap<>();
+
+ @StarlarkMethod(
+ name = "save",
+ parameters = {
+ @Param(name = "name", doc = "Name under which to save the value"),
+ @Param(name = "value", doc = "Value to save")
+ },
+ doc = "Saves a Starlark value for testing from Java")
+ public synchronized void save(String name, Object value) {
+ saved.put(name, value);
+ }
+ }
+
+ @Override
+ protected ConfiguredRuleClassProvider createRuleClassProvider() {
+ ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
+ TestRuleClassProvider.addStandardRules(builder);
+ testStarlarkBuiltin = new TestStarlarkBuiltin();
+ builder.addStarlarkAccessibleTopLevels("test", testStarlarkBuiltin);
+ return builder.build();
+ }
+
+ private Object getSaved(String name) {
+ return testStarlarkBuiltin.saved.get(name);
+ }
+
+ @Test
+ public void existingRule_handlesSelect() throws Exception {
+ scratch.file("test/starlark/BUILD");
+ scratch.file(
+ "test/starlark/rulestr.bzl", //
+ "def rule_dict(name):",
+ " return native.existing_rule(name)");
+
+ scratch.file(
+ "test/getrule/BUILD",
+ "load('//test/starlark:rulestr.bzl', 'rule_dict')",
+ "cc_library(",
+ " name ='x',",
+ " srcs = select({'//conditions:default': []}),",
+ ")",
+ "rule_dict('x')");
+
+ // Parse the BUILD file, to make sure select() makes it out of native.existing_rule().
+ assertThat(getConfiguredTarget("//test/getrule:x")).isNotNull();
+ }
+
+ @Test
+ public void existingRule_returnsNone() throws Exception {
+ scratch.file(
+ "test/rulestr.bzl",
+ "def test_rule(name, x):",
+ " print(native.existing_rule(x))",
+ " if native.existing_rule(x) == None:",
+ " native.cc_library(name = name)");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:rulestr.bzl', 'test_rule')",
+ "test_rule('a', 'does not exist')",
+ "test_rule('b', 'BUILD')"); // exists, but as a target and not a rule
+
+ assertThat(getConfiguredTarget("//test:a")).isNotNull();
+ assertThat(getConfiguredTarget("//test:b")).isNotNull();
+ }
+
+ @Test
+ public void existingRule_roundTripsSelect() throws Exception {
+ scratch.file(
+ "test/existing_rule.bzl",
+ "def macro():",
+ " s = select({'//foo': ['//bar']})",
+ " print('Passed: ' + repr(s))",
+ " native.cc_library(name = 'x', srcs = s)",
+ " print('Returned: ' + repr(native.existing_rule('x')['srcs']))",
+ // The value returned here should round-trip fine.
+ " native.cc_library(name = 'y', srcs = native.existing_rule('x')['srcs'])");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:existing_rule.bzl', 'macro')",
+ "macro()",
+ "cc_library(name = 'a', srcs = [])");
+ getConfiguredTarget("//test:a");
+ assertContainsEvent("Passed: select({\"//foo\": [\"//bar\"]}");
+ // The short labels are now in their canonical form, and the sequence is represented as
+ // tuple instead of list, but the meaning is unchanged.
+ assertContainsEvent("Returned: select({\"//foo:foo\": (\"//bar:bar\",)}");
+ }
+
+ @Test
+ public void existingRule_shortensLabelsInSamePackage() throws Exception {
+ scratch.file(
+ "test/existing_rule.bzl",
+ "def save_deps():",
+ " r = native.existing_rule('b')",
+ " test.save(\"r['deps']\", r['deps'])");
+ scratch.file(
+ "test/BUILD",
+ "load('//test:existing_rule.bzl', 'save_deps')",
+ "cc_library(name = 'a', srcs = [])",
+ "cc_binary(name = 'b', deps = ['//test:a'])",
+ "save_deps()");
+ getConfiguredTarget("//test:b");
+ assertThat(Starlark.toIterable(getSaved("r['deps']")))
+ .containsExactly(":a"); // as opposed to "//test:a"
+ }
+
+ @Test
+ public void existingRules_findsRulesAndAttributes() throws Exception {
+ scratch.file("test/BUILD");
+ scratch.file("test/starlark/BUILD");
+ scratch.file(
+ "test/starlark/rulestr.bzl",
+ "def rule_dict(name):",
+ " return native.existing_rule(name)",
+ "def rules_dict():",
+ " return native.existing_rules()",
+ "def nop(ctx):",
+ " pass",
+ "nop_rule = rule(attrs = {'x': attr.label()}, implementation = nop)",
+ "def test_save(name, value):",
+ " test.save(name, value)");
+
+ scratch.file(
+ "test/getrule/BUILD",
+ "load('//test/starlark:rulestr.bzl', 'rules_dict', 'rule_dict', 'nop_rule', 'test_save')",
+ "genrule(name = 'a', outs = ['a.txt'], ",
+ " licenses = ['notice'],",
+ " output_to_bindir = False,",
+ " tools = [ '//test:bla' ], cmd = 'touch $@')",
+ "nop_rule(name = 'c', x = ':a')",
+ "rlist = rules_dict()",
+ "test_save('all_str', [rlist['a']['kind'], rlist['a']['name'],",
+ " rlist['c']['kind'], rlist['c']['name']])",
+ "adict = rule_dict('a')",
+ "cdict = rule_dict('c')",
+ "test_save('a_str', [adict['kind'], adict['name'], adict['outs'][0], adict['tools'][0]])",
+ "test_save('c_str', [cdict['kind'], cdict['name'], cdict['x']])",
+ "test_save('adict.keys()', adict.keys())");
+
+ getConfiguredTarget("//test/getrule:BUILD");
+ assertThat(Starlark.toIterable(getSaved("all_str")))
+ .containsExactly("genrule", "a", "nop_rule", "c").inOrder();
+ assertThat(Starlark.toIterable(getSaved("a_str")))
+ .containsExactly("genrule", "a", ":a.txt", "//test:bla").inOrder();
+ assertThat(Starlark.toIterable(getSaved("c_str")))
+ .containsExactly("nop_rule", "c", ":a")
+ .inOrder();
+ assertThat(Starlark.toIterable(getSaved("adict.keys()")))
+ .containsAtLeast(
+ "name",
+ "visibility",
+ "transitive_configs",
+ "tags",
+ "generator_name",
+ "generator_function",
+ "generator_location",
+ "features",
+ "compatible_with",
+ "target_compatible_with",
+ "restricted_to",
+ "srcs",
+ "tools",
+ "toolchains",
+ "outs",
+ "cmd",
+ "output_to_bindir",
+ "local",
+ "message",
+ "executable",
+ "stamp",
+ "heuristic_label_expansion",
+ "kind");
+ }
+
+ @Test
+ public void existingRule_returnsObjectWithCorrectMutability() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key': 'value'})",
+ " r = native.existing_rule('x')",
+ " r['no_such_attribute'] = 'foo'",
+ " r['define_values']['key'] = 123"); // mutate the dict
+
+ assertThat(getConfiguredTarget("//test:BUILD")).isNotNull(); // no error on mutation
+ }
+
+ @Test
+ public void existingRule_returnsDictLikeObject() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key': 'value'})",
+ " r = native.existing_rule('x')",
+ " print('r == %s' % repr(r))",
+ " test.save('[key for key in r]', [key for key in r])",
+ " test.save('list(r)', list(r))",
+ " test.save('r.keys()', r.keys())",
+ " test.save('r.values()', r.values())",
+ " test.save('r.items()', r.items())",
+ " test.save(\"r['define_values']\", r['define_values'])",
+ " test.save(\"r.get('define_values', 123)\", r.get('define_values', 123))",
+ " test.save(\"r.get('invalid_attr', 123)\", r.get('invalid_attr', 123))",
+ " test.save(\"'define_values' in r\", 'define_values' in r)",
+ " test.save(\"'invalid_attr' in r\", 'invalid_attr' in r)");
+
+ Dict<?, ?> expectedDefineValues = Dict.builder().put("key", "value").buildImmutable();
+ assertThat(getConfiguredTarget("//test:BUILD")).isNotNull(); // no error
+ assertThat(Starlark.toIterable(getSaved("[key for key in r]")))
+ .containsAtLeast("define_values", "name", "kind");
+ assertThat(Starlark.toIterable(getSaved("list(r)")))
+ .containsAtLeast("define_values", "name", "kind");
+ assertThat(Starlark.toIterable(getSaved("r.keys()")))
+ .containsAtLeast("define_values", "name", "kind");
+ assertThat(Starlark.toIterable(getSaved("r.values()")))
+ .containsAtLeast(expectedDefineValues, "x", "config_setting");
+ assertThat(Starlark.toIterable(getSaved("r.items()")))
+ .containsAtLeast(
+ Tuple.of("define_values", expectedDefineValues),
+ Tuple.of("name", "x"),
+ Tuple.of("kind", "config_setting"));
+ assertThat(getSaved("r['define_values']")).isEqualTo(expectedDefineValues);
+ assertThat(getSaved("r.get('define_values', 123)")).isEqualTo(expectedDefineValues);
+ assertThat(getSaved("r.get('invalid_attr', 123)")).isEqualTo(StarlarkInt.of(123));
+ assertThat(getSaved("'define_values' in r")).isEqualTo(true);
+ assertThat(getSaved("'invalid_attr' in r")).isEqualTo(false);
+ }
+
+ @Test
+ public void existingRules_returnsObjectWithCorrectMutability() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key': 'value'})",
+ " rs = native.existing_rules()",
+ " rs['no_such_rule'] = {'name': 'no_such_rule', 'kind': 'config_setting'}"); // mutate
+
+ assertThat(getConfiguredTarget("//test:BUILD")).isNotNull(); // no error on mutation
+ }
+
+ @Test
+ public void existingRules_returnsDictLikeObject() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key_x': 'value_x'})",
+ " native.config_setting(name='y', define_values={'key_y': 'value_y'})",
+ " rs = native.existing_rules()",
+ " print('rs == %s' % repr(rs))",
+ " test.save('[key for key in rs]', [key for key in rs])",
+ " test.save('list(rs)', list(rs))",
+ " test.save('rs.keys()', rs.keys())",
+ " test.save(\"[v['name'] for v in rs.values()]\", [v['name'] for v in rs.values()])",
+ " test.save(\"[(i[0], i[1]['name']) for i in rs.items()]\", [(i[0], i[1]['name']) for i in"
+ + " rs.items()])",
+ " test.save(\"rs['x']['define_values']\", rs['x']['define_values'])",
+ " test.save(\"rs.get('x', {'name': 'z'})['name']\", rs.get('x', {'name': 'z'})['name'])",
+ " test.save(\"rs.get('invalid_rule', {'name': 'invalid_rule'})\", rs.get('invalid_rule',"
+ + " {'name': 'invalid_rule'}))",
+ " test.save(\"'x' in rs\", 'x' in rs)",
+ " test.save(\"'invalid_rule' in rs\", 'invalid_rule' in rs)");
+
+ assertThat(getConfiguredTarget("//test:BUILD")).isNotNull(); // no error
+ assertThat(Starlark.toIterable(getSaved("[key for key in rs]"))).containsExactly("x", "y");
+ assertThat(Starlark.toIterable(getSaved("list(rs)"))).containsExactly("x", "y");
+ assertThat(Starlark.toIterable(getSaved("rs.keys()"))).containsExactly("x", "y");
+ assertThat(Starlark.toIterable(getSaved("[v['name'] for v in rs.values()]")))
+ .containsExactly("x", "y");
+ assertThat(Starlark.toIterable(getSaved("[(i[0], i[1]['name']) for i in rs.items()]")))
+ .containsExactly(Tuple.of("x", "x"), Tuple.of("y", "y"));
+ assertThat(getSaved("rs['x']['define_values']"))
+ .isEqualTo(Dict.builder().put("key_x", "value_x").buildImmutable());
+ assertThat(getSaved("rs.get('x', {'name': 'z'})['name']")).isEqualTo("x");
+ assertThat(getSaved("rs.get('invalid_rule', {'name': 'invalid_rule'})"))
+ .isEqualTo(Dict.builder().put("name", "invalid_rule").buildImmutable());
+ assertThat(getSaved("'x' in rs")).isEqualTo(true);
+ assertThat(getSaved("'invalid_rule' in rs")).isEqualTo(false);
+ }
+
+ @Test
+ public void existingRules_returnsSnapshotOfOnlyRulesInstantiatedUpToThatPoint() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key_x': 'value_x'})",
+ " rs1 = native.existing_rules()",
+ " native.config_setting(name='y', define_values={'key_y': 'value_y'})",
+ " rs2 = native.existing_rules()",
+ " native.config_setting(name='z', define_values={'key_z': 'value_z'})",
+ " rs3 = native.existing_rules()",
+ " test.save('rs1.keys()', rs1.keys())",
+ " test.save('rs2.keys()', rs2.keys())",
+ " test.save('rs3.keys()', rs3.keys())");
+
+ assertThat(getConfiguredTarget("//test:BUILD")).isNotNull(); // no error
+ assertThat(Starlark.toIterable(getSaved("rs1.keys()"))).containsExactly("x");
+ assertThat(Starlark.toIterable(getSaved("rs2.keys()"))).containsExactly("x", "y");
+ assertThat(Starlark.toIterable(getSaved("rs3.keys()"))).containsExactly("x", "y", "z");
+ }
+
+ /**
+ * Tests for {@code native.existing_rule} and {@code native.existing_rules} Starlark functions
+ * with the {@code --experimental_existing_rules_immutable_view} flag set.
+ */
+ @RunWith(JUnit4.class)
+ public static final class WithImmutableView extends NativeExistingRulesTest {
+
+ @Override
+ protected void setupOptions() throws Exception {
+ setBuildLanguageOptions("--experimental_existing_rules_immutable_view");
+ }
+
+ @Test
+ @Override
+ public void existingRule_returnsObjectWithCorrectMutability() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key': 'value'})",
+ " r = native.existing_rule('x')",
+ " r['no_such_attribute'] = 123"); // mutate the view
+
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//test:BUILD")).isNull(); // mutation fails
+ assertContainsEvent("can only assign an element in a dictionary or a list");
+ }
+
+ @Test
+ @Override
+ public void existingRules_returnsObjectWithCorrectMutability() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key': 'value'})",
+ " rs = native.existing_rules()",
+ " rs['no_such_rule'] = {'name': 'no_such_rule', 'kind': 'config_setting'}"); // mutate
+
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//test:BUILD")).isNull(); // mutation fails
+ assertContainsEvent("can only assign an element in a dictionary or a list");
+ }
+
+ @Test
+ public void existingRules_returnsDeeplyImmutableView() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load('inc.bzl', 'f')", //
+ "f()");
+ scratch.file(
+ "test/inc.bzl", //
+ "def f():",
+ " native.config_setting(name='x', define_values={'key': 'value'})",
+ " rs = native.existing_rules()",
+ " rs['x']['define_values']['key'] = 123"); // mutate an attribute value within the view
+
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//test:BUILD")).isNull();
+ assertContainsEvent("trying to mutate a frozen dict value");
+ }
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SnapshottableBiMapTest.java b/src/test/java/com/google/devtools/build/lib/packages/SnapshottableBiMapTest.java
new file mode 100644
index 0000000..efd58f9
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/packages/SnapshottableBiMapTest.java
@@ -0,0 +1,396 @@
+// Copyright 2021 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.packages;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.ImmutableBiMap;
+import com.google.common.collect.ImmutableMap;
+import java.util.AbstractMap;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Map;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link SnapshottableBiMap}. */
+@RunWith(JUnit4.class)
+public final class SnapshottableBiMapTest {
+ // Dummy value type for maps under test. AutoValue for correct hash/equals behavior.
+ @AutoValue
+ abstract static class Value {
+ static Value trackedOf(String name) {
+ return new AutoValue_SnapshottableBiMapTest_Value(name, true);
+ }
+
+ static Value untrackedOf(String name) {
+ return new AutoValue_SnapshottableBiMapTest_Value(name, false);
+ }
+
+ static boolean track(Value value) {
+ return value.tracked();
+ }
+
+ abstract String name();
+
+ abstract boolean tracked();
+ }
+
+ private static <E> void verifyCollectionSizeAndContentsInOrder(
+ Collection<E> collection, Collection<E> expected) {
+ // Exhaustive testing of a collection's methods; we cannot rely on a minimal usual set of JUnit
+ // helpers because we want to verify that the collection has valid Collection semantics.
+ if (expected.isEmpty()) {
+ assertThat(collection).isEmpty();
+ } else {
+ assertThat(collection).isNotEmpty();
+ }
+ assertThat(collection).hasSize(expected.size());
+ assertThat(collection).containsExactlyElementsIn(expected).inOrder();
+ for (E entry : expected) {
+ // JUnit's containsExactlyElementsIn iterates over the collection under test, but doesn't call
+ // its contains() method.
+ assertThat(collection).contains(entry);
+ }
+ }
+
+ private static <K, V> void verifyMapSizeAndContentsInOrder(Map<K, V> map, Map<K, V> expectedMap) {
+ // Exhaustive testing of a map's methods; we cannot rely on a minimal usual set of JUnit helpers
+ // because we want to verify that the map has valid Map semantics.
+ if (expectedMap.isEmpty()) {
+ assertThat(map).isEmpty();
+ } else {
+ assertThat(map).isNotEmpty();
+ }
+
+ assertThat(map).hasSize(expectedMap.size());
+ assertThat(map).containsExactlyEntriesIn(expectedMap).inOrder();
+
+ for (Map.Entry<K, V> entry : expectedMap.entrySet()) {
+ assertThat(map.containsKey(entry.getKey()))
+ .isTrue(); // JUnit's containsKey implementation does not explicitly call map.containsKey
+ assertThat(map.containsValue(entry.getValue())).isTrue();
+ }
+
+ verifyCollectionSizeAndContentsInOrder(map.entrySet(), expectedMap.entrySet());
+ verifyCollectionSizeAndContentsInOrder(map.keySet(), expectedMap.keySet());
+ verifyCollectionSizeAndContentsInOrder(map.values(), expectedMap.values());
+ }
+
+ @SuppressWarnings("unchecked") // test-only convenience vararg transformation
+ private static <K, V> void verifyMapSizeAndContentsInOrder(
+ Map<K, V> map, K key0, V value0, Object... rest) {
+ ImmutableMap.Builder<K, V> expectedBuilder = ImmutableMap.builder();
+ expectedBuilder.put(key0, value0);
+ Preconditions.checkArgument(
+ rest.length % 2 == 0, "rest must be a flattened list of key-value pairs");
+ for (int i = 0; i < rest.length; i += 2) {
+ expectedBuilder.put((K) rest[i], (V) rest[i + 1]);
+ }
+ Map<K, V> expectedMap = expectedBuilder.build();
+ verifyMapSizeAndContentsInOrder(map, expectedMap);
+ }
+
+ private static <K, V> void verifyMapDoesNotContainEntry(Map<K, V> map, K key, V value) {
+ Map.Entry<K, V> entry = new AbstractMap.SimpleEntry<>(key, value);
+
+ // Exhaustive testing of a map's methods; we cannot rely on a minimal usual set of JUnit helpers
+ // because we want to verify that the map has valid Map semantics.
+ assertThat(map.containsKey(key))
+ .isFalse(); // JUnit's containsKey implementation does not explicitly call map.containsKeys
+ assertThat(map.containsValue(value)).isFalse();
+ assertThat(map.entrySet()).doesNotContain(entry);
+ assertThat(map.keySet()).doesNotContain(key);
+ assertThat(map.values()).doesNotContain(value);
+ }
+
+ private static <K, V> void verifyMapIsEmpty(Map<K, V> map) {
+ verifyMapSizeAndContentsInOrder(map, ImmutableMap.of());
+ }
+
+ private static <E> void verifyIteratorDoesNotAllowDeletions(Iterator<E> iterator) {
+ while (iterator.hasNext()) {
+ iterator.next();
+ assertThrows(UnsupportedOperationException.class, iterator::remove);
+ }
+ }
+
+ private static <K, V> void verifyMapDoesNotAllowDeletions(Map<K, V> map) {
+ for (Map.Entry<K, V> entry : map.entrySet()) {
+ K key = entry.getKey();
+ V value = entry.getValue();
+ assertThrows(UnsupportedOperationException.class, () -> map.remove(key));
+ assertThrows(UnsupportedOperationException.class, () -> map.keySet().remove(key));
+ assertThrows(UnsupportedOperationException.class, () -> map.values().remove(value));
+ assertThrows(UnsupportedOperationException.class, () -> map.entrySet().remove(entry));
+ }
+
+ verifyIteratorDoesNotAllowDeletions(map.keySet().iterator());
+ verifyIteratorDoesNotAllowDeletions(map.values().iterator());
+ verifyIteratorDoesNotAllowDeletions(map.entrySet().iterator());
+
+ assertThrows(UnsupportedOperationException.class, map::clear);
+ }
+
+ @SuppressWarnings("unchecked") // test-only convenience vararg transformation
+ private static <K, V> void verifyBiMapSizeAndContentsInOrder(
+ BiMap<K, V> bimap, K key0, V value0, Object... rest) {
+ ImmutableBiMap.Builder<K, V> expectedBuilder = ImmutableBiMap.builder();
+ expectedBuilder.put(key0, value0);
+ Preconditions.checkArgument(
+ rest.length % 2 == 0, "rest must be a flattened list of key-value pairs");
+ for (int i = 0; i < rest.length; i += 2) {
+ expectedBuilder.put((K) rest[i], (V) rest[i + 1]);
+ }
+ BiMap<K, V> expectedBiMap = expectedBuilder.build();
+ verifyMapSizeAndContentsInOrder(bimap, expectedBiMap);
+ verifyMapSizeAndContentsInOrder(bimap.inverse(), expectedBiMap.inverse());
+ }
+
+ private static <K, V> void verifyBiMapIsEmpty(BiMap<K, V> bimap) {
+ verifyMapSizeAndContentsInOrder(bimap, ImmutableMap.of());
+ verifyMapSizeAndContentsInOrder(bimap.inverse(), ImmutableMap.of());
+ }
+
+ @Test
+ public void containsInsertedEntries() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ verifyBiMapIsEmpty(map);
+ Value a = Value.trackedOf("a");
+ Value b = Value.untrackedOf("b");
+ Value c = Value.trackedOf("c");
+ Value z = Value.trackedOf("z");
+
+ map.put("a", a);
+ verifyBiMapSizeAndContentsInOrder(map, "a", a);
+
+ map.put("b", b);
+ verifyBiMapSizeAndContentsInOrder(map, "a", a, "b", b);
+
+ map.put("c", c);
+ verifyBiMapSizeAndContentsInOrder(map, "a", a, "b", b, "c", c);
+
+ // verify that the map's various contains*() methods don't always return true.
+ verifyMapDoesNotContainEntry(map, "z", z);
+ }
+
+ @Test
+ public void put_replacesEntries() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value trackedA = Value.trackedOf("a");
+ Value replaceA = Value.trackedOf("replace a");
+ Value untrackedB = Value.untrackedOf("b");
+ Value replaceB = Value.untrackedOf("b");
+
+ map.put("a", trackedA);
+ map.put("a", replaceA);
+ map.put("b", untrackedB);
+ map.put("b", replaceB);
+ verifyBiMapSizeAndContentsInOrder(map, "a", replaceA, "b", replaceB);
+ }
+
+ @Test
+ public void put_nonUniqueValue_illegal() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value tracked = Value.trackedOf("a");
+ Value untracked = Value.untrackedOf("b");
+
+ map.put("a", tracked);
+ assertThrows(IllegalArgumentException.class, () -> map.put("aa", tracked));
+ map.put("b", untracked);
+ assertThrows(IllegalArgumentException.class, () -> map.put("bb", untracked));
+ }
+
+ @Test
+ public void put_replacingUntrackedWithTracked_legal() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value tracked = Value.trackedOf("a");
+ Value untracked = Value.untrackedOf("A");
+
+ map.getTrackedSnapshot(); // start tracking
+ map.put("a", untracked);
+ map.put("a", tracked);
+ verifyBiMapSizeAndContentsInOrder(map, "a", tracked);
+ }
+
+ @Test
+ public void put_replacingTrackedWithUntracked_illegal() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value tracked = Value.trackedOf("a");
+ Value untracked = Value.untrackedOf("A");
+
+ map.getTrackedSnapshot(); // start tracking
+ map.put("a", tracked);
+ assertThrows(IllegalArgumentException.class, () -> map.put("a", untracked));
+ }
+
+ @Test
+ @SuppressWarnings("deprecation") // test verifying that deprecated methods don't work
+ public void deletions_unsupported() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value value = Value.trackedOf("a");
+ Value replacement = Value.trackedOf("replacement a");
+
+ map.put("a", value);
+ verifyMapDoesNotAllowDeletions(map);
+ verifyMapDoesNotAllowDeletions(map.inverse());
+ assertThrows(UnsupportedOperationException.class, () -> map.forcePut("a", replacement));
+ assertThrows(UnsupportedOperationException.class, () -> map.inverse().forcePut(value, "aa"));
+ }
+
+ @Test
+ public void getUnderlyingBiMap_returnsBiMapSupportingRemove() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value a = Value.trackedOf("a");
+ Value b = Value.untrackedOf("b");
+ Value c = Value.trackedOf("c");
+
+ map.put("a", a);
+ map.put("b", b);
+ map.put("c", c);
+ BiMap<String, Value> underlying = map.getUnderlyingBiMap();
+ verifyBiMapSizeAndContentsInOrder(underlying, "a", a, "b", b, "c", c);
+
+ underlying.remove("a");
+ verifyBiMapSizeAndContentsInOrder(underlying, "b", b, "c", c);
+ }
+
+ @Test
+ public void snapshot_containsExpectedEntries() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value trackedA = Value.trackedOf("a");
+ Value untrackedB = Value.untrackedOf("b");
+ Value trackedC = Value.trackedOf("c");
+ Value z = Value.trackedOf("z");
+
+ Map<String, Value> snapshot0 = map.getTrackedSnapshot();
+ verifyMapIsEmpty(snapshot0);
+
+ map.put("a", trackedA);
+ Map<String, Value> snapshot1 = map.getTrackedSnapshot();
+ verifyMapIsEmpty(snapshot0);
+ verifyMapSizeAndContentsInOrder(snapshot1, "a", trackedA);
+
+ map.put("b", untrackedB);
+ Map<String, Value> snapshot2 = map.getTrackedSnapshot();
+ verifyMapIsEmpty(snapshot0);
+ verifyMapSizeAndContentsInOrder(snapshot1, "a", trackedA);
+ verifyMapSizeAndContentsInOrder(snapshot2, "a", trackedA); // b is untracked
+
+ map.put("c", Value.trackedOf("c"));
+ Map<String, Value> snapshot3 = map.getTrackedSnapshot();
+ verifyMapIsEmpty(snapshot0);
+ verifyMapSizeAndContentsInOrder(snapshot1, "a", trackedA); // c was added after snapshot
+ verifyMapSizeAndContentsInOrder(snapshot2, "a", trackedA);
+ verifyMapSizeAndContentsInOrder(snapshot3, "a", trackedA, "c", trackedC);
+
+ // verify that a snapshot's various contains*() methods don't always return true.
+ verifyMapDoesNotContainEntry(snapshot1, "z", z);
+ verifyMapDoesNotContainEntry(snapshot2, "z", z);
+ verifyMapDoesNotContainEntry(snapshot3, "z", z);
+ }
+
+ @Test
+ public void snapshot_isUnmodifiable() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ map.put("a", Value.trackedOf("a"));
+ map.put("b", Value.untrackedOf("b"));
+ map.put("c", Value.trackedOf("c"));
+ Map<String, Value> snapshot = map.getTrackedSnapshot();
+
+ verifyMapDoesNotAllowDeletions(snapshot);
+ assertThrows(
+ UnsupportedOperationException.class, () -> snapshot.put("a", Value.trackedOf("replace a")));
+ assertThrows(
+ UnsupportedOperationException.class, () -> snapshot.put("d", Value.trackedOf("d")));
+ }
+
+ @Test
+ public void snapshot_containsReplacementsPerformedBeforeSnapshotCreation() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value trackedA = Value.trackedOf("a");
+ Value replacementA = Value.trackedOf("replacement a");
+ Value untrackedB = Value.untrackedOf("b");
+ Value replacementB = Value.trackedOf("replacement b");
+
+ map.put("a", trackedA);
+ map.put("b", untrackedB);
+ verifyMapSizeAndContentsInOrder(map, "a", trackedA, "b", untrackedB);
+ map.put("a", replacementA);
+ map.put("b", replacementB);
+ verifyMapSizeAndContentsInOrder(map, "a", replacementA, "b", replacementB);
+
+ Map<String, Value> snapshot = map.getTrackedSnapshot();
+ verifyMapSizeAndContentsInOrder(snapshot, "a", replacementA, "b", replacementB);
+ }
+
+ @Test
+ public void snapshot_afterReplacingEntryInSnapshot_containsReplacement() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value original = Value.trackedOf("a");
+ Value replacement = Value.trackedOf("replacement a");
+
+ map.put("a", original);
+ Map<String, Value> snapshot = map.getTrackedSnapshot();
+ verifyMapSizeAndContentsInOrder(snapshot, "a", original);
+
+ map.put("a", replacement);
+ verifyMapSizeAndContentsInOrder(snapshot, "a", replacement);
+ }
+
+ @Test
+ public void snapshot_afterReplacingEntryNotInSnapshot_doesNotContainReplacement() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value untrackedA = Value.untrackedOf("a");
+ Value replacementA = Value.trackedOf("replacement a");
+ Value trackedB = Value.trackedOf("b");
+ Value replacementB = Value.trackedOf("replacement b");
+
+ map.put("a", untrackedA);
+ Map<String, Value> snapshot = map.getTrackedSnapshot();
+ verifyMapIsEmpty(snapshot);
+
+ map.put("a", replacementA);
+ map.put("b", trackedB);
+ map.put("b", replacementB);
+ verifyMapSizeAndContentsInOrder(map, "a", replacementA, "b", replacementB);
+ verifyMapIsEmpty(snapshot);
+ }
+
+ @Test
+ public void snapshot_containsReplacementEntries_inOriginalKeyInsertionOrder() {
+ SnapshottableBiMap<String, Value> map = new SnapshottableBiMap<>(Value::track);
+ Value a = Value.trackedOf("a");
+ Value b = Value.trackedOf("b");
+ Value replaceB = Value.trackedOf("replacement b");
+ Value c = Value.trackedOf("c");
+ Value replaceC = Value.trackedOf("replacement c");
+
+ map.put("a", a);
+ map.put("b", b);
+ map.put("c", c);
+
+ Map<String, Value> snapshot = map.getTrackedSnapshot();
+ verifyMapSizeAndContentsInOrder(snapshot, "a", a, "b", b, "c", c);
+
+ map.put("c", replaceC);
+ map.put("b", replaceB);
+ verifyMapSizeAndContentsInOrder(snapshot, "a", a, "b", replaceB, "c", replaceC);
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
index ecc2ae3..fd40d3d 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java
@@ -519,158 +519,6 @@
}
@Test
- public void testGetRuleSelect() throws Exception {
- scratch.file("test/starlark/BUILD");
- scratch.file(
- "test/starlark/rulestr.bzl", "def rule_dict(name):", " return native.existing_rule(name)");
-
- scratch.file(
- "test/getrule/BUILD",
- "load('//test/starlark:rulestr.bzl', 'rule_dict')",
- "cc_library(name ='x', ",
- " srcs = select({'//conditions:default': []})",
- ")",
- "rule_dict('x')");
-
- // Parse the BUILD file, to make sure select() makes it out of native.rule().
- createRuleContext("//test/getrule:x");
- }
-
- @Test
- public void testExistingRuleReturnNone() throws Exception {
- scratch.file(
- "test/rulestr.bzl",
- "def test_rule(name, x):",
- " print(native.existing_rule(x))",
- " if native.existing_rule(x) == None:",
- " native.cc_library(name = name)");
- scratch.file(
- "test/BUILD",
- "load('//test:rulestr.bzl', 'test_rule')",
- "test_rule('a', 'does not exist')",
- "test_rule('b', 'BUILD')");
-
- assertThat(getConfiguredTarget("//test:a")).isNotNull();
- assertThat(getConfiguredTarget("//test:b")).isNotNull();
- }
-
- @Test
- public void existingRuleWithSelect() throws Exception {
- scratch.file(
- "test/existing_rule.bzl",
- "def macro():",
- " s = select({'//foo': ['//bar']})",
- " print('Passed: ' + repr(s))",
- " native.cc_library(name = 'x', srcs = s)",
- " print('Returned: ' + repr(native.existing_rule('x')['srcs']))",
- // The value returned here should round-trip fine.
- " native.cc_library(name = 'y', srcs = native.existing_rule('x')['srcs'])");
- scratch.file(
- "test/BUILD",
- "load('//test:existing_rule.bzl', 'macro')",
- "macro()",
- "cc_library(name = 'a', srcs = [])");
- getConfiguredTarget("//test:a");
- assertContainsEvent("Passed: select({\"//foo\": [\"//bar\"]}");
- // The short labels are now in their canonical form, and the sequence is represented as
- // tuple instead of list, but the meaning is unchanged.
- assertContainsEvent("Returned: select({\"//foo:foo\": (\"//bar:bar\",)}");
- }
-
- @Test
- public void testGetRule() throws Exception {
- scratch.file("test/starlark/BUILD");
- scratch.file(
- "test/starlark/rulestr.bzl",
- "def rule_dict(name):",
- " return native.existing_rule(name)",
- "def rules_dict():",
- " return native.existing_rules()",
- "def nop(ctx):",
- " pass",
- "nop_rule = rule(attrs = {'x': attr.label()}, implementation = nop)",
- "consume_rule = rule(attrs = {'s': attr.string_list()}, implementation = nop)");
-
- scratch.file(
- "test/getrule/BUILD",
- "load('//test/starlark:rulestr.bzl', 'rules_dict', 'rule_dict', 'nop_rule',"
- + "'consume_rule')",
- "genrule(name = 'a', outs = ['a.txt'], ",
- " licenses = ['notice'],",
- " output_to_bindir = False,",
- " tools = [ '//test:bla' ], cmd = 'touch $@')",
- "nop_rule(name = 'c', x = ':a')",
- "rlist= rules_dict()",
- "consume_rule(name = 'all_str', s = [rlist['a']['kind'], rlist['a']['name'], ",
- " rlist['c']['kind'], rlist['c']['name']])",
- "adict = rule_dict('a')",
- "cdict = rule_dict('c')",
- "consume_rule(name = 'a_str', ",
- " s = [adict['kind'], adict['name'], adict['outs'][0], adict['tools'][0]])",
- "consume_rule(name = 'genrule_attr', ",
- " s = adict.keys())",
- "consume_rule(name = 'c_str', s = [cdict['kind'], cdict['name'], cdict['x']])");
-
- StarlarkRuleContext allContext = createRuleContext("//test/getrule:all_str");
- setRuleContext(allContext);
- List<?> result = (List) ev.eval("ruleContext.attr.s");
- assertThat(result).containsExactly("genrule", "a", "nop_rule", "c");
-
- setRuleContext(createRuleContext("//test/getrule:a_str"));
- result = (List) ev.eval("ruleContext.attr.s");
- assertThat(result).containsExactly("genrule", "a", ":a.txt", "//test:bla");
-
- setRuleContext(createRuleContext("//test/getrule:c_str"));
- result = (List) ev.eval("ruleContext.attr.s");
- assertThat(result).containsExactly("nop_rule", "c", ":a");
-
- setRuleContext(createRuleContext("//test/getrule:genrule_attr"));
- result = (List) ev.eval("ruleContext.attr.s");
- assertThat(result)
- .containsAtLeast(
- "name",
- "visibility",
- "transitive_configs",
- "tags",
- "generator_name",
- "generator_function",
- "generator_location",
- "features",
- "compatible_with",
- "target_compatible_with",
- "restricted_to",
- "srcs",
- "tools",
- "toolchains",
- "outs",
- "cmd",
- "output_to_bindir",
- "local",
- "message",
- "executable",
- "stamp",
- "heuristic_label_expansion",
- "kind");
- }
-
- @Test
- public void testExistingRuleDictIsMutable() throws Exception {
- scratch.file(
- "test/BUILD",
- "load('inc.bzl', 'f')", //
- "f()");
- scratch.file(
- "test/inc.bzl", //
- "def f():",
- " native.config_setting(name='x', define_values={'key': 'value'})",
- " r = native.existing_rule('x')",
- " r['define_values']['key'] = 123"); // mutate the dict
-
- // Logically this belongs among the loading-phase tests of existing_rules. Where are they?
- assertThat(getConfiguredTarget("//test:BUILD")).isNotNull(); // no error
- }
-
- @Test
public void testGetRuleAttributeListValue() throws Exception {
StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
setRuleContext(ruleContext);
