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

Pattern for managing swift compilation improvements #251

Closed
wants to merge 2 commits into from

Conversation

jerrymarino
Copy link
Contributor

@jerrymarino jerrymarino commented Jun 8, 2021

This PR is a pattern to add required features to swift compilation like
AST dumping ( bazelbuild/rules_swift#623 ) and
Index While Building V2 (
bazelbuild/rules_swift#567 ) for rules_ios in a
maintainable way without ditching rules_swift altogether. It does this
in way where we can create our own toolchain and add our own features in
it, and integrate them or replace upstream features quite nicely.

It achieves this by providing:

  1. a swift_library rule to override attributes
  2. a "toolchain" composed of existing toolchains but allows full overriding
    of all attributes and most logic.

The benefit is that we can add add new action configs ( configurators ),
and run improved worker code without maintaining a fork. Longer term we
can propose features and gradually get them upstreamed.

For now, it bases a swift_library rule on the rules_swift rule, which is
Apache compatible. Longer term, we may consider calling
swift_common.compile directly and get rid of the extra rule in our
stack. I suspect if rules_swift uses register_toolchain idiom we may
also be able to get rid of this.

Now it works without worker.

- Add the updated flag for IWBV2
- Remove index-import as it's now baked in

Note: please hold until the PR at rules_swift merges
bazelbuild/rules_swift#567
This PR is a pattern to add required features to swift compilation like
AST dumping ( bazelbuild/rules_swift#623 )  and
Index While Building V2 (
bazelbuild/rules_swift#567 ) for rules_ios in a
maintainable way without ditching rules_swift altogether. It does this
in way where we can create our own toolchain and add our own features in
it, and integrate them or replace upstream features quite nicely. It
achieves this by providing a swift_libray rule, and "composed" a
toolchain which calls existing code but allows full overriding these
features need.

The benefit is that we can add add new action configs ( configurators ),
and run improved worker code without maintaining a fork. Longer term we
can propose features and gradually get them upstreamed.

For now, it bases a swift_library rule on the rules_swift rule, which is
Apache compatible. Longer term, we may consider calling
swift_common.compile directly and get rid of the extra rule in our
stack. I suspect if rules_swift uses register_toolchain idiom we may
also be able to get rid of this.
@jerrymarino jerrymarino force-pushed the jmarino/composable_swift_toolchain branch from fd24513 to 1be1d29 Compare June 8, 2021 21:50
@jerrymarino
Copy link
Contributor Author

For now this PR is based on the rules_swift update branch which contained the changes for index while building V2. We'll need another PR to migrate index while building V2 into the toolchain and appending it's swift_toolchain_config.action_config. I suspect we can take a similar approach for bazelbuild/rules_swift#623

Base automatically changed from jmarino/update_rules_swift to master June 9, 2021 18:35
@jerrymarino
Copy link
Contributor Author

Taking a different approach for this

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.

1 participant