-
Notifications
You must be signed in to change notification settings - Fork 39
fix: honor package level language mode configuration #1770
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple of formatting comments. Just let us know when tests are green and we can take another look.
@cgrindel while working on the failing test I realized that despite the flexibility that the existing block of code around language modes would imply, the only feature that's actually supported by Existing code: rules_swift_package_manager/swiftpkg/internal/swiftpkg_build_files.bzl Lines 144 to 151 in 0eef504
As a result we can enforce that packages that use Swift 6 language mode are correctly compiled, but we can't use that feature flag to force a package down to language mode 5 if the main project has set In order to force a package down to language mode 5 we'd have to append |
This comment was marked as outdated.
This comment was marked as outdated.
@jflan-dd I am not sure. @brentleyjones @luispadron @jpsim Do you have any thoughts? |
I think we should support pinning the language mode down to Swift 5 and only use The v6 feature potentially has other effects if the user isn't using the Swift 6+ compiler that package authors wouldn't expect.
I believe if the package used |
@jflan-dd Sounds reasonable to me. Is there anything else that you want to add to this PR before we merge? |
@cgrindel I pushed a commit to use only I don't have any other changes for this PR unless there's additional feedback during review. |
@Mergifyio queue |
🛑 The pull request has been removed from the queue
|
This pull request has been removed from the queue for the following reason: The pull request can't be updated. You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. |
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at def3045 |
Ensure targets are built with the correct Swift language mode based on package level information.
Fixes #1743