Use Swift info provider during framework packaging to avoid symbol redefinition #53
Conversation
The order doesn't matter, it's functionally a set of the different providers -- there can only be one provider of each "type" per target returned
It's a "splat" that turns key-value pairs from a dictionary into keyword arguments
yes, they are counterparts. They don't represent source files, but rather are bazel "providers", which are the ways of propagating information about how to consume the target to the other targets that depend on this one |
9fc92cb to
a613799
Compare
| @@ -25,7 +25,7 @@ def apple_framework(name, apple_library = apple_library, **kwargs): | |||
|
|
|||
| def _find_framework_dir(outputs): | |||
| for output in outputs: | |||
| prefix = output.path.split(".framework/")[0] | |||
| prefix = output.path.rsplit(".framework/", 1)[0] | |||
There was a problem hiding this comment.
can give an example of how this changes what's inside the prefix variable?
There was a problem hiding this comment.
It only would if output.path contained multiple instances of .framework, in which case now it will return the innermost framework, instead of the outermost
There was a problem hiding this comment.
so a.framework/b/c.framework/c to give a.framework/b/c.framework
| ] | ||
|
|
||
| return [ | ||
| apple_common.new_objc_provider(**objc_provider_fields), |
There was a problem hiding this comment.
if no change on objc_provider, why not having objc_provider = apple_common.new_objc_provider(**objc_provider_fields) as b4?
There was a problem hiding this comment.
there's no reason to capture the providers in locals, since they're only returned and never read from.
| return [objc_provider, swift_provider, default_info_provider] | ||
| # gather swift info fields | ||
| swift_info_fields = { | ||
| "swift_infos": [dep[SwiftInfo] for dep in ctx.attr.transitive_deps if SwiftInfo in dep], |
There was a problem hiding this comment.
for each of the transitive dependencies of ctr.attr (the current context we are in), if there is SwiftInfo in dep, take it out and add to an array. This array then becomes the value with key = "swift_infos"
|
|
||
| if swiftmodule_out: | ||
| # only add a swift module to the SwiftInfo if we've actually got a swiftmodule | ||
| swiftmodule_name = paths.split_extension(swiftmodule_in.basename)[0] |
There was a problem hiding this comment.
example of paths? I am confused on this one
Also why not just use info inside swiftmodule_in as the swiftmodule_name ? I would look at what's actually inside the swiftmodule_in to figure this part out
| # Swift won't find swiftmodule files inside of frameworks whose name doesn't match the | ||
| # module name. It's annoying (since clang finds them just fine), but we have no choice but to point to the | ||
| # original swift module/doc, so that swift can find it. | ||
| swift_module = swift_common.create_swift_module( |
There was a problem hiding this comment.
so this overrides the swift_module we just created because its name does not match the framework name...
| load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library") | ||
|
|
||
| swift_library( | ||
| name = "frameworkBar", |
There was a problem hiding this comment.
What do you mean by frameworkBar? Could you add some information explaining what is being tested in this example? Or change the frameworks names so it is more clear what they are used for?
There was a problem hiding this comment.
Can give some suggestion on the name? In this case what happened is there is a target to compile a swift library called frameworkBar that depends FrameworkFoo, then it was packaged into a framework with a name Unique_FrameworkBar which also depends on FrameworkFoo. Without the change introduced in this PR, it will result in redefinition of module FrameworkBar because it is not properly propagated through the swift info. Does this makes sense?
With updates on rules_swift, if a framework
Foodepends on targetBar1andBar2, whileBar1also depends onBar2, without changes in this PR, bazel will produce 2 module maps for Bar2, causes symbol redefinition error (inbazel-out, it will produce Bar2.framework andBar2-modulemapwhich containsBar2.modulemap).The fix is to provide the swift_info while packing framework
Fooso thatrules_swiftwon't produce theBar2-modulemap.This PR also points to latest
rules_swiftandrules_applethat we can support.This PR also removes a patch fix that is no longer useful with the intro of swift_info
Major work is done by @segiddins in this PR