Restrict direct dependency propagation to INCLUDE items
The only remaining intended use of this feature is for
objc_proto_library strict dependency includes, internal to Google. I
would like to lock this down to avoid unintended usage.
RELNOTES: None
PiperOrigin-RevId: 292189451
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
index f4d502e..7f446cc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleSkylarkCommon.java
@@ -80,6 +80,10 @@
+ "iterable with %s.";
@VisibleForTesting
+ public static final String BAD_DIRECT_DEPENDENCY_KEY_ERROR =
+ "Key %s not allowed to be in direct_dep_provider.";
+
+ @VisibleForTesting
public static final String NOT_SET_ERROR = "Value for key %s must be a set, instead found %s.";
@VisibleForTesting
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
index e9c5296..07468c7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
@@ -373,6 +373,9 @@
UMBRELLA_HEADER,
WEAK_SDK_FRAMEWORK);
+ /** A white list of keys we support for strict-dependency / non-propagated items. */
+ static final ImmutableList<Key<?>> STRICT_DEPENDENCY_KEYS = ImmutableList.<Key<?>>of(INCLUDE);
+
/**
* Keys that should be kept as directItems. This is limited to a few keys that have larger
* performance implications when flattened in a transitive fashion and/or require non-transitive
@@ -1081,17 +1084,31 @@
return this;
}
+ private static <E> EvalException badDirectDependencyKeyError(Key<E> key) {
+ return new EvalException(
+ null,
+ String.format(
+ AppleSkylarkCommon.BAD_DIRECT_DEPENDENCY_KEY_ERROR, key.getSkylarkKeyName()));
+ }
+
/**
* Add all keys and values from the given provider, but propagate any normally-propagated items
* only to direct dependers of this ObjcProvider.
*/
- public Builder addAsDirectDeps(ObjcProvider provider) {
+ public Builder addAsDirectDeps(ObjcProvider provider) throws EvalException {
for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.items.entrySet()) {
- uncheckedAddTransitive(
- typeEntry.getKey(), typeEntry.getValue(), this.strictDependencyItems);
+ Key<?> key = typeEntry.getKey();
+ if (!ObjcProvider.STRICT_DEPENDENCY_KEYS.contains(key)) {
+ throw badDirectDependencyKeyError(key);
+ }
+ uncheckedAddTransitive(key, typeEntry.getValue(), this.strictDependencyItems);
}
for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.strictDependencyItems.entrySet()) {
- uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
+ Key<?> key = typeEntry.getKey();
+ if (!ObjcProvider.STRICT_DEPENDENCY_KEYS.contains(key)) {
+ throw badDirectDependencyKeyError(key);
+ }
+ uncheckedAddTransitive(key, typeEntry.getValue(), this.nonPropagatedItems);
}
return this;
}
@@ -1129,6 +1146,7 @@
* Add elements in toAdd, and do not propagate to dependents of this provider.
*/
public <E> Builder addAllNonPropagable(Key<E> key, Iterable<? extends E> toAdd) {
+ Preconditions.checkState(ObjcProvider.STRICT_DEPENDENCY_KEYS.contains(key));
uncheckedAddAll(key, toAdd, this.nonPropagatedItems);
return this;
}
@@ -1137,6 +1155,7 @@
* Add element toAdd, and propagate it only to direct dependents of this provider.
*/
public <E> Builder addForDirectDependents(Key<E> key, E toAdd) {
+ Preconditions.checkState(ObjcProvider.STRICT_DEPENDENCY_KEYS.contains(key));
uncheckedAddAll(key, ImmutableList.of(toAdd), this.strictDependencyItems);
return this;
}
@@ -1145,6 +1164,7 @@
* Add elements in toAdd, and propagate them only to direct dependents of this provider.
*/
public <E> Builder addAllForDirectDependents(Key<E> key, Iterable<? extends E> toAdd) {
+ Preconditions.checkState(ObjcProvider.STRICT_DEPENDENCY_KEYS.contains(key));
uncheckedAddAll(key, toAdd, this.strictDependencyItems);
return this;
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java
index 49b5019..3043343 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcSkylarkTest.java
@@ -1008,6 +1008,24 @@
}
@Test
+ public void testSkylarkStrictDepsWhitelist() throws Exception {
+ AssertionError e =
+ assertThrows(
+ AssertionError.class,
+ () ->
+ createObjcProviderSkylarkTarget(
+ " strict_defines = depset(['def1'])",
+ " strict_provider = apple_common.new_objc_provider\\",
+ "(define=strict_defines)",
+ " created_provider = apple_common.new_objc_provider\\",
+ "(direct_dep_providers=[strict_provider])",
+ " return [created_provider]"));
+ assertThat(e)
+ .hasMessageThat()
+ .contains(String.format(AppleSkylarkCommon.BAD_DIRECT_DEPENDENCY_KEY_ERROR, "define"));
+ }
+
+ @Test
public void testSkylarkCanCreateObjcProviderFromObjcProvider() throws Exception {
ConfiguredTarget skylarkTarget =
createObjcProviderSkylarkTarget(
