-
Notifications
You must be signed in to change notification settings - Fork 439
Add bazel configuration #1251
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 bazel configuration #1251
Conversation
For context through an offline conversation with @ahoppen we decided I should submit a PR for wider discussion. Currently users are encouraged to use this wrapper repo https://github.com/keith/swift-syntax-bazel/ which includes the same BUILD file (and previously automatically statically linked the C++ library for you). I can continue maintaining that repo if this feels like too much overhead, but I think there is some value in centralizing this as I know quite a few companies who have written tools based on swift-syntax that are built with bazel. This would also be nicer for adding swift-syntax to bazel's central dependency registry as opposed to pointing to a repo like mine. We could also enable this on CI if we wanted to validate it, but in this PR I've chosen to act like the LLVM bazel setup which means it will be updated with fixes async but also not block folks who don't care about bazel. |
This would be useful for us at Airbnb. |
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.
It makes sense to me to have this Bazel configuration here in the repository. Should the README be updated with Bazel build instructions, or is the presence of the Bazel build files sufficient for most users?
7d364e4
to
f498529
Compare
I added a small blurb to the README with basic instructions. It's pretty vanilla configuration so overall it should be pretty natural for folks to include |
@swift-ci please test |
@ahoppen @DougGregor can you review 🙏🏻 |
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.
My apologies for the long delay in responding here. This is a tricky area for us, because we are already maintaining and testing two distinct build systems (SwiftPM and CMake) that are necessary for the project. The primary maintainers do not feel like they have the capacity to maintain support for a third build system, both due to lack of experience with Bazel as well as ongoing maintenance of additional Continuous Integration to ensure that the Bazel build remains healthy.
That said, we recognize the benefits to the Swift community of having Bazel support in this repository, so we are willing to try this out. We can merge in this Bazel configuration, but we want to clearly call out that you are responsible for its ongoing maintenance (as well as any co-maintainers you'd like to nominate), including responding to any issues raised against Bazel support in this repository. I've made some suggestions in the code review. If in the future the Bazel configuration is not being maintained well---for example, reported issues with it are going unaddressed for long periods of time---we will deprecate and remove it from the repository. It's possible that someone could bring up community-hosted CI to catch Bazel issues more quickly when they arise.
(Pinging @keith in case you missed the above, sorry again for the delay) |
This allows bazel users to depend on these libraries directly through the SwiftSyntax repo instead of everyone having to provide their own duplicate BUILD files. Since swift-syntax is pretty uniform this is setup using a simple bazel macro that uses the conventions of swiftpm to setup each library. This also includes a bazel target for always building a library with optimizations which can be useful if you want the improved performance and aren't trying to debug swift-syntax itself.
Thanks for the review! Those conditions are totally reasonable. We can add more folks to the list if the issue count gets out of hand, but I don't expect it will. |
a5283eb
to
0a73615
Compare
@swift-ci please test and merge |
@swift-ci please test |
1 similar comment
@swift-ci please test |
@swift-ci please test Windows |
@swift-ci please clean test Windows |
All set! Merging. @keith would you mind cherry-picking this to the |
Thanks! #1488 (I believe I have access here I think it was a blip earlier when starting CI failed for me) |
This allows bazel users to depend on these libraries directly through the SwiftSyntax repo instead of everyone having to provide their own duplicate BUILD files. Since swift-syntax is pretty uniform this is setup using a simple bazel macro that uses the conventions of swiftpm to setup each library. This also includes a bazel target for always building a library with optimizations which can be useful if you want the improved performance and aren't trying to debug swift-syntax itself.
Build instructions: