Skip to content

Exposes more fields in FrameworkInfo #282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
congt merged 1 commit into from
Jul 30, 2021
Merged

Exposes more fields in FrameworkInfo #282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions rules/framework.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ def _get_virtual_framework_info(ctx, framework_files, compilation_context_fields
continue
framework_info = dep[FrameworkInfo]
fw_dep_vfsoverlays.extend(framework_info.vfsoverlay_infos)
propagated_interface_headers.append(framework_info.framework_headers)
framework_headers = depset(framework_info.headers + framework_info.modulemap + framework_info.private_headers)
Copy link
Contributor

@thiagohmcruz thiagohmcruz Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify why framework_info.framework_headers is not being appended anymore here? Is the new set already covering the same files?

Copy link
Contributor Author

@congt congt Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, framework_headers originally included headers, modulemap, private_headers.

propagated_interface_headers.append(framework_headers)

# Collect generated headers. consider exposing all required generated
# headers in respective providers: -Swift, modulemap, -umbrella.h
Expand All @@ -161,8 +162,6 @@ def _get_virtual_framework_info(ctx, framework_files, compilation_context_fields
merge_vfsoverlays = fw_dep_vfsoverlays + import_vfsoverlays,
)

framework_headers = depset(outputs.headers + outputs.modulemap + outputs.private_headers)

# Includes interface headers here ( handled in cc_info merge for no virtual )
compilation_context_fields["headers"] = depset(
direct = outputs.headers + outputs.private_headers + outputs.modulemap,
Expand All @@ -171,7 +170,11 @@ def _get_virtual_framework_info(ctx, framework_files, compilation_context_fields

return FrameworkInfo(
vfsoverlay_infos = [vfs.vfs_info],
framework_headers = framework_headers,
headers = outputs.headers,
private_headers = outputs.private_headers,
modulemap = outputs.modulemap,
swiftmodule = outputs.swiftmodule,
swiftdoc = outputs.swiftdoc,
)

def _get_framework_files(ctx):
Expand Down Expand Up @@ -453,8 +456,13 @@ def _apple_framework_packaging_impl(ctx):
if virtualize_frameworks:
framework_info = _get_virtual_framework_info(ctx, framework_files, compilation_context_fields)
else:
# This is empty by default
framework_info = FrameworkInfo()
framework_info = FrameworkInfo(
headers = outputs.headers,
private_headers = outputs.private_headers,
modulemap = outputs.modulemap,
swiftmodule = outputs.swiftmodule,
swiftdoc = outputs.swiftdoc,
)

# If not virtualizing the framework - then it runs a "clean"
_get_symlinked_framework_clean_action(ctx, framework_files, compilation_context_fields)
Expand Down
7 changes: 6 additions & 1 deletion rules/providers.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
FrameworkInfo = provider(
fields = {
"vfsoverlay_infos": "Merged VFS overlay infos, present when virtual frameworks enabled",
"framework_headers": "Headers part of the framework's public interface",
Copy link
Contributor

@thiagohmcruz thiagohmcruz Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a breaking API change right? Just checking to ensure this is intentional

Copy link
Contributor Author

@congt congt Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, FrameworkInfo was just introduced to support vfs framework by @jerrymarino. I believe it's only used internally.

Copy link
Contributor Author

@congt congt Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerrymarino PTAL

"binary": "The binary of the framework",
Copy link
Contributor

@jerrymarino jerrymarino Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is returned inside of the apple binary already - any reason to replicate here?

Copy link
Contributor Author

@congt congt Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to reuse FrameworkInfo to package a dynamic framework. By adding binary and swiftmodule here, I won't need to pass around AppleBundleInfo or swiftinfo.

Copy link
Contributor

@jerrymarino jerrymarino Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm saying is that today, upstream dependees of this rule are required to retrieve the swiftmodule, and binaries.
https://github.com/bazel-ios/rules_ios/blob/master/rules/framework.bzl#L497 - so it's already passed around. To access the attributes, just use framework[SwiftInfo].swiftmodule or the other existing ones

Copy link
Contributor Author

@congt congt Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally understood. I want to make FrameworkInfo self-contain from the API's perspective. Then downstream targets can only return FrameworkInfo without returning SwiftInfo or AppleBundleInfo.

"headers": "Headers of the framework's public interface",
"private_headers": "Private headers of the framework",
"modulemap": "The module map of the framework",
"swiftmodule": "The swiftmodule",
Copy link
Contributor

@jerrymarino jerrymarino Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These ones are in swiftinfo - any reason to put here too?

"swiftdoc": " The Swift doc",
Copy link
Contributor

@gyfelton gyfelton Jul 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what;s the relationship between swiftmoduel and swiftdoc? right now based on what i have seen it feels like swiftdoc is a child of swiftmodule... along with swiftinterface

Copy link
Contributor Author

@congt congt Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that's just how they're packaged in a framework.

Copy link
Contributor

@thiagohmcruz thiagohmcruz Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're all binary formats, you can read more about swift modules here: https://github.com/apple/swift/blob/main/docs/Serialization.md

(TL;DR functionally they're like headers that expose public APIs)

Also IIRC .swiftdoc files hold documentation related things like when you use "///" to document a public class and want its description to show up in the UI for who is consuming it (it could be holding more info than that but AFAIK this format is not documented)

},
)