Make ObjcBundleLibrary use the "families" attribute.

Currently the ObjcBundleLibrary#create() function creates a
BundleSupport object with the default "iphone" target family.
An ObjcBundleLibrary is a descendant of a Bundle, which has
a "families" attribute, so the information is available.
Currently, ObjcBundleLibrary will unconditionally use "iphone"
as the target device family.

This commit updates ObjcBundleLibrary to use the "families"
attribute. It does this by making BundleSupport
control the "families" attribute, and then provide access
to it for the other classes that use it, like
ObjcBundleLibrary. The other classes are changed to
get their target device family information from their
BundleSupport objects.

--
Change-Id: I4a5be2849315dabfe7f2975c006a2092a56027af
Reviewed-on: https://bazel-review.googlesource.com/#/c/1260/3
MOS_MIGRATED_REVID=93456579

4 files changed, 44 insertions, 41 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
index e392ced916..56bc24669b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleSupport.java
@@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCASSETS_DIR
 
 import com.google.common.base.Optional;
 import com.google.common.base.Verify;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -28,16 +29,19 @@ import com.google.devtools.build.lib.analysis.actions.CommandLine;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.objc.ObjcActionsBuilder.ExtraActoolArgs;
+import com.google.devtools.build.lib.rules.objc.TargetDeviceFamily.InvalidFamilyNameException;
+import com.google.devtools.build.lib.rules.objc.TargetDeviceFamily.RepeatedFamilyNameException;
 import com.google.devtools.build.lib.rules.objc.XcodeProvider.Builder;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 
 /**
  * Support for generating iOS bundles which contain metadata (a plist file), assets, resources and
@@ -55,7 +59,6 @@ final class BundleSupport {
   }
 
   private final RuleContext ruleContext;
-  private final Set<TargetDeviceFamily> targetDeviceFamilies;
   private final ExtraActoolArgs extraActoolArgs;
   private final Bundling bundling;
   private final Attributes attributes;
@@ -100,28 +103,22 @@ final class BundleSupport {
    * Creates a new bundle support with no special {@code actool} arguments.
    *
    * @param ruleContext context this bundle is constructed in
-   * @param targetDeviceFamilies device families used in asset catalogue construction and storyboard
-   *     compilation
    * @param bundling bundle information as configured for this rule
    */
-  public BundleSupport(
-      RuleContext ruleContext, Set<TargetDeviceFamily> targetDeviceFamilies, Bundling bundling) {
-    this(ruleContext, targetDeviceFamilies, bundling, new ExtraActoolArgs());
+  public BundleSupport(RuleContext ruleContext, Bundling bundling) {
+    this(ruleContext, bundling, new ExtraActoolArgs());
   }
 
   /**
    * Creates a new bundle support.
    *
    * @param ruleContext context this bundle is constructed in
-   * @param targetDeviceFamilies device families used in asset catalogue construction and storyboard
-   *     compilation
    * @param bundling bundle information as configured for this rule
    * @param extraActoolArgs any additional parameters to be used for invoking {@code actool}
    */
-  public BundleSupport(RuleContext ruleContext, Set<TargetDeviceFamily> targetDeviceFamilies,
+  public BundleSupport(RuleContext ruleContext,
       Bundling bundling, ExtraActoolArgs extraActoolArgs) {
     this.ruleContext = ruleContext;
-    this.targetDeviceFamilies = targetDeviceFamilies;
     this.extraActoolArgs = extraActoolArgs;
     this.bundling = bundling;
     this.attributes = new Attributes(ruleContext);
@@ -197,6 +194,15 @@ final class BundleSupport {
     return this;
   }
 
+  /**
+   * Returns a set containing the {@link TargetDeviceFamily} values
+   * which this bundle is targeting. Returns an empty set for any
+   * invalid value of the target device families attribute.
+   */
+  ImmutableSet<TargetDeviceFamily> targetDeviceFamilies() {
+    return attributes.families();
+  }
+
   private void registerInterfaceBuilderActions(ObjcProvider objcProvider) {
     IntermediateArtifacts intermediateArtifacts =
         ObjcRuleClasses.intermediateArtifacts(ruleContext);
@@ -223,7 +229,7 @@ final class BundleSupport {
         .addPath(ObjcActionsBuilder.IBTOOL)
         .add("--minimum-deployment-target").add(bundling.getMinimumOsVersion());
 
-    for (TargetDeviceFamily targetDeviceFamily : targetDeviceFamilies) {
+    for (TargetDeviceFamily targetDeviceFamily : attributes.families()) {
       commandLine.add("--target-device").add(targetDeviceFamily.name().toLowerCase(Locale.US));
     }
 
@@ -350,7 +356,7 @@ final class BundleSupport {
         .addExecPath("--output-partial-info-plist", partialInfoPlist)
         .add("--minimum-deployment-target").add(bundling.getMinimumOsVersion());
 
-    for (TargetDeviceFamily targetDeviceFamily : targetDeviceFamilies) {
+    for (TargetDeviceFamily targetDeviceFamily : attributes.families()) {
       commandLine.add("--target-device").add(targetDeviceFamily.name().toLowerCase(Locale.US));
     }
 
@@ -399,6 +405,21 @@ final class BundleSupport {
     }
 
     /**
+     * Returns the value of the {@code families} attribute in a form
+     * that is more useful than a list of strings. Returns an empty
+     * set for any invalid {@code families} attribute value, including
+     * an empty list.
+     */
+    ImmutableSet<TargetDeviceFamily> families() {
+      List<String> rawFamilies = ruleContext.attributes().get("families", Type.STRING_LIST);
+      try {
+        return ImmutableSet.copyOf(TargetDeviceFamily.fromNamesInRule(rawFamilies));
+      } catch (InvalidFamilyNameException | RepeatedFamilyNameException e) {
+        return ImmutableSet.of();
+      }
+    }
+
+    /**
      * Returns the location of the ibtoolzip deploy jar.
      */
     Artifact ibtoolzipDeployJar() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java
index fcf9c315b4..e04c11f1a1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcBundleLibrary.java
@@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.NESTED_BUNDL
 import static com.google.devtools.build.lib.rules.objc.XcodeProductType.BUNDLE;
 
 import com.google.common.base.Optional;
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -27,6 +26,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes;
 
+
 /**
  * Implementation for {@code objc_bundle_library}.
  */
@@ -41,11 +41,8 @@ public class ObjcBundleLibrary implements RuleConfiguredTargetFactory {
 
     XcodeProvider.Builder xcodeProviderBuilder = new XcodeProvider.Builder();
     NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.stableOrder();
-    
-    // TODO(bazel-team): Figure out if the target device is important, and what to set it to. It may
-    // have to inherit this from the binary being built. As of this writing, this is only used for
-    // asset catalogs compilation (actool).
-    new BundleSupport(ruleContext, ImmutableSet.of(TargetDeviceFamily.IPHONE), bundling)
+
+    new BundleSupport(ruleContext, bundling)
         .registerActions(common.getObjcProvider())
         .validateResources(common.getObjcProvider())
         .addXcodeSettings(xcodeProviderBuilder);
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 024fbe1211..9aeb7acd27 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
@@ -849,6 +849,9 @@ public class ObjcRuleClasses {
         This is known as the <code>TARGETED_DEVICE_FAMILY</code> build setting
         in Xcode project files. It is a list of one or more of the strings
         <code>"iphone"</code> and <code>"ipad"</code>.
+
+        <p>By default this is set to <code>"iphone"</code>, if explicitly specified may not be
+        empty.</p>
         <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
         .add(attr("families", STRING_LIST)
              .value(ImmutableList.of(TargetDeviceFamily.IPHONE.getNameInRule())))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
index a525a026ee..9247461580 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ReleaseBundlingSupport.java
@@ -45,15 +45,12 @@ import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.rules.objc.ObjcActionsBuilder.ExtraActoolArgs;
-import com.google.devtools.build.lib.rules.objc.TargetDeviceFamily.InvalidFamilyNameException;
-import com.google.devtools.build.lib.rules.objc.TargetDeviceFamily.RepeatedFamilyNameException;
 import com.google.devtools.build.lib.shell.ShellUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.xcode.xcodegen.proto.XcodeGenProtos.XcodeprojBuildSetting;
 
 import java.util.List;
 import java.util.Map.Entry;
-import java.util.Set;
 
 import javax.annotation.Nullable;
 
@@ -96,7 +93,6 @@ public final class ReleaseBundlingSupport {
   private final Bundling bundling;
   private final ObjcProvider objcProvider;
   private final LinkedBinary linkedBinary;
-  private final ImmutableSet<TargetDeviceFamily> families;
   private final IntermediateArtifacts intermediateArtifacts;
 
   /**
@@ -139,11 +135,10 @@ public final class ReleaseBundlingSupport {
     this.attributes = new Attributes(ruleContext);
     this.ruleContext = ruleContext;
     this.objcProvider = objcProvider;
-    this.families = ImmutableSet.copyOf(attributes.families());
     this.intermediateArtifacts = ObjcRuleClasses.intermediateArtifacts(ruleContext);
     bundling = bundling(
         ruleContext, objcProvider, optionsProvider, bundleDirFormat, bundleMinimumOsVersion);
-    bundleSupport = new BundleSupport(ruleContext, families, bundling, extraActoolArgs());
+    bundleSupport = new BundleSupport(ruleContext, bundling, extraActoolArgs());
   }
 
   /**
@@ -168,7 +163,7 @@ public final class ReleaseBundlingSupport {
       }
     }
 
-    if (families.isEmpty()) {
+    if (bundleSupport.targetDeviceFamilies().isEmpty()) {
       ruleContext.attributeError("families", INVALID_FAMILIES_ERROR);
     }
 
@@ -215,7 +210,7 @@ public final class ReleaseBundlingSupport {
     registerEmbedLabelPlistAction();
 
     BundleMergeControlBytes bundleMergeControlBytes = new BundleMergeControlBytes(
-        bundling, maybeSignedIpa, objcConfiguration, families);
+        bundling, maybeSignedIpa, objcConfiguration, bundleSupport.targetDeviceFamilies());
     registerBundleMergeActions(
         maybeSignedIpa, bundling.getBundleContentArtifacts(), bundleMergeControlBytes);
 
@@ -478,6 +473,7 @@ public final class ReleaseBundlingSupport {
     }
 
     // Convert names to a sequence containing "1" and/or "2" for iPhone and iPad, respectively.
+    ImmutableSet<TargetDeviceFamily> families = bundleSupport.targetDeviceFamilies();
     Iterable<Integer> familyIndexes =
         families.isEmpty() ? ImmutableList.<Integer>of() : UI_DEVICE_FAMILY_VALUES.get(families);
     buildSettings.add(XcodeprojBuildSetting.newBuilder()
@@ -719,20 +715,6 @@ public final class ReleaseBundlingSupport {
       return ruleContext.getPrerequisiteArtifact(":default_provisioning_profile", Mode.TARGET);
     }
 
-    /**
-     * Returns the value of the {@code families} attribute in a form that is more useful than a list
-     * of strings. Returns an empty set for any invalid {@code families} attribute value, including
-     * an empty list.
-     */
-    Set<TargetDeviceFamily> families() {
-      List<String> rawFamilies = ruleContext.attributes().get("families", Type.STRING_LIST);
-      try {
-        return TargetDeviceFamily.fromNamesInRule(rawFamilies);
-      } catch (InvalidFamilyNameException | RepeatedFamilyNameException e) {
-        return ImmutableSet.of();
-      }
-    }
-
     @Nullable
     Artifact entitlements() {
       return ruleContext.getPrerequisiteArtifact("entitlements", Mode.TARGET);