apple_binary "dylibs" attribute depends on AppleDynamicFrameworkProvider

RELNOTES: None.
PiperOrigin-RevId: 159496805

4 files changed, 48 insertions, 21 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
index 0f954a49eb..78c29ee435 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
@@ -257,16 +257,22 @@ public class AppleBinary implements RuleConfiguredTargetFactory {
   }
 
   private static Iterable<ObjcProvider> getDylibProviders(RuleContext ruleContext) {
-    Iterable<ObjcProvider> dylibProviders =
-        ruleContext.getPrerequisites(DYLIBS_ATTR_NAME, Mode.TARGET, ObjcProvider.class);
+    ImmutableList.Builder<ObjcProvider> dylibProviders = ImmutableList.builder();
+    Iterable<AppleDynamicFrameworkProvider> frameworkProviders =
+        ruleContext.getPrerequisites(DYLIBS_ATTR_NAME, Mode.TARGET,
+            AppleDynamicFrameworkProvider.SKYLARK_CONSTRUCTOR.getKey(),
+            AppleDynamicFrameworkProvider.class);
+    for (AppleDynamicFrameworkProvider frameworkProvider : frameworkProviders) {
+      dylibProviders.add(frameworkProvider.getDepsObjcProvider());
+    }
 
     ObjcProvider bundleLoaderObjcProvider =
         ruleContext.getPrerequisite(BUNDLE_LOADER_ATTR_NAME, Mode.TARGET, ObjcProvider.class);
 
     if (bundleLoaderObjcProvider != null) {
-      dylibProviders = Iterables.concat(dylibProviders, ImmutableList.of(bundleLoaderObjcProvider));
+      dylibProviders.add(bundleLoaderObjcProvider);
     }
-    return dylibProviders;
+    return dylibProviders.build();
   }
 
   private static Iterable<ObjcProtoProvider> getDylibProtoProviders(RuleContext ruleContext) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicFrameworkProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicFrameworkProvider.java
index 84d4bbc3d0..848058efc4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicFrameworkProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicFrameworkProvider.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.packages.ClassObjectConstructor;
 import com.google.devtools.build.lib.packages.NativeClassObjectConstructor;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import javax.annotation.Nullable;
 
 /**
  * Provider containing information about an Apple dynamic framework. This provider contains:
@@ -57,24 +58,37 @@ public final class AppleDynamicFrameworkProvider extends SkylarkClassObject
 
   private final NestedSet<PathFragment> dynamicFrameworkDirs;
   private final NestedSet<Artifact> dynamicFrameworkFiles;
-  private final Artifact dylibBinary;
+  private final @Nullable Artifact dylibBinary;
   private final ObjcProvider depsObjcProvider;
 
-  public AppleDynamicFrameworkProvider(Artifact dylibBinary,
+  public AppleDynamicFrameworkProvider(@Nullable Artifact dylibBinary,
       ObjcProvider depsObjcProvider,
       NestedSet<PathFragment> dynamicFrameworkDirs,
       NestedSet<Artifact> dynamicFrameworkFiles) {
-    super(SKYLARK_CONSTRUCTOR, ImmutableMap.<String, Object>of(
-        DYLIB_BINARY_FIELD_NAME, dylibBinary,
-        FRAMEWORK_DIRS_FIELD_NAME, dynamicFrameworkDirs,
-        FRAMEWORK_FILES_FIELD_NAME, dynamicFrameworkFiles,
-        OBJC_PROVIDER_FIELD_NAME, depsObjcProvider));
+    super(SKYLARK_CONSTRUCTOR,
+        skylarkValueMap(dylibBinary, depsObjcProvider, dynamicFrameworkDirs,
+            dynamicFrameworkFiles));
     this.dylibBinary = dylibBinary;
     this.depsObjcProvider = depsObjcProvider;
     this.dynamicFrameworkDirs = dynamicFrameworkDirs;
     this.dynamicFrameworkFiles = dynamicFrameworkFiles;
   }
 
+  private static ImmutableMap<String, Object> skylarkValueMap(
+      @Nullable Artifact dylibBinary,
+      ObjcProvider depsObjcProvider,
+      NestedSet<PathFragment> dynamicFrameworkDirs,
+      NestedSet<Artifact> dynamicFrameworkFiles) {
+    ImmutableMap.Builder<String, Object> mapBuilder = ImmutableMap.<String, Object>builder()
+        .put(FRAMEWORK_DIRS_FIELD_NAME, dynamicFrameworkDirs)
+        .put(FRAMEWORK_FILES_FIELD_NAME, dynamicFrameworkFiles)
+        .put(OBJC_PROVIDER_FIELD_NAME, depsObjcProvider);
+    if (dylibBinary != null) {
+      mapBuilder.put(DYLIB_BINARY_FIELD_NAME, dylibBinary);
+    }
+    return mapBuilder.build();
+  }
+
   /**
    * Returns the framework path names used as link inputs in order to link against the dynamic
    * framework.
@@ -92,8 +106,10 @@ public final class AppleDynamicFrameworkProvider extends SkylarkClassObject
   }
 
   /**
-   * Returns the multi-architecture dylib binary of the dynamic framework.
+   * Returns the multi-architecture dylib binary of the dynamic framework. May return null if
+   * the rule providing the framework only specified framework imports.
    */
+  @Nullable
   public Artifact getAppleDylibBinary() {
     return dylibBinary;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcFramework.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcFramework.java
index e28b5e5636..90df6a21a2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcFramework.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcFramework.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.Builder;
 import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.vfs.PathFragment;
 
 /**
  * Implementation for the {@code objc_framework} rule.
@@ -58,9 +59,17 @@ public class ObjcFramework implements RuleConfiguredTargetFactory {
       ruleContext.attributeError("framework_imports", error);
     }
 
+    ObjcProvider objcProvider = commonBuilder.build().getObjcProvider();
+    Iterable<PathFragment> frameworkDirs =
+        ObjcCommon.uniqueContainers(frameworkImports, ObjcCommon.FRAMEWORK_CONTAINER_TYPE);
+    AppleDynamicFrameworkProvider frameworkProvider =
+        new AppleDynamicFrameworkProvider((Artifact) null, objcProvider,
+            NestedSetBuilder.<PathFragment>linkOrder().addAll(frameworkDirs).build(),
+            NestedSetBuilder.<Artifact>linkOrder().addAll(frameworkImports).build());
     NestedSet<Artifact> filesToBuild = NestedSetBuilder.emptySet(STABLE_ORDER);
     return ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild)
-        .addProvider(ObjcProvider.class, commonBuilder.build().getObjcProvider())
+        .addProvider(ObjcProvider.class, objcProvider)
+        .addNativeDeclaredProvider(frameworkProvider)
         .build();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
index ee6a6ce296..4fcf2d01ad 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java
@@ -49,6 +49,7 @@ import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
 import com.google.devtools.build.lib.rules.apple.AppleToolchain;
 import com.google.devtools.build.lib.rules.apple.AppleToolchain.RequiresXcodeConfigRule;
@@ -986,22 +987,17 @@ public class ObjcRuleClasses {
     @Override
     public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
       return builder
-          // TODO(b/32411441): Restrict the dylibs attribute to take only dylib dependencies.
-          // This will require refactoring ObjcProvider into alternate providers.
           /* <!-- #BLAZE_RULE($apple_dylib_depending_rule).ATTRIBUTE(dylibs) -->
           <p>A list of dynamic library targets to be linked against in this rule and included
           in the final bundle. Libraries which are transitive dependencies of any such dylibs will
           not be statically linked in this target (even if they are otherwise
           transitively depended on via the <code>deps</code> attribute) to avoid duplicate symbols.
-
-          <p>Please note: this attribute should only accept apple dynamic library targets, but
-          currently accepts many other objc or apple targets. This is a bug, so do not rely on this
-          behavior.
           <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
           .add(attr(DYLIBS_ATTR_NAME, LABEL_LIST)
               .direct_compile_time_input()
-              .mandatoryNativeProviders(
-                  ImmutableList.<Class<? extends TransitiveInfoProvider>>of(ObjcProvider.class))
+              .mandatoryProviders(ImmutableList.of(
+                  SkylarkProviderIdentifier.forKey(
+                      AppleDynamicFrameworkProvider.SKYLARK_CONSTRUCTOR.getKey())))
               .allowedFileTypes()
               .aspect(objcProtoAspect))
           .build();