Skip to content
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.

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

Add aspect_hints feature #691

Conversation

keith
Copy link
Member

@keith keith commented Sep 24, 2021

This includes all the upstream commits related to this, which is blocked on bazel's default for this feature

@google-cla
Copy link

google-cla bot commented Sep 24, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Sep 24, 2021
keith referenced this pull request Sep 24, 2021
The new `aspect_hints` attribute in Bazel, available on all rules, lets us attach arbitrary providers to targets that `swift_clang_module_aspect` can read. The `swift_interop_hint` rule uses this to provide the module name for whichever target references it in its `aspect_hints`.

A canonical auto-deriving hint is also provided as part of the Swift build rules (in `.../swift:auto_module`), so that the default/recommended behavior of deriving a module name from the target label can be easily obtained without having to declare one's own `swift_module_hint` targets.

This is a more principled approach to handling Swift interop than the current `tags` implementation (which will be removed in a future change), and lets us associate additional metadata easily in the future, including files (for example, custom module maps or APINotes).

PiperOrigin-RevId: 387147846
@keith keith marked this pull request as ready for review September 24, 2021 02:51
@google-cla
Copy link

google-cla bot commented Sep 24, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@bazelbuild bazelbuild deleted a comment from lyft-lint-bot Sep 24, 2021
keith referenced this pull request Sep 24, 2021
This allows rules like `cc_library` to associate a custom module map if needed (however, this should be rare and used sparingly). It also eliminates the need for the `swift_c_module`, which will be removed.

Added analysis tests around the propagation of the module map artifacts.

PiperOrigin-RevId: 387195026
brentleyjones
brentleyjones previously approved these changes Sep 27, 2021
@keith
Copy link
Member Author

keith commented Oct 4, 2021

blocked on 5.x

@keith keith force-pushed the ks/add-the-swift_interop_hint-rule-to-be-used-with-aspect_hints branch from 29846cc to 8494801 Compare October 4, 2021 21:17
@google-cla
Copy link

google-cla bot commented Oct 4, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 10, 2021
@thii thii force-pushed the ks/add-the-swift_interop_hint-rule-to-be-used-with-aspect_hints branch from 8494801 to 5352de6 Compare November 24, 2021 01:00
@brentleyjones
Copy link
Collaborator

bazelbuild/bazel@47bd65a

@thii
Copy link
Member

thii commented Nov 24, 2021

Many other changes rely on this now. Let's enable the flag and mention about this requirement in the release notes.

@brentleyjones
Copy link
Collaborator

I'm not sure we want to force that flag on all users of rules_swift. Let's see if @allevato has any additional thoughts.

@allevato
Copy link
Member

I'm not sure we want to force that flag on all users of rules_swift. Let's see if @allevato has any additional thoughts.

The only real advice I can offer is to bring up to the Bazel team the difficulty the experimental nature of the feature poses to integrating upstream rules_swift changes (especially as they stack on each other). Unfortunately the decision to make the flag experimental in Bazel was made some time after the feature was implemented and the Swift rules had started adopting it...

@keith keith force-pushed the ks/add-the-swift_interop_hint-rule-to-be-used-with-aspect_hints branch from 5352de6 to 7088ec5 Compare January 24, 2022 18:54
keith referenced this pull request Jan 24, 2022
…using `swift_interop_hint`.

PiperOrigin-RevId: 387355219
keith referenced this pull request Jan 24, 2022
Its functionality has been replaced by `swift_import_hint` and `aspect_hints`.

PiperOrigin-RevId: 387358449
keith referenced this pull request Jan 24, 2022
keith referenced this pull request Jan 24, 2022
Interop for non-Obj-C rules should now exclusively use `aspect_hints` (see the documentation for `swift_interop_hint`).

PiperOrigin-RevId: 388940287
keith referenced this pull request Jan 24, 2022
… that generate them by default, like `objc_library`.

PiperOrigin-RevId: 391087374
keith referenced this pull request Jan 24, 2022
This attribute can be used to exclude a list of headers from the Swift-generated module map (via `exclude header` declarations) without removing them from the hinted target completely. This is often helpful in cases where some subset of headers are not Swift-compatible but still needed as part of the library for other reasons (e.g., they are private headers used by implementation source files, or still used by other non-Swift dependents).

PiperOrigin-RevId: 398076709
keith referenced this pull request Jan 24, 2022
…t_interop_hint.exclude_hdrs` on a `cc_library` that has `include_prefix` and/or `strip_include_prefix` set.

PiperOrigin-RevId: 412917671
keith referenced this pull request Jan 24, 2022
… J2ObjC

Targets processed by J2ObjC's aspects have a new compilation context created that treats the generated J2ObjC headers as textual headers, and the "umbrella" header as a regular header. The root path of the generated headers is added to the compilation context as an include path so the headers can be found by dependents.

To handle `java_libary`'s `exports` attribute (this lists targets that should be treated as a direct dependency when depending on the `java_libary` in question), the `SwiftInfos` from dependencies are split into direct and indirect. These are then propagated as `direct_swift_infos` vs `swift_infos` respectively to provide the semantics of `exports`.

The modular import header rewriter was updated to accommodate the fact that the J2ObjC "umbrella" header is no longer marked as being an umbrella header in the module map. It still needs to be rewritten for the same reason as before, but must now be detected by a portion of the file path (🤮).

PiperOrigin-RevId: 416355827
keith referenced this pull request Jan 24, 2022
…o that

layering checks are still satisfied correctly.

PiperOrigin-RevId: 416824355
keith referenced this pull request Jan 24, 2022
The direct_headers field in ObjcProvider is being deprecated and will
be removed.  The same information can be found in CcInfo.
J2ObjcAspect has been modified so that the CcInfo is accessible via
Starlark.

PiperOrigin-RevId: 417648590
allevato and others added 11 commits April 25, 2022 11:50
…t_interop_hint`.

PiperOrigin-RevId: 388242317
(cherry picked from commit 6ded44e)
Interop for non-Obj-C rules should now exclusively use `aspect_hints` (see the documentation for `swift_interop_hint`).

PiperOrigin-RevId: 388940287
(cherry picked from commit a67043f)
… that generate them by default, like `objc_library`.

PiperOrigin-RevId: 391087374
(cherry picked from commit de0f604)
This attribute can be used to exclude a list of headers from the Swift-generated module map (via `exclude header` declarations) without removing them from the hinted target completely. This is often helpful in cases where some subset of headers are not Swift-compatible but still needed as part of the library for other reasons (e.g., they are private headers used by implementation source files, or still used by other non-Swift dependents).

PiperOrigin-RevId: 398076709
(cherry picked from commit ef6662c)
…t_interop_hint.exclude_hdrs` on a `cc_library` that has `include_prefix` and/or `strip_include_prefix` set.

PiperOrigin-RevId: 412917671
(cherry picked from commit 6b13232)
… J2ObjC

Targets processed by J2ObjC's aspects have a new compilation context created that treats the generated J2ObjC headers as textual headers, and the "umbrella" header as a regular header. The root path of the generated headers is added to the compilation context as an include path so the headers can be found by dependents.

To handle `java_libary`'s `exports` attribute (this lists targets that should be treated as a direct dependency when depending on the `java_libary` in question), the `SwiftInfos` from dependencies are split into direct and indirect. These are then propagated as `direct_swift_infos` vs `swift_infos` respectively to provide the semantics of `exports`.

The modular import header rewriter was updated to accommodate the fact that the J2ObjC "umbrella" header is no longer marked as being an umbrella header in the module map. It still needs to be rewritten for the same reason as before, but must now be detected by a portion of the file path (🤮).

PiperOrigin-RevId: 416355827
(cherry picked from commit dd22a51)
…o that

layering checks are still satisfied correctly.

PiperOrigin-RevId: 416824355
(cherry picked from commit 7c7ee72)
The direct_headers field in ObjcProvider is being deprecated and will
be removed.  The same information can be found in CcInfo.
J2ObjcAspect has been modified so that the CcInfo is accessible via
Starlark.

PiperOrigin-RevId: 417648590
(cherry picked from commit 612725e)
@keith keith force-pushed the ks/add-the-swift_interop_hint-rule-to-be-used-with-aspect_hints branch from f8f5cbe to 6e3d067 Compare April 25, 2022 18:50
@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/343 (must be Lyft employee to view)

keith referenced this pull request Apr 25, 2022
RELNOTES: None
PiperOrigin-RevId: 430276243
keith and others added 3 commits April 25, 2022 11:54
…iptor in `swift_clang_module_aspect`.

This makes it so that the `apple_common.Objc` provider no longer needs to be handled separately for compilation by Swift build APIs. (It is still used for linking until that is migrated entirely onto `CcInfo`.)

PiperOrigin-RevId: 423822059
(cherry picked from commit 8ce9595)
RELNOTES: None
PiperOrigin-RevId: 430276243
(cherry picked from commit 01b9d32)
@keith keith force-pushed the ks/add-the-swift_interop_hint-rule-to-be-used-with-aspect_hints branch from 0f12075 to e310985 Compare April 25, 2022 18:56
keith referenced this pull request Apr 25, 2022
…iptor in `swift_clang_module_aspect`.

This makes it so that the `apple_common.Objc` provider no longer needs to be handled separately for compilation by Swift build APIs. (It is still used for linking until that is migrated entirely onto `CcInfo`.)

PiperOrigin-RevId: 423822059
@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/345 (must be Lyft employee to view)

@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/347 (must be Lyft employee to view)

@keith keith force-pushed the ks/add-the-swift_interop_hint-rule-to-be-used-with-aspect_hints branch from 3c042b7 to 4db9296 Compare April 25, 2022 21:11
@lyft-lint-bot
Copy link

Lyft integration job started: https://buildkite.com/lyft/rules-swift/builds/350 (must be Lyft employee to view)

@luispadron
Copy link
Contributor

Dang i think this is blocking cherry-picking bfcae19 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants