Adds support for strict dependency management. This is done by filtering a list of strict deps keys from being propagated more than one level in the dependency graph. Values added to those keys from dependency providers are routed into the non_propagated_items map, preventing them from being propagated again.

--
MOS_MIGRATED_REVID=120724767

2 files changed, 133 insertions, 41 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
index 7f80a960b6..6464a9968b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
@@ -207,7 +207,7 @@ public final class ObjcCommon {
     public Iterable<String> linkopts() {
       return ruleContext.getTokenizedStringListAttr("linkopts");
     }
-    
+
     /**
      * The clang module maps of direct dependencies of this rule. These are needed to generate
      * this rule's module map.
@@ -303,6 +303,7 @@ public final class ObjcCommon {
     private Iterable<ObjcProvider> directDepObjcProviders = ImmutableList.of();
     private Iterable<String> defines = ImmutableList.of();
     private Iterable<PathFragment> userHeaderSearchPaths = ImmutableList.of();
+    private Iterable<PathFragment> directDependencyHeaderSearchPaths = ImmutableList.of();
     private IntermediateArtifacts intermediateArtifacts;
     private boolean alwayslink;
     private boolean hasModuleMap;
@@ -384,6 +385,17 @@ public final class ObjcCommon {
       return this;
     }
 
+    /**
+     * Adds header search paths that will only be visible by strict dependents of the provider.
+     */
+    public Builder addDirectDependencyHeaderSearchPaths(
+        Iterable<PathFragment> directDependencyHeaderSearchPaths) {
+      this.directDependencyHeaderSearchPaths =
+          Iterables.concat(
+              this.directDependencyHeaderSearchPaths, directDependencyHeaderSearchPaths);
+      return this;
+    }
+
     public Builder addDefines(Iterable<String> defines) {
       this.defines = Iterables.concat(this.defines, defines);
       return this;
@@ -471,20 +483,24 @@ public final class ObjcCommon {
     ObjcCommon build() {
       Iterable<BundleableFile> bundleImports = BundleableFile.bundleImportsFromRule(context);
 
-      ObjcProvider.Builder objcProvider = new ObjcProvider.Builder()
-          .addAll(IMPORTED_LIBRARY, extraImportLibraries)
-          .addAll(BUNDLE_FILE, bundleImports)
-          .addAll(BUNDLE_IMPORT_DIR,
-              uniqueContainers(BundleableFile.toArtifacts(bundleImports), BUNDLE_CONTAINER_TYPE))
-          .addAll(SDK_FRAMEWORK, extraSdkFrameworks)
-          .addAll(WEAK_SDK_FRAMEWORK, extraWeakSdkFrameworks)
-          .addAll(SDK_DYLIB, extraSdkDylibs)
-          .addAll(FRAMEWORK_FILE, frameworkImports)
-          .addAll(FRAMEWORK_DIR, uniqueContainers(frameworkImports, FRAMEWORK_CONTAINER_TYPE))
-          .addAll(INCLUDE, userHeaderSearchPaths)
-          .addAll(DEFINE, defines)
-          .addTransitiveAndPropagate(depObjcProviders)
-          .addTransitiveWithoutPropagating(directDepObjcProviders);
+      ObjcProvider.Builder objcProvider =
+          new ObjcProvider.Builder()
+              .addAll(IMPORTED_LIBRARY, extraImportLibraries)
+              .addAll(BUNDLE_FILE, bundleImports)
+              .addAll(
+                  BUNDLE_IMPORT_DIR,
+                  uniqueContainers(
+                      BundleableFile.toArtifacts(bundleImports), BUNDLE_CONTAINER_TYPE))
+              .addAll(SDK_FRAMEWORK, extraSdkFrameworks)
+              .addAll(WEAK_SDK_FRAMEWORK, extraWeakSdkFrameworks)
+              .addAll(SDK_DYLIB, extraSdkDylibs)
+              .addAll(FRAMEWORK_FILE, frameworkImports)
+              .addAll(FRAMEWORK_DIR, uniqueContainers(frameworkImports, FRAMEWORK_CONTAINER_TYPE))
+              .addAll(INCLUDE, userHeaderSearchPaths)
+              .addAllForDirectDependents(INCLUDE, directDependencyHeaderSearchPaths)
+              .addAll(DEFINE, defines)
+              .addTransitiveAndPropagate(depObjcProviders)
+              .addTransitiveWithoutPropagating(directDepObjcProviders);
 
       for (CppCompilationContext headerProvider : depCcHeaderProviders) {
         objcProvider.addTransitiveAndPropagate(HEADER, headerProvider.getDeclaredIncludeSrcs());
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 5a33f69e30..91f5c1fca2 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
@@ -55,13 +55,13 @@ public final class ObjcProvider implements TransitiveInfoProvider {
   /**
    * The name skylark dependents can use to export a native objc provider to depending native
    * rules.
-   * 
+   *
    * <p>This constant must be different from OBJC_SKYLARK_PROVIDER_NAME to prevent skylark rules
-   * from automatically exporting the ObjcProvider provided to them by dependants.  This can
+   * from automatically exporting the ObjcProvider provided to them by dependents.  This can
    * lead to duplicate symbol linker errors.
    */
   public static final String OBJC_SKYLARK_PROVIDER_TO_EXPORT_NAME = "objc_export";
-  
+
   /**
    * Represents one of the things this provider can provide transitively. Things are provided as
    * {@link NestedSet}s of type E.
@@ -315,7 +315,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
     /**
      * Indicates that C++ (or Objective-C++) is used in any source file. This affects how the linker
      * is invoked.
-    */
+     */
     USES_CPP,
 
     /**
@@ -328,23 +328,28 @@ public final class ObjcProvider implements TransitiveInfoProvider {
      * Indicates that the resulting bundle will have embedded frameworks. This affects linking step.
      */
     USES_FRAMEWORKS,
-    
+
     /**
      * Indicates that watch os 1 extension is present in the bundle.
      */
-    HAS_WATCH1_EXTENSION
+    HAS_WATCH1_EXTENSION,
   }
 
   private final ImmutableMap<Key<?>, NestedSet<?>> items;
 
-  // Items which should be passed to direct dependers, but not transitive dependers.
+  // Items which should not be propagated to dependents.
   private final ImmutableMap<Key<?>, NestedSet<?>> nonPropagatedItems;
 
+  // Items which should be passed to strictly direct dependers, but not transitive dependers.
+  private final ImmutableMap<Key<?>, NestedSet<?>> strictDependencyItems;
+
   private ObjcProvider(
       ImmutableMap<Key<?>, NestedSet<?>> items,
-      ImmutableMap<Key<?>, NestedSet<?>> nonPropagatedItems) {
+      ImmutableMap<Key<?>, NestedSet<?>> nonPropagatedItems,
+      ImmutableMap<Key<?>, NestedSet<?>> strictDependencyItems) {
     this.items = Preconditions.checkNotNull(items);
     this.nonPropagatedItems = Preconditions.checkNotNull(nonPropagatedItems);
+    this.strictDependencyItems = Preconditions.checkNotNull(strictDependencyItems);
   }
 
   /**
@@ -354,6 +359,9 @@ public final class ObjcProvider implements TransitiveInfoProvider {
   public <E> NestedSet<E> get(Key<E> key) {
     Preconditions.checkNotNull(key);
     NestedSetBuilder<E> builder = new NestedSetBuilder<>(key.order);
+    if (strictDependencyItems.containsKey(key)) {
+      builder.addTransitive((NestedSet<E>) strictDependencyItems.get(key));
+    }
     if (nonPropagatedItems.containsKey(key)) {
       builder.addTransitive((NestedSet<E>) nonPropagatedItems.get(key));
     }
@@ -389,7 +397,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
     }
     return new SkylarkClassObject(providerBuilder.build(), "No such attribute '%s'");
   }
- 
+
   /**
    * Returns an {@code ObjcProvider} from a given skylark provider.  For each candidate key
    * in the ObjcProvider, will check the given skylark provider to see if that key is represented
@@ -406,7 +414,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
     }
     return builder.build();
   }
-  
+
   /**
    * A builder for this context with an API that is optimized for collecting information from
    * several transitive dependencies.
@@ -414,6 +422,7 @@ public final class ObjcProvider implements TransitiveInfoProvider {
   public static final class Builder {
     private final Map<Key<?>, NestedSetBuilder<?>> items = new HashMap<>();
     private final Map<Key<?>, NestedSetBuilder<?>> nonPropagatedItems = new HashMap<>();
+    private final Map<Key<?>, NestedSetBuilder<?>> strictDependencyItems = new HashMap<>();
 
     private static void maybeAddEmptyBuilder(Map<Key<?>, NestedSetBuilder<?>> set, Key<?> key) {
       if (!set.containsKey(key)) {
@@ -435,22 +444,27 @@ public final class ObjcProvider implements TransitiveInfoProvider {
     }
 
     /**
-     * Adds elements in items, and propagate them to any (transitive) dependers on this
+     * Add all elements from providers, and propagate them to any (transitive) dependers on this
      * ObjcProvider.
      */
-    public <E> Builder addTransitiveAndPropagate(Key<E> key, NestedSet<E> items) {
-      uncheckedAddTransitive(key, items, this.items);
+    public Builder addTransitiveAndPropagate(Iterable<ObjcProvider> providers) {
+      for (ObjcProvider provider : providers) {
+        addTransitiveAndPropagate(provider);
+      }
       return this;
     }
 
     /**
-     * Add all elements from provider, and propagate them to any (transitive) dependers on this
-     * ObjcProvider.
+     * Add all keys and values from provider, and propagate them to any (transitive) dependers on
+     * this ObjcProvider.
      */
     public Builder addTransitiveAndPropagate(ObjcProvider provider) {
       for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.items.entrySet()) {
         uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.items);
       }
+      for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.strictDependencyItems.entrySet()) {
+        uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
+      }
       return this;
     }
 
@@ -458,19 +472,23 @@ public final class ObjcProvider implements TransitiveInfoProvider {
      * Add all elements from a single key of the given provider, and propagate them to any
      * (transitive) dependers on this ObjcProvider.
      */
-    public <E> Builder addTransitiveAndPropagate(Key<E> key, ObjcProvider provider) {
-      addTransitiveAndPropagate(key, provider.get(key));
+    public Builder addTransitiveAndPropagate(Key key, ObjcProvider provider) {
+      if (provider.items.containsKey(key)) {
+        uncheckedAddTransitive(key, provider.items.get(key), this.items);
+      }
+      if (provider.strictDependencyItems.containsKey(key)) {
+        uncheckedAddTransitive(
+            key, provider.strictDependencyItems.get(key), this.nonPropagatedItems);
+      }
       return this;
     }
 
     /**
-     * Add all elements from providers, and propagate them to any (transitive) dependers on this
+     * Adds elements in items, and propagate them to any (transitive) dependers on this
      * ObjcProvider.
      */
-    public Builder addTransitiveAndPropagate(Iterable<ObjcProvider> providers) {
-      for (ObjcProvider provider : providers) {
-        addTransitiveAndPropagate(provider);
-      }
+    public <E> Builder addTransitiveAndPropagate(Key<E> key, NestedSet<E> items) {
+      uncheckedAddTransitive(key, items, this.items);
       return this;
     }
 
@@ -481,14 +499,52 @@ public final class ObjcProvider implements TransitiveInfoProvider {
      */
     public Builder addTransitiveWithoutPropagating(Iterable<ObjcProvider> providers) {
       for (ObjcProvider provider : providers) {
-        for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.items.entrySet()) {
-          uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
-        }
+        addTransitiveWithoutPropagating(provider);
+      }
+      return this;
+    }
+
+    /**
+     * Add all keys and values from provider, without propagating them to any (transitive) dependers
+     * on this ObjcProvider. These elements will be exposed to {@link #get(Key)} calls, but not to
+     * any ObjcProviders which add this provider to themselves.
+     */
+    public Builder addTransitiveWithoutPropagating(ObjcProvider provider) {
+      for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.items.entrySet()) {
+        uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
+      }
+      for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.strictDependencyItems.entrySet()) {
+        uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
       }
       return this;
     }
 
     /**
+     * Add a single key from provider, without propagating them to any (transitive) dependers
+     * on this ObjcProvider. These elements will be exposed to {@link #get(Key)} calls, but not to
+     * any ObjcProviders which add this provider to themselves.
+     */
+    public Builder addTransitiveWithoutPropagating(Key key, ObjcProvider provider) {
+      if (provider.items.containsKey(key)) {
+        uncheckedAddTransitive(key, provider.items.get(key), this.nonPropagatedItems);
+      }
+      if (provider.strictDependencyItems.containsKey(key)) {
+        uncheckedAddTransitive(
+            key, provider.strictDependencyItems.get(key), this.nonPropagatedItems);
+      }
+      return this;
+    }
+
+    /**
+     * Adds elements in items, without propagating them to any (transitive) dependers on this
+     * ObjcProvider.
+     */
+    public <E> Builder addTransitiveWithoutPropagating(Key<E> key, NestedSet<E> items) {
+      uncheckedAddTransitive(key, items, this.nonPropagatedItems);
+      return this;
+    }
+
+    /**
      * Add element, and propagate it to any (transitive) dependers on this ObjcProvider.
      */
     public <E> Builder add(Key<E> key, E toAdd) {
@@ -504,6 +560,22 @@ public final class ObjcProvider implements TransitiveInfoProvider {
       return this;
     }
 
+    /**
+     * Add element toAdd, and propagate it only to direct dependents of this provider.
+     */
+    public <E> Builder addForDirectDependents(Key<E> key, E toAdd) {
+      uncheckedAddAll(key, ImmutableList.of(toAdd), this.strictDependencyItems);
+      return this;
+    }
+
+    /**
+     * 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) {
+      uncheckedAddAll(key, toAdd, this.strictDependencyItems);
+      return this;
+    }
+
     public ObjcProvider build() {
       ImmutableMap.Builder<Key<?>, NestedSet<?>> propagated = new ImmutableMap.Builder<>();
       for (Map.Entry<Key<?>, NestedSetBuilder<?>> typeEntry : items.entrySet()) {
@@ -513,7 +585,11 @@ public final class ObjcProvider implements TransitiveInfoProvider {
       for (Map.Entry<Key<?>, NestedSetBuilder<?>> typeEntry : nonPropagatedItems.entrySet()) {
         nonPropagated.put(typeEntry.getKey(), typeEntry.getValue().build());
       }
-      return new ObjcProvider(propagated.build(), nonPropagated.build());
+      ImmutableMap.Builder<Key<?>, NestedSet<?>> strictDependency = new ImmutableMap.Builder<>();
+      for (Map.Entry<Key<?>, NestedSetBuilder<?>> typeEntry : strictDependencyItems.entrySet()) {
+        strictDependency.put(typeEntry.getKey(), typeEntry.getValue().build());
+      }
+      return new ObjcProvider(propagated.build(), nonPropagated.build(), strictDependency.build());
     }
   }
 }