Add support for bundle binaries in apple_binary. -- MOS_MIGRATED_REVID=140094935
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 643ba0f..112daa5 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
@@ -14,10 +14,17 @@ package com.google.devtools.build.lib.rules.objc; +import static com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode.TARGET; import static com.google.devtools.build.lib.rules.objc.ObjcProvider.MULTI_ARCH_LINKED_BINARIES; import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.DylibDependingRule.DYLIBS_ATTR_NAME; +import static com.google.devtools.build.lib.syntax.Type.STRING; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Functions; +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.Iterables; 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; @@ -40,6 +47,39 @@ */ public class AppleBinary implements RuleConfiguredTargetFactory { + // TODO(b/33077308): Expand into DYLIB when apple_dynamic_library is removed. + enum BinaryType { + EXECUTABLE, BUNDLE; + + @Override + public String toString() { + return name().toLowerCase(); + } + + /** + * Returns the {@link BinaryType} with given name (case insensitive). + * + * @throws IllegalArgumentException if the name does not match a valid platform type. + */ + public static BinaryType fromString(String name) { + for (BinaryType binaryType : BinaryType.values()) { + if (name.equalsIgnoreCase(binaryType.toString())) { + return binaryType; + } + } + throw new IllegalArgumentException(String.format("Unsupported binary type \"%s\"", name)); + } + + /** Returns the enum values as a list of strings for validation. */ + static Iterable<String> getValues() { + return Iterables.transform(ImmutableList.copyOf(values()), Functions.toStringFunction()); + } + } + + @VisibleForTesting + static final String BUNDLE_LOADER_NOT_IN_BUNDLE_ERROR = + "Can only use bundle_loader when binary_type is bundle."; + @Override public final ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException { @@ -62,8 +102,14 @@ Map<BuildConfiguration, ObjcCommon> objcCommonByDepConfiguration = multiArchBinarySupport.objcCommonByDepConfiguration(childConfigurations, configToDepsCollectionMap, configurationToNonPropagatedObjcMap, dylibProviders); - multiArchBinarySupport.registerActions(platform, new ExtraLinkArgs(), - objcCommonByDepConfiguration, configToDepsCollectionMap, outputArtifact); + + multiArchBinarySupport.registerActions( + platform, + getExtraLinkArgs(ruleContext), + getExtraLinkInputs(ruleContext), + objcCommonByDepConfiguration, + configToDepsCollectionMap, + outputArtifact); NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.<Artifact>stableOrder().add(outputArtifact); @@ -80,6 +126,45 @@ return targetBuilder.build(); } + private ExtraLinkArgs getExtraLinkArgs(RuleContext ruleContext) throws RuleErrorException { + String binaryTypeString = ruleContext + .attributes() + .get(AppleBinaryRule.BINARY_TYPE_ATTR, STRING); + BinaryType binaryType = BinaryType.fromString(binaryTypeString); + + ImmutableList.Builder<String> extraLinkArgs = new ImmutableList.Builder<>(); + + boolean didProvideBundleLoader = + ruleContext + .attributes() + .isAttributeValueExplicitlySpecified(AppleBinaryRule.BUNDLE_LOADER_ATTR); + + if (didProvideBundleLoader && binaryType != BinaryType.BUNDLE) { + ruleContext.throwWithRuleError(BUNDLE_LOADER_NOT_IN_BUNDLE_ERROR); + } + + switch(binaryType) { + case EXECUTABLE: break; + case BUNDLE: { + extraLinkArgs.add("-bundle"); + if (didProvideBundleLoader) { + Artifact bundleLoader = + ruleContext.getPrerequisiteArtifact(AppleBinaryRule.BUNDLE_LOADER_ATTR, TARGET); + extraLinkArgs.add("-bundle_loader " + bundleLoader.getExecPathString()); + } + break; + } + } + + return new ExtraLinkArgs(extraLinkArgs.build()); + } + + private Iterable<Artifact> getExtraLinkInputs(RuleContext ruleContext) { + return Optional.fromNullable( + ruleContext.getPrerequisiteArtifact(AppleBinaryRule.BUNDLE_LOADER_ATTR, TARGET)) + .asSet(); + } + private Set<BuildConfiguration> getChildConfigurations(RuleContext ruleContext) { // This is currently a hack to obtain all child configurations regardless of the attribute // values of this rule -- this rule does not currently use the actual info provided by
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinaryRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinaryRule.java index 1075547..e62eb4a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinaryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinaryRule.java
@@ -15,12 +15,15 @@ package com.google.devtools.build.lib.rules.objc; import static com.google.devtools.build.lib.packages.Attribute.attr; +import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.ImplicitOutputsFunction.fromTemplates; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; +import static com.google.devtools.build.lib.syntax.Type.STRING; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.RuleClass; @@ -32,12 +35,41 @@ */ public class AppleBinaryRule implements RuleDefinition { + public static final String BINARY_TYPE_ATTR = "binary_type"; + public static final String BUNDLE_LOADER_ATTR = "bundle_loader"; + /** * Template for the fat binary output (using Apple's "lipo" tool to combine binaries of * multiple architectures). */ private static final SafeImplicitOutputsFunction LIPOBIN = fromTemplates("%{name}_lipobin"); + /** + * There are 3 classes of fully linked binaries in Mach: executable, dynamic library and bundle. + * + * <p>The executable is the binary that can be run directly by the operating system. It implements + * implements the main method that is the entry point to the program. In Apple apps, they are + * usually distributed in .app bundles, which are folders that contain the executable along with + * required resources to run. + * + * <p>Dynamic libraries are binaries meant to be loaded at load time (when the operating system is + * loading the binary into memory), and they _cant'_ be unloaded. This is a great way to reduce + * binary size of executables by providing a dynamic library that groups common functionality into + * one dynamic library that can then be loaded by multiple executables. They are usually + * distributed in frameworks, which are .framework bundles that contain the dylib as well as well + * as required resources to run. + * + * <p>Bundles are binaries that can be loaded by other binaries at runtime, and they can't be + * directly executed by the operating system. When linking, a bundle_loader binary may be passed + * which signals the linker on where to look for unimplemented symbols, basically declaring that + * the bundle should be loaded by that binary. Bundle binaries are usually found in Plugins, and + * one common use case is tests. Tests are bundled into an .xctest bundle which contains the tests + * binary along with required resources. The test bundle is then loaded and run during test + * execution. + * + * <p>apple_binary for now supports both the executable and the bundle types, and this is + * configurable by the "binary_type" attribute described below. + */ @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { MultiArchSplitTransitionProvider splitTransitionProvider = @@ -45,8 +77,42 @@ return builder .requiresConfigurationFragments( ObjcConfiguration.class, J2ObjcConfiguration.class, AppleConfiguration.class) - .add(attr("$is_executable", BOOLEAN).value(true) - .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")) + .add( + attr("$is_executable", BOOLEAN) + .value(true) + .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")) + /* <!-- #BLAZE_RULE(apple_binary).ATTRIBUTE(binary_type) --> + The type of binary that this target should build. + + Options are: + <ul> + <li> + <code>executable</code> (default): the output binary is an executable and must implement + the main() function. + </li> + <li> + <code>bundle</code>: the output binary is a loadable bundle that may be loaded at + runtime. When building a bundle, you may also pass a bundle_loader binary that contains + symbols referenced but not implemented in the bundle. + </li> + </ul> + <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ + .add( + attr(BINARY_TYPE_ATTR, STRING) + .value(AppleBinary.BinaryType.EXECUTABLE.toString()) + .allowedValues(new AllowedValueSet(AppleBinary.BinaryType.getValues()))) + /* <!-- #BLAZE_RULE(apple_binary).ATTRIBUTE(bundle_loader) --> + The bundle loader to be used when linking this bundle. Can only be set when binary_type is + "bundle". This bundle loader may contain symbols that were not linked into the bundle to + reduce binary size. + <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ + // TODO(blaze-team): Change into requesting specific providers when they are actually + // provided. + .add( + attr(BUNDLE_LOADER_ATTR, LABEL) + .allowedRuleClasses("apple_binary") + .legacyAllowAnyFileType() + .singleArtifact()) .override(builder.copy("deps").cfg(splitTransitionProvider)) .override(builder.copy("non_propagated_deps").cfg(splitTransitionProvider)) /*<!-- #BLAZE_RULE(apple_binary).IMPLICIT_OUTPUTS --> @@ -55,8 +121,7 @@ binary. All transitive dependencies and <code>srcs</code> are linked.</li> </ul> <!-- #END_BLAZE_RULE.IMPLICIT_OUTPUTS -->*/ - .setImplicitOutputsFunction( - ImplicitOutputsFunction.fromFunctions(LIPOBIN)) + .setImplicitOutputsFunction(ImplicitOutputsFunction.fromFunctions(LIPOBIN)) .build(); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java index 69ec0ac..99a6c02 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibrary.java
@@ -18,6 +18,7 @@ import static com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.DylibDependingRule.DYLIBS_ATTR_NAME; import com.google.common.collect.ImmutableListMultimap; +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; @@ -62,8 +63,13 @@ Map<BuildConfiguration, ObjcCommon> objcCommonByDepConfiguration = multiArchBinarySupport.objcCommonByDepConfiguration(childConfigurations, configToDepsCollectionMap, configurationToNonPropagatedObjcMap, dylibProviders); - multiArchBinarySupport.registerActions(platform, new ExtraLinkArgs("-dynamiclib"), - objcCommonByDepConfiguration, configToDepsCollectionMap, outputArtifact); + multiArchBinarySupport.registerActions( + platform, + new ExtraLinkArgs("-dynamiclib"), + ImmutableSet.<Artifact>of(), + objcCommonByDepConfiguration, + configToDepsCollectionMap, + outputArtifact); NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.<Artifact>stableOrder()
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index a09e626..efa6b1d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -207,6 +207,10 @@ ExtraLinkArgs(String... args) { super(args); } + + ExtraLinkArgs(Iterable<String> args) { + super(args); + } } /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java index 6e6cf53..a127bc5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java
@@ -52,6 +52,7 @@ * @param platform the platform for which the binary is targeted * @param extraLinkArgs the extra linker args to add to link actions linking single-architecture * binaries together + * @param extraLinkInputs the extra linker inputs to be made available during link actions * @param configurationToObjcCommon a map from from dependency configuration to the * {@link ObjcCommon} which comprises all information about the dependencies in that * configuration. Can be obtained via {@link #objcCommonByDepConfiguration} @@ -62,8 +63,10 @@ * this support * @throws RuleErrorException if there are attribute errors in the current rule context */ - public void registerActions(Platform platform, + public void registerActions( + Platform platform, ExtraLinkArgs extraLinkArgs, + Iterable<Artifact> extraLinkInputs, Map<BuildConfiguration, ObjcCommon> configurationToObjcCommon, ImmutableListMultimap<BuildConfiguration, TransitiveInfoCollection> configToDepsCollectionMap, Artifact outputLipoBinary) @@ -101,7 +104,7 @@ j2ObjcMappingFileProvider, j2ObjcEntryClassProvider, extraLinkArgs, - ImmutableList.<Artifact>of(), + extraLinkInputs, DsymOutputType.APP) .validateAttributes(); ruleContext.assertNoErrors();