Sign bundles for device if ios_multi_cpu contains device cpus. Introduces a restriction that prohibits using both simulator and device architectures for ios_multi_cpus in the same build. -- MOS_MIGRATED_REVID=89597247
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java index 67b4fb0..6b85809 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BinaryLinkingTargetFactory.java
@@ -87,7 +87,7 @@ .validateAttributes(); Optional<XcTestAppProvider> xcTestAppProvider; - Optional<RunfilesSupport> maybeRunfilesSupport = Optional.<RunfilesSupport>absent(); + Optional<RunfilesSupport> maybeRunfilesSupport = Optional.absent(); switch (hasReleaseBundlingSupport) { case YES: // TODO(bazel-team): Remove once all bundle users are migrated to ios_application. @@ -100,8 +100,10 @@ .addFilesToBuild(filesToBuild) .validateResources() .validateAttributes(); + + ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext); xcTestAppProvider = Optional.of(releaseBundlingSupport.xcTestAppProvider()); - if (ObjcRuleClasses.objcConfiguration(ruleContext).getPlatform() == Platform.SIMULATOR) { + if (objcConfiguration.getBundlingPlatform() == Platform.SIMULATOR) { Artifact runnerScript = intermediateArtifacts.runnerScript(); Artifact ipaFile = ruleContext.getImplicitOutputArtifact(ReleaseBundlingSupport.IPA); releaseBundlingSupport.registerGenerateRunnerScriptAction(runnerScript, ipaFile);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java index 38a1b33..d0be20c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BundleMergeControlBytes.java
@@ -69,7 +69,7 @@ // TODO(bazel-team): Add rule attribute for specifying targeted device family .setMinimumOsVersion(objcConfiguration.getMinimumOs()) .setSdkVersion(objcConfiguration.getIosSdkVersion()) - .setPlatform(objcConfiguration.getPlatform().name()) + .setPlatform(objcConfiguration.getBundlingPlatform().name()) .setBundleRoot(bundling.getBundleDir()); for (Artifact mergeZip : bundling.getMergeZips()) {
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 822f53b..676e273 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
@@ -14,7 +14,9 @@ package com.google.devtools.build.lib.rules.objc; +import static com.google.devtools.build.lib.rules.objc.ObjcProvider.ASSET_CATALOG; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STRINGS; +import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCASSETS_DIR; import com.google.common.base.Optional; import com.google.common.base.Verify; @@ -23,14 +25,17 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; +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.rules.objc.ObjcActionsBuilder.ExtraActoolArgs; 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.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -228,7 +233,7 @@ return ImmutableList.of( "-XD_MOMC_SDKROOT=" + IosSdkCommands.sdkDir(configuration), "-XD_MOMC_IOS_TARGET_VERSION=" + configuration.getMinimumOs(), - "-MOMC_PLATFORMS", configuration.getPlatform().getLowerCaseNameInPlist(), + "-MOMC_PLATFORMS", configuration.getBundlingPlatform().getLowerCaseNameInPlist(), "-XD_MOMC_TARGET_VERSION=10.6"); } @@ -288,18 +293,53 @@ return; } - ObjcActionsBuilder actionsBuilder = ObjcRuleClasses.actionsBuilder(ruleContext); - Artifact actoolPartialInfoplist = actoolPartialInfoplist(ruleContext, objcProvider).get(); - actionsBuilder.registerActoolzipAction( - new ObjcRuleClasses.Tools(ruleContext), - objcProvider, - actoolzipOutput.get(), - actoolPartialInfoplist, - extraActoolArgs, - targetDeviceFamilies); + Artifact zipOutput = actoolzipOutput.get(); + + // TODO(bazel-team): Do not use the deploy jar explicitly here. There is currently a bug where + // we cannot .setExecutable({java_binary target}) and set REQUIRES_DARWIN in the execution info. + // Note that below we set the archive root to the empty string. This means that the generated + // zip file will be rooted at the bundle root, and we have to prepend the bundle root to each + // entry when merging it with the final .ipa file. + ruleContext.registerAction( + ObjcActionsBuilder.spawnJavaOnDarwinActionBuilder(attributes.actoolzipDeployJar()) + .setMnemonic("AssetCatalogCompile") + .addTransitiveInputs(objcProvider.get(ASSET_CATALOG)) + .addOutput(zipOutput) + .addOutput(actoolPartialInfoplist) + .setCommandLine(actoolzipCommandLine( + objcProvider, + zipOutput, + actoolPartialInfoplist)) + .build(ruleContext)); } + private CommandLine actoolzipCommandLine( + final ObjcProvider provider, + final Artifact zipOutput, + final Artifact partialInfoPlist) { + ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext); + CustomCommandLine.Builder commandLine = CustomCommandLine.builder() + // The next three arguments are positional, i.e. they don't have flags before them. + .addPath(zipOutput.getExecPath()) + .add("") // archive root + .add(IosSdkCommands.ACTOOL_PATH) + + .add("--platform").add(objcConfiguration.getBundlingPlatform().getLowerCaseNameInPlist()) + .addExecPath("--output-partial-info-plist", partialInfoPlist) + .add("--minimum-deployment-target").add(objcConfiguration.getMinimumOs()); + + for (TargetDeviceFamily targetDeviceFamily : targetDeviceFamilies) { + commandLine.add("--target-device").add(targetDeviceFamily.name().toLowerCase(Locale.US)); + } + + return commandLine + .add(PathFragment.safePathStrings(provider.get(XCASSETS_DIR))) + .add(extraActoolArgs) + .build(); + } + + /** * Returns the artifact that is a plist file generated by an invocation of {@code actool} or * {@link Optional#absent()} if no asset catalogues are present in this target and its @@ -350,5 +390,12 @@ Artifact momczipDeployJar() { return ruleContext.getPrerequisiteArtifact("$momczip_deploy", Mode.HOST); } + + /** + * Returns the location of the actoolzip deploy jar. + */ + Artifact actoolzipDeployJar() { + return ruleContext.getPrerequisiteArtifact("$actoolzip_deploy", Mode.HOST); + } } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java index f703c4e..4b8c760 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosApplication.java
@@ -50,7 +50,8 @@ protected void configureTarget(RuleConfiguredTargetBuilder target, RuleContext ruleContext, ReleaseBundlingSupport releaseBundlingSupport) { // If this is an application built for the simulator, make it runnable. - if (ObjcRuleClasses.objcConfiguration(ruleContext).getPlatform() == Platform.SIMULATOR) { + ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext); + if (objcConfiguration.getBundlingPlatform() == Platform.SIMULATOR) { Artifact runnerScript = ObjcRuleClasses.intermediateArtifacts(ruleContext).runnerScript(); Artifact ipaFile = ruleContext.getImplicitOutputArtifact(ReleaseBundlingSupport.IPA); releaseBundlingSupport.registerGenerateRunnerScriptAction(runnerScript, ipaFile);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java index e694710..dc9f938 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IosSdkCommands.java
@@ -67,14 +67,17 @@ throw new UnsupportedOperationException("static-only"); } + private static String getPlatformPlistName(ObjcConfiguration configuration) { + return Platform.forArch(configuration.getIosCpu()).getNameInPlist(); + } + private static String platformDir(ObjcConfiguration configuration) { - return DEVELOPER_DIR + "/Platforms/" + configuration.getPlatform().getNameInPlist() - + ".platform"; + return DEVELOPER_DIR + "/Platforms/" + getPlatformPlistName(configuration) + ".platform"; } public static String sdkDir(ObjcConfiguration configuration) { return platformDir(configuration) + "/Developer/SDKs/" - + configuration.getPlatform().getNameInPlist() + configuration.getIosSdkVersion() + ".sdk"; + + getPlatformPlistName(configuration) + configuration.getIosSdkVersion() + ".sdk"; } public static String frameworkDir(ObjcConfiguration configuration) { @@ -92,7 +95,7 @@ public static List<String> commonLinkAndCompileArgsForClang( ObjcProvider provider, ObjcConfiguration configuration) { ImmutableList.Builder<String> builder = new ImmutableList.Builder<>(); - if (configuration.getPlatform() == Platform.SIMULATOR) { + if (Platform.forArch(configuration.getIosCpu()) == Platform.SIMULATOR) { builder.add("-mios-simulator-version-min=" + configuration.getMinimumOs()); } else { builder.add("-miphoneos-version-min=" + configuration.getMinimumOs()); @@ -125,7 +128,7 @@ } private static List<String> platformSpecificCompileArgsForClang(ObjcConfiguration configuration) { - switch (configuration.getPlatform()) { + switch (Platform.forArch(configuration.getIosCpu())) { case DEVICE: return ImmutableList.of(); case SIMULATOR:
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcActionsBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcActionsBuilder.java index 4b1868a..713e633 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcActionsBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcActionsBuilder.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.objc; import static com.google.devtools.build.lib.rules.objc.IosSdkCommands.BIN_DIR; -import static com.google.devtools.build.lib.rules.objc.ObjcProvider.ASSET_CATALOG; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.DEFINE; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FORCE_LOAD_LIBRARY; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.FRAMEWORK_DIR; @@ -28,14 +27,12 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_DYLIB; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.SDK_FRAMEWORK; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.WEAK_SDK_FRAMEWORK; -import static com.google.devtools.build.lib.rules.objc.ObjcProvider.XCASSETS_DIR; import com.google.common.base.Joiner; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.io.ByteSource; import com.google.devtools.build.lib.actions.Action; @@ -55,8 +52,6 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.List; -import java.util.Locale; -import java.util.Set; import javax.annotation.CheckReturnValue; @@ -278,62 +273,6 @@ } } - void registerActoolzipAction( - ObjcRuleClasses.Tools tools, - ObjcProvider provider, - Artifact zipOutput, - Artifact partialInfoPlist, - ExtraActoolArgs extraActoolArgs, - Set<TargetDeviceFamily> families) { - // TODO(bazel-team): Do not use the deploy jar explicitly here. There is currently a bug where - // we cannot .setExecutable({java_binary target}) and set REQUIRES_DARWIN in the execution info. - // Note that below we set the archive root to the empty string. This means that the generated - // zip file will be rooted at the bundle root, and we have to prepend the bundle root to each - // entry when merging it with the final .ipa file. - register(spawnJavaOnDarwinActionBuilder(tools.actoolzipDeployJar()) - .setMnemonic("AssetCatalogCompile") - .addTransitiveInputs(provider.get(ASSET_CATALOG)) - .addOutput(zipOutput) - .addOutput(partialInfoPlist) - .setCommandLine(actoolzipCommandLine( - objcConfiguration, - provider, - zipOutput, - partialInfoPlist, - extraActoolArgs, - ImmutableSet.copyOf(families))) - .build(context)); - } - - private static CommandLine actoolzipCommandLine( - final ObjcConfiguration objcConfiguration, - final ObjcProvider provider, - final Artifact zipOutput, - final Artifact partialInfoPlist, - final ExtraActoolArgs extraActoolArgs, - final ImmutableSet<TargetDeviceFamily> families) { - return new CommandLine() { - @Override - public Iterable<String> arguments() { - ImmutableList.Builder<String> args = new ImmutableList.Builder<String>() - // The next three arguments are positional, i.e. they don't have flags before them. - .add(zipOutput.getExecPathString()) - .add("") // archive root - .add(IosSdkCommands.ACTOOL_PATH) - .add("--platform") - .add(objcConfiguration.getPlatform().getLowerCaseNameInPlist()) - .add("--output-partial-info-plist").add(partialInfoPlist.getExecPathString()) - .add("--minimum-deployment-target").add(objcConfiguration.getMinimumOs()); - for (TargetDeviceFamily targetDeviceFamily : families) { - args.add("--target-device").add(targetDeviceFamily.name().toLowerCase(Locale.US)); - } - return args - .addAll(PathFragment.safePathStrings(provider.get(XCASSETS_DIR))) - .addAll(extraActoolArgs) - .build(); - } - }; - } private static final String FRAMEWORK_SUFFIX = ".framework";
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java index d5d1b98..379b5b3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
@@ -121,7 +121,24 @@ return iosCpu; } - public Platform getPlatform() { + /** + * Returns the platform of the configuration for the current bundle, based on configured + * architectures (for example, {@code i386} maps to {@link Platform#SIMULATOR}). + * + * <p>If {@link #getIosMultiCpus()} is set, returns {@link Platform#DEVICE} if any of the + * architectures matches it, otherwise returns the mapping for {@link #getIosCpu()}. + * + * <p>Note that this method should not be used to determine the platform for code compilation. + * Derive the platform from {@link #getIosCpu()} instead. + */ + // TODO(bazel-team): This method should be enabled to return multiple values once all call sites + // (in particular actool, bundlemerge, momc) have been upgraded to support multiple values. + public Platform getBundlingPlatform() { + for (String architecture : getIosMultiCpus()) { + if (Platform.forArch(architecture) == Platform.DEVICE) { + return Platform.DEVICE; + } + } return Platform.forArch(getIosCpu()); } @@ -238,5 +255,17 @@ reporter.handle(Event.error( "--objc_generate_debug_symbols is not supported when --ios_multi_cpus is set")); } + + // TODO(bazel-team): Remove this constraint once getBundlingPlatform can return multiple values. + Platform platform = null; + for (String architecture : iosMultiCpus) { + if (platform == null) { + platform = Platform.forArch(architecture); + } else if (platform != Platform.forArch(architecture)) { + reporter.handle(Event.error( + String.format("--ios_multi_cpus does not currently allow values for both simulator and " + + "device builds but was %s", iosMultiCpus))); + } + } } }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java index 011e99a..b8a5996 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfigurationLoader.java
@@ -47,7 +47,7 @@ } Label defaultProvisioningProfileLabel = null; - if (Platform.forArch(objcOptions.iosCpu) == Platform.DEVICE) { + if (getPlatform(objcOptions) == Platform.DEVICE) { defaultProvisioningProfileLabel = forceLoad(env, "//tools/objc:default_provisioning_profile"); } @@ -60,11 +60,19 @@ return ObjcConfiguration.class; } + private Platform getPlatform(ObjcCommandLineOptions objcOptions) { + for (String architecture : objcOptions.iosMultiCpus) { + if (Platform.forArch(architecture) == Platform.DEVICE) { + return Platform.DEVICE; + } + } + return Platform.forArch(objcOptions.iosCpu); + } + private static Label forceLoad(ConfigurationEnvironment env, String target) throws InvalidConfigurationException { - Label label = null; try { - label = Label.parseAbsolute(target); + Label label = Label.parseAbsolute(target); env.getTarget(label); return label; } catch (Label.SyntaxException | NoSuchPackageException | NoSuchTargetException e) {
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 20567cb..88cbe70 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
@@ -845,7 +845,7 @@ public Label getDefault(Rule rule, BuildConfiguration configuration) { ObjcConfiguration objcConfiguration = configuration.getFragment(ObjcConfiguration.class); - if (objcConfiguration.getPlatform() != Platform.DEVICE) { + if (objcConfiguration.getBundlingPlatform() != Platform.DEVICE) { return null; } if (rule.isAttributeValueExplicitlySpecified("provisioning_profile")) { @@ -950,10 +950,6 @@ this.ruleContext = Preconditions.checkNotNull(ruleContext); } - Artifact actoolzipDeployJar() { - return ruleContext.getPrerequisiteArtifact("$actoolzip_deploy", Mode.HOST); - } - FilesToRunProvider xcodegen() { return ruleContext.getExecutablePrerequisite("$xcodegen", Mode.HOST); }
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 2086298..20e1bc9 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
@@ -224,7 +224,7 @@ Artifact ipaOutput = ruleContext.getImplicitOutputArtifact(IPA); Artifact maybeSignedIpa; - if (objcConfiguration.getPlatform() == Platform.SIMULATOR) { + if (objcConfiguration.getBundlingPlatform() == Platform.SIMULATOR) { maybeSignedIpa = ipaOutput; } else if (attributes.provisioningProfile() == null) { throw new IllegalStateException(DEVICE_NO_PROVISIONING_PROFILE); @@ -371,7 +371,7 @@ String bundleDirFormat) { ImmutableList<BundleableFile> extraBundleFiles; ObjcConfiguration objcConfiguration = ObjcRuleClasses.objcConfiguration(ruleContext); - if (objcConfiguration.getPlatform() == Platform.DEVICE) { + if (objcConfiguration.getBundlingPlatform() == Platform.DEVICE) { extraBundleFiles = ImmutableList.of(new BundleableFile( new Attributes(ruleContext).provisioningProfile(), PROVISIONING_PROFILE_BUNDLE_FILE));