Skip to content

Commit

Permalink
Merge clang/swift modules into one direct_module in SwiftInfo for a…
Browse files Browse the repository at this point in the history
… framework (#916)

### What changed:
Rework of `_get_merged_swift_info` by bubble up the creation of clang
module to `_apple_framework_packaging_impl` level, where it determine
how to populate the clang module based on VFS feature on/off.
Remove the need to pass `virtualize_frameworks` flag to the private
method `_get_merged_swift_info`

### Why this change
add `fail(swift_info.direct_modules)` after creation of it at the end of
the packging impl method when running `bazel build
//tests/ios/frameworks/testonly/... --config=ios --config=vfs`, you get
```
[
struct(
clang = struct(compilation_context = <unknown object com.google.devtools.build.lib.rules.cpp.CcCompilationContext>, module_map = None, precompiled_module = None), compilation_context = None, is_system = False, name = "SwiftLibrary", 
swift = None),

 struct(
 clang = None, compilation_context = None, is_system = False, name = "SwiftLibrary",
 swift = struct(ast_files = (), defines = (), indexstore = None, plugins = [], private_swiftinterface = None, swiftdoc = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftdoc>, swiftinterface = None, swiftmodule = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftmodule>, swiftsourceinfo = None, symbol_graph = None))
]
```
with the current impl in `master` branch. Note that for the same module
`SwiftLibrary`, there are two structs representing it, one has clang and
no swift, and one has swift and nothing inside `clang`.

So this PR effectively merge two, but the clang module generation differ
based on VFS turn on or not:
with VFS on:
```
[struct(
    clang = struct(compilation_context = <unknown object com.google.devtools.build.lib.rules.cpp.CcCompilationContext>, module_map = None, precompiled_module = None), compilation_context = None, is_system = False, name = "SwiftLibrary", swift = None), struct(clang = None, compilation_context = None, is_system = False, name = "SwiftLibrary", 
 
   swift = struct(ast_files = (), defines = (), indexstore = None, plugins = [], private_swiftinterface = None, swiftdoc = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftdoc>, swiftinterface = None, swiftmodule = <generated file tests/ios/frameworks/testonly/SwiftLibrary.swiftmodule>, swiftsourceinfo = None, symbol_graph = None))
]
```
with VFS off:
```
[struct(

clang = struct(compilation_context = <unknown object com.google.devtools.build.lib.rules.cpp.CcCompilationContext>, module_map = <generated file tests/ios/frameworks/testonly/SwiftLibrary/SwiftLibrary.framework/Modules/module.modulemap>, precompiled_module = None), compilation_context = None, is_system = False, name = "SwiftLibrary", 

swift = struct(ast_files = (), defines = (), indexstore = None, plugins = [], private_swiftinterface = None, swiftdoc = <generated file tests/ios/frameworks/testonly/SwiftLibrary/SwiftLibrary.framework/Modules/SwiftLibrary.swiftmodule/arm64.swiftdoc>, swiftinterface = None, swiftmodule = <generated file tests/ios/frameworks/testonly/SwiftLibrary/SwiftLibrary.framework/Modules/SwiftLibrary.swiftmodule/arm64.swiftmodule>, swiftsourceinfo = None, symbol_graph = None))
]
```
This way we now have one direct_module instead of two representing the
same `SwiftLIbrary` module. This is important when trying to collect
providers for `docc_archive` rule on rules_apple side as otherwise we
face `in dep attribute of docc_archive rule <some_target_docc>:
'<some_target>' does not have mandatory providers: 'DocCBundleInfo' or
'DocCSymbolGraphsInfo'.` error.

### Tests done
1. tested with downstream big repo's full CI job (which has VFS on +
bazel version 7.2.0)
2. CI job itself being green
  • Loading branch information
gyfelton committed Sep 18, 2024
1 parent 6c6d621 commit e74047f
Showing 1 changed file with 25 additions and 29 deletions.
54 changes: 25 additions & 29 deletions rules/framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ def _create_swiftmodule(attrs):
**kwargs
)

def _copy_swiftmodule(ctx, framework_files, virtualize_frameworks):
def _copy_swiftmodule(ctx, framework_files, clang_module):
inputs = framework_files.inputs
outputs = framework_files.outputs

Expand All @@ -578,39 +578,23 @@ def _copy_swiftmodule(ctx, framework_files, virtualize_frameworks):
# original swift module/doc, so that swift can find it.
swift_module = _create_swiftmodule(inputs)

# Setup the `clang` attr of the Swift module for non-vfs case this is required to have it locate the modulemap
# and headers correctly.
clang = None
if not virtualize_frameworks:
module_map = outputs.modulemaps[0] if outputs.modulemaps else None
clang = swift_common.create_clang_module(
module_map = module_map,
compilation_context = cc_common.create_compilation_context(
headers = depset(_compact(
outputs.headers +
outputs.private_headers +
[module_map],
)),
),
)

return [
# only add the swift module, the objc modulemap is already listed as a header,
# and it will be discovered via the framework search path
swift_common.create_module(
name = swiftmodule_name,
clang = clang,
clang = clang_module,
swift = swift_module,
),
]

def _get_merged_swift_info(ctx, swift_module_context, framework_files, transitive_deps, virtualize_frameworks):
def _get_merged_swift_info(ctx, framework_files, transitive_deps, clang_module):
swift_info_fields = {
"swift_infos": [dep[SwiftInfo] for dep in transitive_deps if SwiftInfo in dep],
"modules": [swift_module_context],
}
if framework_files.outputs.swiftmodule:
swift_info_fields["modules"] += _copy_swiftmodule(ctx, framework_files, virtualize_frameworks)
swift_info_fields["modules"] = _copy_swiftmodule(ctx, framework_files, clang_module)

return swift_common.create_swift_info(**swift_info_fields)

def _merge_root_infoplists(ctx):
Expand Down Expand Up @@ -1108,15 +1092,27 @@ def _apple_framework_packaging_impl(ctx):
bundle_outs = _bundle_static_framework(ctx, is_extension_safe = is_extension_safe, current_apple_platform = current_apple_platform, outputs = outputs)
avoid_deps_info = AvoidDepsInfo(libraries = depset(avoid_deps).to_list(), link_dynamic = False)

# rules_swift 2.x no longers takes compilation_context from CcInfo, need to pass it in via SwiftInfo
swift_module_context = swift_common.create_module(
name = ctx.attr.name,
clang = swift_common.create_clang_module(
compilation_context = compilation_context,
# rules_swift 2.x no longers takes compilation_context from CcInfo, need to pass it in via SwiftInfo's clang_module
if virtualize_frameworks:
clang_module = swift_common.create_clang_module(
module_map = None,
),
)
swift_info = _get_merged_swift_info(ctx, swift_module_context, framework_files, transitive_deps, virtualize_frameworks)
compilation_context = compilation_context,
)
else:
# Setup the `clang` attr of the Swift module for non-vfs case this is required to have it locate the modulemap
# and headers correctly.
module_map = outputs.modulemaps[0] if outputs.modulemaps else None
clang_module = swift_common.create_clang_module(
module_map = module_map,
compilation_context = cc_common.create_compilation_context(
headers = depset(_compact(
outputs.headers +
outputs.private_headers +
[module_map],
)),
),
)
swift_info = _get_merged_swift_info(ctx, framework_files, transitive_deps, clang_module)

# Build out the default info provider
out_files = _compact([outputs.binary, outputs.swiftmodule, outputs.infoplist])
Expand Down

0 comments on commit e74047f

Please sign in to comment.