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

Remove swift info provider creation inside hmap rule #919

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

gyfelton
Copy link
Contributor

@gyfelton gyfelton commented Sep 17, 2024

Last place that touched this code is on https://github.com/bazel-ios/rules_ios/pull/906where I thought it's important to construct SwiftInfo provider for hmap and add compilation context to pass the failing test.
However that leads to a new issue with docc_archive rule with latest rules_apple/rules_swift. We realized it's actually not needed to provide any SwiftInfo provider at all or it will "confuse" rules_swift/apple (since newer version starts to rely more on clang info from Swift info itself instead apple_common.Objc). Removing this also have the original failing test passing.

Tests done:

  • my own downstream repo still builds fine.
  • CI is green

Will squash before merge as it contains already merged commits

@gyfelton gyfelton changed the title [WIP] Remove swift info creation inside hmap [WIP] Remove swift info creation inside hmap rule Sep 18, 2024
@gyfelton gyfelton marked this pull request as ready for review September 18, 2024 13:38
@gyfelton gyfelton changed the title [WIP] Remove swift info creation inside hmap rule Remove swift info provider creation inside hmap rule Sep 18, 2024
module_map = None,
),
)],
)
providers = [
DefaultInfo(
files = depset([headermap]),
),
apple_common.new_objc_provider(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should try to remove this as well

Copy link
Contributor Author

@gyfelton gyfelton Sep 18, 2024

Choose a reason for hiding this comment

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

my understanding is bazel 6 still need it. but i can be wrong.
I feel that demands a separate PR to just remove all apple_common.Objc references? Also helps when have test against bazel "7.4" / nightly as the API apple_common.Objc is just gone now: https://bazel.build/rules/lib/toplevel/apple_common

@gyfelton gyfelton enabled auto-merge (squash) September 18, 2024 21:09
@gyfelton gyfelton merged commit 9e72bb6 into master Sep 19, 2024
23 checks passed
@gyfelton gyfelton deleted the yuanfeng/remove-swift-info-passing-hmap branch September 19, 2024 00:38
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.

3 participants