Skip to content

Commit

Permalink
Add implementation deps support for Objective-C (bazelbuild#18372)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivan-golub authored May 16, 2023
1 parent 80b20b1 commit 9b3a7eb
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,11 @@ static Map<Label, Collection<Artifact>> buildLocationMap(
Iterables.addAll(
depsDataAndTools, ruleContext.getPrerequisitesIf("deps", FilesToRunProvider.class));
}
if (ruleContext.getRule().isAttrDefined("implementation_deps", BuildType.LABEL_LIST)) {
Iterables.addAll(
depsDataAndTools,
ruleContext.getPrerequisitesIf("implementation_deps", FilesToRunProvider.class));
}
if (allowDataAttributeEntriesInLabel
&& ruleContext.getRule().isAttrDefined("data", BuildType.LABEL_LIST)) {
Iterables.addAll(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ static class Builder {
private final List<CcCompilationContext> ccCompilationContexts = new ArrayList<>();
private final List<CcLinkingContext> ccLinkingContexts = new ArrayList<>();
private final List<CcCompilationContext> directCCompilationContexts = new ArrayList<>();
private final List<CcCompilationContext> implementationCcCompilationContexts =
new ArrayList<>();
// List of CcLinkingContext to be merged into ObjcProvider, to be done for deps that don't have
// ObjcProviders.
// TODO(b/171413861): remove after objc link info migration.
Expand Down Expand Up @@ -179,6 +181,13 @@ Builder addCcLinkingContexts(Iterable<CcInfo> ccInfos) {
return this;
}

@CanIgnoreReturnValue
Builder addImplementationCcCompilationContexts(Iterable<CcInfo> ccInfos) {
ccInfos.forEach(
ccInfo -> implementationCcCompilationContexts.add(ccInfo.getCcCompilationContext()));
return this;
}

@CanIgnoreReturnValue
Builder addCcInfos(Iterable<CcInfo> ccInfos) {
addCcCompilationContexts(ccInfos);
Expand Down Expand Up @@ -291,6 +300,8 @@ ObjcCommon build() {
ImmutableList.copyOf(this.ccLinkingContexts);
ImmutableList<CcCompilationContext> directCCompilationContexts =
ImmutableList.copyOf(this.directCCompilationContexts);
ImmutableList<CcCompilationContext> implementationCcCompilationContexts =
ImmutableList.copyOf(this.implementationCcCompilationContexts);
ImmutableList<CcLinkingContext> ccLinkingContextsForMerging =
ImmutableList.copyOf(this.ccLinkingContextsForMerging);

Expand All @@ -310,7 +321,8 @@ ObjcCommon build() {
.addDirectCcCompilationContexts(directCCompilationContexts)
// TODO(bazel-team): This pulls in stl via
// CcCompilationHelper.getStlCcCompilationContext(), but probably shouldn't.
.addCcCompilationContexts(ccCompilationContexts);
.addCcCompilationContexts(ccCompilationContexts)
.addImplementationCcCompilationContexts(implementationCcCompilationContexts);

for (CcLinkingContext ccLinkingContext : ccLinkingContextsForMerging) {
ImmutableList<String> linkOpts = ccLinkingContext.getFlattenedUserLinkFlags();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public final class ObjcCompilationContext implements StarlarkValue {
private final ImmutableList<PathFragment> strictDependencyIncludes;
private final ImmutableList<CcCompilationContext> directCcCompilationContexts;
private final ImmutableList<CcCompilationContext> ccCompilationContexts;
private final ImmutableList<CcCompilationContext> implementationCcCompilationContexts;

ObjcCompilationContext(
Iterable<String> defines,
Expand All @@ -64,7 +65,8 @@ public final class ObjcCompilationContext implements StarlarkValue {
Iterable<PathFragment> quoteIncludes,
Iterable<PathFragment> strictDependencyIncludes,
Iterable<CcCompilationContext> directCcCompilationContexts,
Iterable<CcCompilationContext> ccCompilationContexts) {
Iterable<CcCompilationContext> ccCompilationContexts,
Iterable<CcCompilationContext> implementationCcCompilationContexts) {
this.defines = ImmutableList.copyOf(defines);
this.publicHeaders = ImmutableList.copyOf(publicHeaders);
this.publicTextualHeaders = ImmutableList.copyOf(publicTextualHeaders);
Expand All @@ -75,6 +77,8 @@ public final class ObjcCompilationContext implements StarlarkValue {
this.strictDependencyIncludes = ImmutableList.copyOf(strictDependencyIncludes);
this.directCcCompilationContexts = ImmutableList.copyOf(directCcCompilationContexts);
this.ccCompilationContexts = ImmutableList.copyOf(ccCompilationContexts);
this.implementationCcCompilationContexts =
ImmutableList.copyOf(implementationCcCompilationContexts);
}

public ImmutableList<String> getDefines() {
Expand Down Expand Up @@ -159,11 +163,23 @@ public ImmutableList<CcCompilationContext> getCcCompilationContexts() {
return ccCompilationContexts;
}

public ImmutableList<CcCompilationContext> getImplementationCcCompilationContexts() {
return implementationCcCompilationContexts;
}

@StarlarkMethod(name = "cc_compilation_contexts", documented = false, structField = true)
public Sequence<CcCompilationContext> getCcCompilationContextsForStarlark() {
return StarlarkList.immutableCopyOf(getCcCompilationContexts());
}

@StarlarkMethod(
name = "implementation_cc_compilation_contexts",
documented = false,
structField = true)
public Sequence<CcCompilationContext> getImplementationCcCompilationContextsForStarlark() {
return StarlarkList.immutableCopyOf(getImplementationCcCompilationContexts());
}

public CcCompilationContext createCcCompilationContext() {
CcCompilationContext.Builder builder =
CcCompilationContext.builder(
Expand Down Expand Up @@ -200,6 +216,8 @@ static class Builder {
private final List<PathFragment> strictDependencyIncludes = new ArrayList<>();
private final List<CcCompilationContext> directCcCompilationContexts = new ArrayList<>();
private final List<CcCompilationContext> ccCompilationContexts = new ArrayList<>();
private final List<CcCompilationContext> implementationCcCompilationContexts =
new ArrayList<>();

Builder() {}

Expand Down Expand Up @@ -266,6 +284,13 @@ public Builder addDirectCcCompilationContexts(
return this;
}

@CanIgnoreReturnValue
public Builder addImplementationCcCompilationContexts(
Iterable<CcCompilationContext> ccCompilationContexts) {
Iterables.addAll(this.implementationCcCompilationContexts, ccCompilationContexts);
return this;
}

@CanIgnoreReturnValue
public Builder addCcCompilationContext(CcCompilationContext ccCompilationContext) {
this.ccCompilationContexts.add(ccCompilationContext);
Expand All @@ -283,7 +308,8 @@ ObjcCompilationContext build() {
quoteIncludes,
strictDependencyIncludes,
directCcCompilationContexts,
ccCompilationContexts);
ccCompilationContexts,
implementationCcCompilationContexts);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,19 @@ Header file to prepend to every source file being compiled (both arc
.direct_compile_time_input()
.mandatoryProviders(AppleDynamicFrameworkInfo.STARLARK_CONSTRUCTOR.id())
.allowedFileTypes())
/* <!-- #BLAZE_RULE($objc_compiling_rule).ATTRIBUTE(implementation_deps) -->
The list of other libraries that the library target depends on. Unlike with
<code>deps</code>, the headers and include paths of these libraries (and all their
transitive deps) are only used for compilation of this library, and not libraries that
depend on it. Libraries specified with <code>implementation_deps</code> are still linked
in binary targets that depend on this library.
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
.add(
attr("implementation_deps", LABEL_LIST)
.direct_compile_time_input()
.allowedRuleClasses(ALLOWED_CC_DEPS_RULE_CLASSES)
.mandatoryProviders(ObjcProvider.STARLARK_CONSTRUCTOR.id())
.allowedFileTypes())
/* <!-- #BLAZE_RULE($objc_compiling_rule).ATTRIBUTE(defines) -->
Extra <code>-D</code> flags to pass to the compiler. They should be in
the form <code>KEY=VALUE</code> or simply <code>KEY</code> and are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,11 @@ public InstrumentedFilesInfo createInstrumentedFilesInfo(
positional = false,
defaultValue = "[]",
named = true),
@Param(
name = "implementation_cc_compilation_contexts",
positional = false,
defaultValue = "[]",
named = true),
@Param(name = "defines", positional = false, defaultValue = "[]", named = true),
@Param(name = "includes", positional = false, defaultValue = "[]", named = true),
})
Expand All @@ -278,6 +283,7 @@ public ObjcCompilationContext createCompilationContext(
Sequence<?> providers,
Sequence<?> directCcCompilationContexts,
Sequence<?> ccCompilationContexts,
Sequence<?> implementationCcCompilationContexts,
Sequence<?> defines,
Sequence<?> includes)
throws InterruptedException, EvalException {
Expand All @@ -293,6 +299,11 @@ public ObjcCompilationContext createCompilationContext(
.addCcCompilationContexts(
Sequence.cast(
ccCompilationContexts, CcCompilationContext.class, "cc_compilation_contexts"))
.addImplementationCcCompilationContexts(
Sequence.cast(
implementationCcCompilationContexts,
CcCompilationContext.class,
"implementation_cc_compilation_contexts"))
.addDefines(Sequence.cast(defines, String.class, "defines"))
.addIncludes(
Sequence.cast(includes, String.class, "includes").stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def _build_common_variables(
empty_compilation_artifacts = False,
deps = [],
runtime_deps = [],
implementation_deps = [],
extra_disabled_features = [],
extra_enabled_features = [],
extra_import_libraries = [],
Expand All @@ -95,6 +96,7 @@ def _build_common_variables(
compilation_artifacts = compilation_artifacts,
deps = deps,
runtime_deps = runtime_deps,
implementation_deps = implementation_deps,
intermediate_artifacts = intermediate_artifacts,
alwayslink = alwayslink,
has_module_map = has_module_map,
Expand Down Expand Up @@ -193,6 +195,7 @@ def _compile(
system_includes = objc_compilation_context.system_includes,
quote_includes = objc_compilation_context.quote_includes,
compilation_contexts = objc_compilation_context.cc_compilation_contexts,
implementation_compilation_contexts = objc_compilation_context.implementation_cc_compilation_contexts,
user_compile_flags = user_compile_flags,
grep_includes = _get_grep_includes(common_variables.ctx),
module_map = module_map,
Expand Down
27 changes: 27 additions & 0 deletions src/main/starlark/builtins_bzl/common/objc/objc_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def _create_context_and_provider(
extra_import_libraries,
deps,
runtime_deps,
implementation_deps,
attr_linkopts):
objc_providers = []
cc_compilation_contexts = []
Expand Down Expand Up @@ -57,6 +58,31 @@ def _create_context_and_provider(
if CcInfo in runtime_dep:
cc_compilation_contexts.append(runtime_dep[CcInfo].compilation_context)

implementation_cc_compilation_contexts = []
for impl_dep in implementation_deps:
if apple_common.Objc in impl_dep:
# For implementation deps, we only need to propagate linker inputs
# with Objc provider, but no compilation artifacts
# (eg module_map, umbrella_header).
implementation_dep_objc_provider_kwargs = {
"force_load_library": impl_dep[apple_common.Objc].force_load_library,
"imported_library": impl_dep[apple_common.Objc].imported_library,
"library": impl_dep[apple_common.Objc].library,
"linkopt": impl_dep[apple_common.Objc].linkopt,
"sdk_dylib": impl_dep[apple_common.Objc].sdk_dylib,
"sdk_framework": impl_dep[apple_common.Objc].sdk_framework,
"source": impl_dep[apple_common.Objc].source,
"weak_sdk_framework": impl_dep[apple_common.Objc].weak_sdk_framework,
}
objc_provider = apple_common.new_objc_provider(**implementation_dep_objc_provider_kwargs)
objc_providers.append(objc_provider)
elif CcInfo in impl_dep:
cc_linking_contexts_for_merging.append(impl_dep[CcInfo].linking_context)

if CcInfo in impl_dep:
implementation_cc_compilation_contexts.append(impl_dep[CcInfo].compilation_context)
cc_linking_contexts.append(impl_dep[CcInfo].linking_context)

link_order_keys = [
"imported_library",
"cc_library",
Expand All @@ -82,6 +108,7 @@ def _create_context_and_provider(
objc_compilation_context_kwargs = {
"providers": objc_providers + runtime_objc_providers,
"cc_compilation_contexts": cc_compilation_contexts,
"implementation_cc_compilation_contexts": implementation_cc_compilation_contexts,
"public_hdrs": [],
"private_hdrs": [],
"public_textual_hdrs": [],
Expand Down
2 changes: 2 additions & 0 deletions src/main/starlark/builtins_bzl/common/objc/objc_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def _objc_library_impl(ctx):
use_pch = True,
deps = ctx.attr.deps,
runtime_deps = ctx.attr.runtime_deps,
implementation_deps = ctx.attr.implementation_deps,
attr_linkopts = ctx.attr.linkopts,
alwayslink = ctx.attr.alwayslink,
)
Expand Down Expand Up @@ -89,6 +90,7 @@ objc_library = rule(
attrs = common_attrs.union(
{
"data": attr.label_list(allow_files = True),
"implementation_deps": attr.label_list(providers = [CcInfo], allow_files = False),
},
common_attrs.CC_TOOLCHAIN_RULE,
common_attrs.LICENSES,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,21 @@ public void testCompilesSources() throws Exception {
.write();

createLibraryTargetWriter("//objc/lib2")
.setAndCreateFiles("srcs", "b.m")
.setAndCreateFiles("hdrs", "private.h")
.write();

createLibraryTargetWriter("//objc/lib3")
.setAndCreateFiles("srcs", "a.m")
.setAndCreateFiles("hdrs", "hdr.h")
.setList("deps", "//objc/lib1")
.setList("implementation_deps", "//objc/lib2")
.write();

createLibraryTargetWriter("//objc:x")
.setAndCreateFiles("srcs", "a.m", "private.h")
.setAndCreateFiles("hdrs", "hdr.h")
.setList("deps", "//objc/lib2:lib2")
.setList("deps", "//objc/lib3:lib3")
.write();

CppCompileAction compileA = (CppCompileAction) compileAction("//objc:x", "a.o");
Expand Down Expand Up @@ -1077,20 +1083,34 @@ public void testProvidesObjcLibraryAndHeaders() throws Exception {
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
.setAndCreateFiles("hdrs", "a.h", "b.h")
.write();
ConfiguredTarget impltarget =
createLibraryTargetWriter("//objc_impl:lib")
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
.setAndCreateFiles("hdrs", "a.h", "b.h")
.write();
ConfiguredTarget depender =
createLibraryTargetWriter("//objc2:lib")
createLibraryTargetWriter("//objc_depender:lib")
.setAndCreateFiles("srcs", "a.m", "b.m", "private.h")
.setAndCreateFiles("hdrs", "c.h", "d.h")
.setList("deps", "//objc:lib")
.setList("implementation_deps", "//objc_impl:lib")
.write();
assertThat(getArifactPaths(target, LIBRARY)).containsExactly("objc/liblib.a");
assertThat(getArifactPaths(depender, LIBRARY)).containsExactly(
"objc/liblib.a", "objc2/liblib.a");
assertThat(getArifactPaths(depender, LIBRARY))
.containsExactly("objc/liblib.a", "objc_impl/liblib.a", "objc_depender/liblib.a");
assertThat(getArifactPaths(impltarget, LIBRARY)).containsExactly("objc_impl/liblib.a");
assertThat(getArifactPathsOfHeaders(target))
.containsExactly("objc/a.h", "objc/b.h", "objc/private.h");
assertThat(getArifactPathsOfHeaders(impltarget))
.containsExactly("objc_impl/a.h", "objc_impl/b.h", "objc_impl/private.h");
assertThat(getArifactPathsOfHeaders(depender))
.containsExactly(
"objc/a.h", "objc/b.h", "objc/private.h", "objc2/c.h", "objc2/d.h", "objc2/private.h");
"objc/a.h",
"objc/b.h",
"objc/private.h",
"objc_depender/c.h",
"objc_depender/d.h",
"objc_depender/private.h");
}

private static Iterable<String> getArifactPaths(
Expand Down

0 comments on commit 9b3a7eb

Please sign in to comment.