"deps" attribute avoids linking libraries transitively included in "dylibs" attribute This prevents duplicate symbol errors for objects that would otherwise be linked both in the application binary and a dylib the binary is linked against. -- PiperOrigin-RevId: 141478238 MOS_MIGRATED_REVID=141478238
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 112daa5..4fc59ea 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
@@ -99,15 +99,17 @@ ObjcRuleClasses.intermediateArtifacts(ruleContext).combinedArchitectureBinary(); MultiArchBinarySupport multiArchBinarySupport = new MultiArchBinarySupport(ruleContext); - Map<BuildConfiguration, ObjcCommon> objcCommonByDepConfiguration = - multiArchBinarySupport.objcCommonByDepConfiguration(childConfigurations, - configToDepsCollectionMap, configurationToNonPropagatedObjcMap, dylibProviders); + + Map<BuildConfiguration, ObjcProvider> objcProviderByDepConfiguration = + multiArchBinarySupport.objcProviderByDepConfiguration(childConfigurations, + configToDepsCollectionMap, configurationToNonPropagatedObjcMap, + dylibProviders); multiArchBinarySupport.registerActions( platform, getExtraLinkArgs(ruleContext), + objcProviderByDepConfiguration, getExtraLinkInputs(ruleContext), - objcCommonByDepConfiguration, configToDepsCollectionMap, outputArtifact); @@ -117,8 +119,8 @@ ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build()); ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder(); - for (ObjcCommon objcCommon : objcCommonByDepConfiguration.values()) { - objcProviderBuilder.addTransitiveAndPropagate(objcCommon.getObjcProvider()); + for (ObjcProvider objcProvider : objcProviderByDepConfiguration.values()) { + objcProviderBuilder.addTransitiveAndPropagate(objcProvider); } objcProviderBuilder.add(MULTI_ARCH_LINKED_BINARIES, outputArtifact);
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 99a6c02..228e37e 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
@@ -60,26 +60,26 @@ ObjcRuleClasses.intermediateArtifacts(ruleContext).combinedArchitectureDylib(); MultiArchBinarySupport multiArchBinarySupport = new MultiArchBinarySupport(ruleContext); - Map<BuildConfiguration, ObjcCommon> objcCommonByDepConfiguration = - multiArchBinarySupport.objcCommonByDepConfiguration(childConfigurations, + Map<BuildConfiguration, ObjcProvider> objcProviderByDepConfiguration = + multiArchBinarySupport.objcProviderByDepConfiguration(childConfigurations, configToDepsCollectionMap, configurationToNonPropagatedObjcMap, dylibProviders); + multiArchBinarySupport.registerActions( platform, new ExtraLinkArgs("-dynamiclib"), + objcProviderByDepConfiguration, ImmutableSet.<Artifact>of(), - objcCommonByDepConfiguration, configToDepsCollectionMap, outputArtifact); NestedSetBuilder<Artifact> filesToBuild = - NestedSetBuilder.<Artifact>stableOrder() - .add(outputArtifact); + NestedSetBuilder.<Artifact>stableOrder().add(outputArtifact); RuleConfiguredTargetBuilder targetBuilder = ObjcRuleClasses.ruleConfiguredTarget(ruleContext, filesToBuild.build()); ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder(); - for (ObjcCommon objcCommon : objcCommonByDepConfiguration.values()) { - objcProviderBuilder.addTransitiveAndPropagate(objcCommon.getObjcProvider()); + for (ObjcProvider objcProvider : objcProviderByDepConfiguration.values()) { + objcProviderBuilder.addTransitiveAndPropagate(objcProvider); } objcProviderBuilder.add(MULTI_ARCH_DYNAMIC_LIBRARIES, outputArtifact);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryRule.java index 7b7b448..123d300 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryRule.java
@@ -27,6 +27,7 @@ /** * Rule definition for apple_dynamic_library. */ +// TODO(b/33077308): Deprecate this rule for apple_binary with attribute changes. public class AppleDynamicLibraryRule implements RuleDefinition { /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java index 22006e7..4213a58 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java
@@ -18,11 +18,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; -import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableSet; -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; @@ -30,7 +28,6 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; @@ -38,7 +35,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes; import com.google.devtools.build.lib.rules.objc.ObjcProvider.Key; -import java.util.HashSet; import java.util.List; import java.util.Set; @@ -48,31 +44,24 @@ public class AppleStaticLibrary implements RuleConfiguredTargetFactory { /** - * Set of {@link ObjcProvider} keys whose values are subtracted by avoid_deps. Specifically, a - * value is propagated if present in the transitive "deps" but *not* present in the transitive - * "avoid_deps". + * Set of {@link ObjcProvider} values which are propagated from dependencies to dependers by + * this rule. */ - private static final ImmutableSet<Key<?>> AVOID_DEPS_KEYS = + private static final ImmutableSet<Key<?>> PROPAGATE_KEYS = ImmutableSet.<Key<?>>of( ObjcProvider.ASSET_CATALOG, ObjcProvider.BUNDLE_FILE, ObjcProvider.GENERAL_RESOURCE_DIR, ObjcProvider.GENERAL_RESOURCE_FILE, + ObjcProvider.SDK_DYLIB, + ObjcProvider.SDK_FRAMEWORK, ObjcProvider.STORYBOARD, ObjcProvider.STRINGS, + ObjcProvider.WEAK_SDK_FRAMEWORK, ObjcProvider.XCDATAMODEL, ObjcProvider.XIB, ObjcProvider.XCASSETS_DIR); - /** - * Set of {@link ObjcProvider} whose values are propagated regardless of avoid_deps. - */ - private static final ImmutableSet<Key<?>> PROPAGATE_REGARDLESS_KEYS = - ImmutableSet.<Key<?>>of( - ObjcProvider.SDK_DYLIB, - ObjcProvider.SDK_FRAMEWORK, - ObjcProvider.WEAK_SDK_FRAMEWORK); - @VisibleForTesting static final String UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT = "Unsupported platform type \"%s\""; @@ -84,7 +73,8 @@ ImmutableListMultimap<BuildConfiguration, TransitiveInfoCollection> configToDepsCollectionMap = ruleContext.getPrerequisitesByConfiguration("deps", Mode.SPLIT); ImmutableListMultimap<BuildConfiguration, ObjcProvider> configToAvoidDepsMap = - ruleContext.getPrerequisitesByConfiguration("avoid_deps", Mode.SPLIT, ObjcProvider.class); + ruleContext.getPrerequisitesByConfiguration(AppleStaticLibraryRule.AVOID_DEPS_ATTR_NAME, + Mode.SPLIT, ObjcProvider.class); Set<BuildConfiguration> childConfigurations = getChildConfigurations(ruleContext); @@ -117,20 +107,20 @@ intermediateArtifacts, nullToEmptyList(configToDepsCollectionMap.get(childConfig)), protosObjcProvider); - + ObjcProvider objcProvider = + common.getObjcProvider().subtractSubtrees(configToAvoidDepsMap.get(childConfig)); + librariesToLipo.add(intermediateArtifacts.strippedSingleArchitectureLibrary()); new LegacyCompilationSupport(ruleContext, childConfig) .registerCompileAndArchiveActions(common) - .registerFullyLinkActionWithAvoids( - common.getObjcProvider(), - intermediateArtifacts.strippedSingleArchitectureLibrary(), - configToAvoidDepsMap.get(childConfig)) + .registerFullyLinkAction( + objcProvider, + intermediateArtifacts.strippedSingleArchitectureLibrary()) .validateAttributes(); ruleContext.assertNoErrors(); - addTransitiveAndAvoid(objcProviderBuilder, common.getObjcProvider(), - configToAvoidDepsMap.get(childConfig)); + addTransitivePropagatedKeys(objcProviderBuilder, objcProvider); } AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class); @@ -150,27 +140,12 @@ targetBuilder.addProvider(ObjcProvider.class, objcProviderBuilder.build()); return targetBuilder.build(); } - - private void addTransitiveAndAvoid(ObjcProvider.Builder objcProviderBuilder, - ObjcProvider provider, ImmutableList<ObjcProvider> avoidProviders) { - for (Key<?> key : PROPAGATE_REGARDLESS_KEYS) { + + private void addTransitivePropagatedKeys(ObjcProvider.Builder objcProviderBuilder, + ObjcProvider provider) { + for (Key<?> key : PROPAGATE_KEYS) { objcProviderBuilder.addTransitiveAndPropagate(key, provider); } - for (Key<?> key : AVOID_DEPS_KEYS) { - addTransitiveAndAvoid(objcProviderBuilder, key, provider, avoidProviders); - } - } - - private <T> void addTransitiveAndAvoid(ObjcProvider.Builder objcProviderBuilder, Key<T> key, - ObjcProvider provider, ImmutableList<ObjcProvider> avoidProviders) { - HashSet<T> avoidItemsSet = new HashSet<T>(); - for (ObjcProvider avoidProvider : avoidProviders) { - avoidItemsSet.addAll(avoidProvider.getPropagable(key).toList()); - } - NestedSet<T> items = provider.getPropagable(key); - - objcProviderBuilder.addAll(key, - Iterables.filter(items.toList(), Predicates.not(Predicates.in(avoidItemsSet)))); } private ObjcCommon common(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryRule.java index 5c78165..dd9f2b7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryRule.java
@@ -41,6 +41,12 @@ */ static final SafeImplicitOutputsFunction LIPO_ARCHIVE = fromTemplates("%{name}_lipo.a"); + /** + * Attribute name for dependent libraries which should not be linked into the outputs of this + * rule. + */ + static final String AVOID_DEPS_ATTR_NAME = "avoid_deps"; + @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment env) { MultiArchSplitTransitionProvider splitTransitionProvider = @@ -68,7 +74,7 @@ an application importing both X and C would have duplicate symbols for C.</p> <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .add( - attr("avoid_deps", LABEL_LIST) + attr(AVOID_DEPS_ATTR_NAME, LABEL_LIST) .direct_compile_time_input() .allowedRuleClasses(ObjcRuleClasses.CompilingRule.ALLOWED_DEPS_RULE_CLASSES) .mandatoryNativeProviders(
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 f3f5cd5..d3206f4 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
@@ -336,6 +336,22 @@ /** * Registers all actions necessary to compile this rule's sources and archive them. * + * @param compilationArtifacts collection of artifacts required for the compilation + * @param objcProvider provides all compiling and linking information to register these actions + * @return this compilation support + * @throws RuleErrorException for invalid crosstool files + */ + CompilationSupport registerCompileAndArchiveActions( + CompilationArtifacts compilationArtifacts, + ObjcProvider objcProvider) throws RuleErrorException, InterruptedException { + return registerCompileAndArchiveActions( + compilationArtifacts, objcProvider, ExtraCompileArgs.NONE, + ImmutableList.<PathFragment>of()); + } + + /** + * Registers all actions necessary to compile this rule's sources and archive them. + * * @param common common information about this rule and its dependencies * @return this compilation support * @throws RuleErrorException for invalid crosstool files @@ -452,16 +468,12 @@ * Registers an action that will generate a clang module map for this target, using the hdrs * attribute of this rule. */ - CompilationSupport registerGenerateModuleMapAction( - Optional<CompilationArtifacts> compilationArtifacts) { + CompilationSupport registerGenerateModuleMapAction(CompilationArtifacts compilationArtifacts) { // TODO(bazel-team): Include textual headers in the module map when Xcode 6 support is // dropped. // TODO(b/32225593): Include private headers in the module map. Iterable<Artifact> publicHeaders = attributes.hdrs(); - if (compilationArtifacts.isPresent()) { - CompilationArtifacts artifacts = compilationArtifacts.get(); - publicHeaders = Iterables.concat(publicHeaders, artifacts.getAdditionalHdrs()); - } + publicHeaders = Iterables.concat(publicHeaders, compilationArtifacts.getAdditionalHdrs()); CppModuleMap moduleMap = intermediateArtifacts.moduleMap(); registerGenerateModuleMapAction(moduleMap, publicHeaders); @@ -554,17 +566,38 @@ /** * Registers all actions necessary to compile this rule's sources and archive them. * - * @param common common information about this rule and its dependencies + * @param compilationArtifacts collection of artifacts required for the compilation + * @param objcProvider provides all compiling and linking information to register these actions * @param extraCompileArgs args to be added to compile actions * @param priorityHeaders priority headers to be included before the dependency headers * @return this compilation support * @throws RuleErrorException for invalid crosstool files */ abstract CompilationSupport registerCompileAndArchiveActions( - ObjcCommon common, ExtraCompileArgs extraCompileArgs, Iterable<PathFragment> priorityHeaders) + CompilationArtifacts compilationArtifacts, ObjcProvider objcProvider, + ExtraCompileArgs extraCompileArgs, Iterable<PathFragment> priorityHeaders) throws RuleErrorException, InterruptedException; /** + * Registers all actions necessary to compile this rule's sources and archive them. + * + * @param common common information about this rule and its dependencies + * @param extraCompileArgs args to be added to compile actions + * @param priorityHeaders priority headers to be included before the dependency headers + * @return this compilation support + * @throws RuleErrorException for invalid crosstool files + */ + CompilationSupport registerCompileAndArchiveActions( + ObjcCommon common, ExtraCompileArgs extraCompileArgs, Iterable<PathFragment> priorityHeaders) + throws RuleErrorException, InterruptedException { + if (common.getCompilationArtifacts().isPresent()) { + registerCompileAndArchiveActions(common.getCompilationArtifacts().get(), + common.getObjcProvider(), extraCompileArgs, priorityHeaders); + } + return this; + } + + /** * Registers any actions necessary to link this rule and its dependencies. * * <p>Dsym bundle is generated if {@link ObjcConfiguration#generateDsym()} is set. @@ -875,8 +908,9 @@ * Registers an action that will generate a clang module map. * @param moduleMap the module map to generate * @param publicHeaders the headers that should be directly accessible by dependers + * @return this compilation support */ - private void registerGenerateModuleMapAction( + public CompilationSupport registerGenerateModuleMapAction( CppModuleMap moduleMap, Iterable<Artifact> publicHeaders) { publicHeaders = Iterables.filter(publicHeaders, MODULE_MAP_HEADER); ruleContext.registerAction( @@ -891,6 +925,8 @@ /*moduleMapHomeIsCwd=*/ false, /*generateSubModules=*/ false, /*externDependencies=*/ true)); + + return this; } /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java index 51f406c..1b03052 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
@@ -89,15 +89,15 @@ CompilationAttributes.Builder.fromRuleContext(ruleContext).build()); this.compilationArtifacts = compilationArtifacts(ruleContext); } - + @Override CompilationSupport registerCompileAndArchiveActions( - ObjcCommon common, ExtraCompileArgs extraCompileArgs, Iterable<PathFragment> priorityHeaders) - throws RuleErrorException, InterruptedException { - + CompilationArtifacts compilationArtifacts, + ObjcProvider objcProvider, ExtraCompileArgs extraCompileArgs, + Iterable<PathFragment> priorityHeaders) throws RuleErrorException, InterruptedException { ObjcVariablesExtension.Builder extension = new ObjcVariablesExtension.Builder() .setRuleContext(ruleContext) - .setObjcProvider(common.getObjcProvider()) + .setObjcProvider(objcProvider) .setCompilationArtifacts(compilationArtifacts) .setIntermediateArtifacts(intermediateArtifacts) .setConfiguration(ruleContext.getConfiguration()); @@ -105,17 +105,17 @@ if (compilationArtifacts.getArchive().isPresent()) { Artifact objList = intermediateArtifacts.archiveObjList(); - + // TODO(b/30783125): Signal the need for this action in the CROSSTOOL. registerObjFilelistAction(getObjFiles(compilationArtifacts, intermediateArtifacts), objList); - + extension.addVariableCategory(VariableCategory.ARCHIVE_VARIABLES); - helper = createCcLibraryHelper(common, extension.build()) + helper = createCcLibraryHelper(objcProvider, extension.build()) .setLinkType(LinkTargetType.OBJC_ARCHIVE) .addLinkActionInput(objList); } else { - helper = createCcLibraryHelper(common, extension.build()); + helper = createCcLibraryHelper(objcProvider, extension.build()); } helper.build(); @@ -228,7 +228,8 @@ return this; } - private CcLibraryHelper createCcLibraryHelper(ObjcCommon common, VariablesExtension extension) { + private CcLibraryHelper createCcLibraryHelper(ObjcProvider objcProvider, + VariablesExtension extension) { PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext); Collection<Artifact> arcSources = Sets.newHashSet(compilationArtifacts.getSrcs()); Collection<Artifact> nonArcSources = Sets.newHashSet(compilationArtifacts.getNonArcSrcs()); @@ -240,22 +241,22 @@ return new CcLibraryHelper( ruleContext, new ObjcCppSemantics( - common.getObjcProvider(), ruleContext.getFragment(ObjcConfiguration.class)), + objcProvider, ruleContext.getFragment(ObjcConfiguration.class)), getFeatureConfiguration(ruleContext), CcLibraryHelper.SourceCategory.CC_AND_OBJC) .addSources(arcSources, ImmutableMap.of("objc_arc", "")) .addSources(nonArcSources, ImmutableMap.of("no_objc_arc", "")) .addSources(privateHdrs) - .addDefines(common.getObjcProvider().get(DEFINE)) + .addDefines(objcProvider.get(DEFINE)) .enableCompileProviders() .addPublicHeaders(publicHdrs) .addPublicHeaders(pchHdrList) .addPrecompiledFiles(precompiledFiles) .addDeps(ruleContext.getPrerequisites("deps", Mode.TARGET)) .addCopts(getCompileRuleCopts()) - .addIncludeDirs(common.getObjcProvider().get(INCLUDE)) + .addIncludeDirs(objcProvider.get(INCLUDE)) .addCopts(ruleContext.getFragment(ObjcConfiguration.class).getCoptsForCompilationMode()) - .addSystemIncludeDirs(common.getObjcProvider().get(INCLUDE_SYSTEM)) + .addSystemIncludeDirs(objcProvider.get(INCLUDE_SYSTEM)) .setCppModuleMap(intermediateArtifacts.moduleMap()) .addVariableExtension(extension); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java index 7297563..2c015ea 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
@@ -172,34 +172,23 @@ super(ruleContext, buildConfiguration, intermediateArtifacts, compilationAttributes); } - /** - * Registers all actions necessary to compile this rule's sources and archive them. - * - * @param common common information about this rule and its dependencies - * @param extraCompileArgs args to be added to compile actions - * @param priorityHeaders priority headers to be included before the dependency headers - * @return this compilation support - */ @Override CompilationSupport registerCompileAndArchiveActions( - ObjcCommon common, - ExtraCompileArgs extraCompileArgs, - Iterable<PathFragment> priorityHeaders) { - if (common.getCompilationArtifacts().isPresent()) { - registerGenerateModuleMapAction(common.getCompilationArtifacts()); - Optional<CppModuleMap> moduleMap; - if (objcConfiguration.moduleMapsEnabled()) { - moduleMap = Optional.of(intermediateArtifacts.moduleMap()); - } else { - moduleMap = Optional.absent(); - } - registerCompileAndArchiveActions( - common.getCompilationArtifacts().get(), - common.getObjcProvider(), - extraCompileArgs, - priorityHeaders, - moduleMap); + CompilationArtifacts compilationArtifacts, ObjcProvider objcProvider, + ExtraCompileArgs extraCompileArgs, Iterable<PathFragment> priorityHeaders) { + registerGenerateModuleMapAction(compilationArtifacts); + Optional<CppModuleMap> moduleMap; + if (objcConfiguration.moduleMapsEnabled()) { + moduleMap = Optional.of(intermediateArtifacts.moduleMap()); + } else { + moduleMap = Optional.absent(); } + registerCompileAndArchiveActions( + compilationArtifacts, + objcProvider, + extraCompileArgs, + priorityHeaders, + moduleMap); return this; } @@ -269,20 +258,6 @@ } } - /** - * Adds a source file to a command line, honoring the useAbsolutePathForActions flag. - */ - private CustomCommandLine.Builder addSource(CustomCommandLine.Builder commandLine, - Artifact sourceFile) { - PathFragment sourceExecPathFragment = sourceFile.getExecPath(); - String sourcePath = sourceExecPathFragment.getPathString(); - if (!sourceExecPathFragment.isAbsolute() && objcConfiguration.getUseAbsolutePathsForActions()) { - sourcePath = objcConfiguration.getXcodeWorkspaceRoot() + "/" + sourcePath; - } - commandLine.add(sourcePath); - return commandLine; - } - private CustomCommandLine compileActionCommandLine( Artifact sourceFile, Artifact objFile,
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 a127bc5..5a44edc 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,10 +52,10 @@ * @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 configurationToObjcProvider a map from from dependency configuration to the + * {@link ObjcProvider} which comprises all information about the dependencies in that + * configuration. Can be obtained via {@link #objcProviderByDepConfiguration} * @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} * @param configToDepsCollectionMap a multimap from dependency configuration to the * list of provider collections which are propagated from the dependencies of that * configuration @@ -66,16 +66,15 @@ public void registerActions( Platform platform, ExtraLinkArgs extraLinkArgs, + Map<BuildConfiguration, ObjcProvider> configurationToObjcProvider, Iterable<Artifact> extraLinkInputs, - Map<BuildConfiguration, ObjcCommon> configurationToObjcCommon, ImmutableListMultimap<BuildConfiguration, TransitiveInfoCollection> configToDepsCollectionMap, Artifact outputLipoBinary) throws RuleErrorException, InterruptedException { NestedSetBuilder<Artifact> binariesToLipo = NestedSetBuilder.<Artifact>stableOrder(); - for (BuildConfiguration childConfig : configurationToObjcCommon.keySet()) { - ObjcCommon common = configurationToObjcCommon.get(childConfig); + for (BuildConfiguration childConfig : configurationToObjcProvider.keySet()) { IntermediateArtifacts intermediateArtifacts = ObjcRuleClasses.intermediateArtifacts(ruleContext, childConfig); ImmutableList.Builder<J2ObjcMappingFileProvider> j2ObjcMappingFileProviders = @@ -97,10 +96,14 @@ binariesToLipo.add(intermediateArtifacts.strippedSingleArchitectureBinary()); + ObjcProvider objcProvider = configurationToObjcProvider.get(childConfig); + CompilationArtifacts compilationArtifacts = + CompilationSupport.compilationArtifacts(ruleContext, + ObjcRuleClasses.intermediateArtifacts(ruleContext, childConfig)); new LegacyCompilationSupport(ruleContext, childConfig) - .registerCompileAndArchiveActions(common) + .registerCompileAndArchiveActions(compilationArtifacts, objcProvider) .registerLinkActions( - common.getObjcProvider(), + objcProvider, j2ObjcMappingFileProvider, j2ObjcEntryClassProvider, extraLinkArgs, @@ -129,17 +132,17 @@ * the current rule have propagated in that configuration * @param configurationToNonPropagatedObjcMap a map from child configuration to providers that * "non_propagated_deps" of the current rule have propagated in that configuration - * @param configToDylibsObjcMap providers that dynamic library dependencies of the current rule + * @param dylibProviders providers that dynamic library dependencies of the current rule * have propagated * @throws RuleErrorException if there are attribute errors in the current rule context */ - public Map<BuildConfiguration, ObjcCommon> objcCommonByDepConfiguration( + public Map<BuildConfiguration, ObjcProvider> objcProviderByDepConfiguration( Set<BuildConfiguration> childConfigurations, ImmutableListMultimap<BuildConfiguration, TransitiveInfoCollection> configToDepsCollectionMap, ImmutableListMultimap<BuildConfiguration, ObjcProvider> configurationToNonPropagatedObjcMap, - Iterable<ObjcProvider> configToDylibsObjcMap) + Iterable<ObjcProvider> dylibProviders) throws RuleErrorException, InterruptedException { - ImmutableMap.Builder<BuildConfiguration, ObjcCommon> configurationToObjcCommonBuilder = + ImmutableMap.Builder<BuildConfiguration, ObjcProvider> configurationToObjcProviderBuilder = ImmutableMap.builder(); for (BuildConfiguration childConfig : childConfigurations) { Optional<ObjcProvider> protosObjcProvider; @@ -156,7 +159,7 @@ IntermediateArtifacts intermediateArtifacts = ObjcRuleClasses.intermediateArtifacts(ruleContext, childConfig); - Iterable<ObjcProvider> additionalDepProviders = Iterables.concat(configToDylibsObjcMap, + Iterable<ObjcProvider> additionalDepProviders = Iterables.concat(dylibProviders, ruleContext.getPrerequisites("bundles", Mode.TARGET, ObjcProvider.class), protosObjcProvider.asSet()); @@ -168,11 +171,12 @@ nullToEmptyList(configToDepsCollectionMap.get(childConfig)), nullToEmptyList(configurationToNonPropagatedObjcMap.get(childConfig)), additionalDepProviders); + ObjcProvider objcProvider = common.getObjcProvider().subtractSubtrees(dylibProviders); - configurationToObjcCommonBuilder.put(childConfig, common); + configurationToObjcProviderBuilder.put(childConfig, objcProvider); } - return configurationToObjcCommonBuilder.build(); + return configurationToObjcProviderBuilder.build(); } private ObjcCommon common(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java index 69a41c4..cbadc2c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcImport.java
@@ -16,14 +16,13 @@ import static com.google.devtools.build.lib.rules.objc.XcodeProductType.LIBRARY_STATIC; -import com.google.common.base.Optional; 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; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; +import com.google.devtools.build.lib.rules.cpp.CppModuleMap; import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes; import com.google.devtools.build.lib.syntax.Type; @@ -51,8 +50,16 @@ XcodeProvider.Builder xcodeProviderBuilder = new XcodeProvider.Builder(); NestedSetBuilder<Artifact> filesToBuild = NestedSetBuilder.stableOrder(); + CompilationAttributes compilationAttributes = + CompilationAttributes.Builder.fromRuleContext(ruleContext).build(); + IntermediateArtifacts intermediateArtifacts = + ObjcRuleClasses.intermediateArtifacts(ruleContext); + + Iterable<Artifact> publicHeaders = compilationAttributes.hdrs(); + CppModuleMap moduleMap = intermediateArtifacts.moduleMap(); + new LegacyCompilationSupport(ruleContext) - .registerGenerateModuleMapAction(Optional.<CompilationArtifacts>absent()) + .registerGenerateModuleMapAction(moduleMap, publicHeaders) .addXcodeSettings(xcodeProviderBuilder, common) .validateAttributes();
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 3c2bac6..291599f 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
@@ -21,6 +21,7 @@ import com.google.common.base.Predicates; 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.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; @@ -39,6 +40,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.xcode.xcodegen.proto.XcodeGenProtos.TargetControl; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; /** @@ -243,24 +245,6 @@ new Key<>(STABLE_ORDER, "root_merge_zip", Artifact.class); /** - * Exec paths of {@code .framework} directories corresponding to static frameworks to link. These - * cause -F arguments (framework search paths) to be added to each compile action, and - * -framework (link framework) arguments to be added to each link action. These differ from - * dynamic frameworks in that they are statically linked into the binary. - */ - public static final Key<PathFragment> STATIC_FRAMEWORK_DIR = - new Key<>(LINK_ORDER, "framework_dir", PathFragment.class); - - /** - * Exec paths of {@code .framework} directories corresponding to dynamic frameworks to link. These - * cause -F arguments (framework search paths) to be added to each compile action, and - * -framework (link framework) arguments to be added to each link action. These differ from - * static frameworks in that they are not statically linked into the binary. - */ - public static final Key<PathFragment> DYNAMIC_FRAMEWORK_DIR = - new Key<>(LINK_ORDER, "dynamic_framework_dir", PathFragment.class); - - /** * Exec paths of {@code .framework} directories corresponding to frameworks to include in search * paths, but not to link. These cause -F arguments (framework search paths) to be added to * each compile action, but do not cause -framework (link framework) arguments to be added to @@ -270,6 +254,16 @@ new Key<>(LINK_ORDER, "framework_search_paths", PathFragment.class); /** + * Exec paths of {@code .framework} directories corresponding to static frameworks to link. These + * cause -F arguments (framework search paths) to be added to each compile action, and + * -framework (link framework) arguments to be added to each link action. These differ from + * dynamic frameworks in that they are statically linked into the binary. + */ + // TODO(cparsons): Rename this key to static_framework_dir. + public static final Key<PathFragment> STATIC_FRAMEWORK_DIR = + new Key<>(LINK_ORDER, "framework_dir", PathFragment.class); + + /** * Files in {@code .framework} directories that should be included as inputs when compiling and * linking. */ @@ -277,6 +271,15 @@ new Key<>(STABLE_ORDER, "static_framework_file", Artifact.class); /** + * Exec paths of {@code .framework} directories corresponding to dynamic frameworks to link. These + * cause -F arguments (framework search paths) to be added to each compile action, and + * -framework (link framework) arguments to be added to each link action. These differ from + * static frameworks in that they are not statically linked into the binary. + */ + public static final Key<PathFragment> DYNAMIC_FRAMEWORK_DIR = + new Key<>(LINK_ORDER, "dynamic_framework_dir", PathFragment.class); + + /** * Files in {@code .framework} directories belonging to a dynamically linked framework. They * should be included as inputs when compiling and linking as well as copied into the final * application bundle. @@ -450,6 +453,42 @@ TOP_LEVEL_MODULE_MAP); /** + * Set of {@link ObjcProvider} whose values are not subtracted via {@link #subtractSubtrees}. + * + * <p>Only keys which are unrelated to statically-linked library dependencies should be listed. + * For example, LIBRARY is a subtractable key because it contains objects of individual + * objc_library dependencies, but keys pertaining to resources are non-subtractable keys, because + * the top level binary will need these resources whether or not the library is statically or + * dynamically linked. + */ + private static final ImmutableSet<Key<?>> NON_SUBTRACTABLE_KEYS = + ImmutableSet.<Key<?>>of( + ASSET_CATALOG, + BUNDLE_FILE, + BUNDLE_IMPORT_DIR, + DEFINE, + DYNAMIC_FRAMEWORK_DIR, + DYNAMIC_FRAMEWORK_FILE, + FLAG, + GENERAL_RESOURCE_DIR, + GENERAL_RESOURCE_FILE, + MERGE_ZIP, + ROOT_MERGE_ZIP, + FRAMEWORK_SEARCH_PATH_ONLY, + HEADER, + INCLUDE, + INCLUDE_SYSTEM, + LINKOPT, + SDK_DYLIB, + SDK_FRAMEWORK, + STORYBOARD, + STRINGS, + WEAK_SDK_FRAMEWORK, + XCASSETS_DIR, + XCDATAMODEL, + XIB); + + /** * Returns the skylark key for the given string, or null if no such key exists or is available * to Skylark. */ @@ -504,11 +543,23 @@ } /** + * Returns all keys that have at least one value in this provider (values may be propagable, + * non-propagable, or strict). + */ + private Iterable<Key<?>> getValuedKeys() { + return ImmutableSet.<Key<?>>builder() + .addAll(strictDependencyItems.keySet()) + .addAll(nonPropagatedItems.keySet()) + .addAll(items.keySet()) + .build(); + } + + /** * All artifacts, bundleable files, etc, that should be propagated to transitive dependers, of * the type specified by {@code key}. */ @SuppressWarnings("unchecked") - public <E> NestedSet<E> getPropagable(Key<E> key) { + private <E> NestedSet<E> getPropagable(Key<E> key) { Preconditions.checkNotNull(key); NestedSetBuilder<E> builder = new NestedSetBuilder<>(key.order); if (items.containsKey(key)) { @@ -553,6 +604,55 @@ } /** + * Subtracts dependency subtrees from this provider and returns the result (subtraction does not + * mutate this provider). Note that not all provider keys are subtracted; generally only keys + * which correspond with compiled libraries will be subtracted. + * + * <p>This is an expensive operation, as it requires flattening of all nested sets contained + * in each provider. + * + * @param avoidProviders providers which contain the dependency subtrees to subtract + */ + // TODO(b/19795062): Investigate subtraction generalized to NestedSet. + public ObjcProvider subtractSubtrees(Iterable<ObjcProvider> avoidProviders) { + ObjcProvider.Builder objcProviderBuilder = new ObjcProvider.Builder(); + for (Key<?> key : getValuedKeys()) { + if (NON_SUBTRACTABLE_KEYS.contains(key)) { + addTransitiveAndAvoid(objcProviderBuilder, key, ImmutableList.<ObjcProvider>of()); + } else { + addTransitiveAndAvoid(objcProviderBuilder, key, avoidProviders); + } + } + return objcProviderBuilder.build(); + } + + @SuppressWarnings("unchecked") + private <T> void addTransitiveAndAvoid(ObjcProvider.Builder objcProviderBuilder, Key<T> key, + Iterable<ObjcProvider> avoidProviders) { + HashSet<T> avoidItemsSet = new HashSet<T>(); + for (ObjcProvider avoidProvider : avoidProviders) { + avoidItemsSet.addAll(avoidProvider.getPropagable(key).toList()); + } + NestedSet<T> propagableItems = (NestedSet<T>) items.get(key); + NestedSet<T> nonPropagableItems = (NestedSet<T>) nonPropagatedItems.get(key); + NestedSet<T> strictItems = (NestedSet<T>) strictDependencyItems.get(key); + + if (propagableItems != null) { + objcProviderBuilder.addAll(key, + Iterables.filter(propagableItems.toList(), Predicates.not(Predicates.in(avoidItemsSet)))); + } + if (nonPropagableItems != null) { + objcProviderBuilder.addAllNonPropagable(key, + Iterables.filter(nonPropagableItems.toList(), + Predicates.not(Predicates.in(avoidItemsSet)))); + } + if (strictItems != null) { + objcProviderBuilder.addAllForDirectDependents(key, + Iterables.filter(strictItems.toList(), Predicates.not(Predicates.in(avoidItemsSet)))); + } + } + + /** * A builder for this context with an API that is optimized for collecting information from * several transitive dependencies. */ @@ -713,6 +813,14 @@ } /** + * Add elements in toAdd, and do not propagate to dependents of this provider. + */ + public <E> Builder addAllNonPropagable(Key<E> key, Iterable<? extends E> toAdd) { + uncheckedAddAll(key, toAdd, this.nonPropagatedItems); + return this; + } + + /** * Add element toAdd, and propagate it only to direct dependents of this provider. */ public <E> Builder addForDirectDependents(Key<E> key, E toAdd) {
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 af000294..7209c7a 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
@@ -967,8 +967,16 @@ return builder // TODO(b/32411441): Restrict the dylibs attribute to take only dylib dependencies. // This will require refactoring ObjcProvider into alternate providers. - // TODO(cparsons): Subtract transitive dependencies from "deps" during linking, as needed. - // Also document this attribute when this is resolved. + /* <!-- #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(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java index c130eff..b483541 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
@@ -147,7 +147,7 @@ ruleContext, intermediateArtifacts, new CompilationAttributes.Builder().build()) - .registerGenerateModuleMapAction(Optional.of(moduleMapCompilationArtifacts.build())); + .registerGenerateModuleMapAction(moduleMapCompilationArtifacts.build()); } /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java index 4ae0c36..803d777 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
@@ -99,7 +99,7 @@ throws RuleErrorException, InterruptedException { new LegacyCompilationSupport(ruleContext) .registerCompileAndArchiveActions(getCommon()) - .registerGenerateModuleMapAction(Optional.of(getCompilationArtifacts())); + .registerGenerateModuleMapAction(getCompilationArtifacts()); return this; }