swift-composable-architecture icon indicating copy to clipboard operation
swift-composable-architecture copied to clipboard

Support building via Bazel

Open DJBen opened this issue 2 years ago • 6 comments

Overview

This PR adds support for building via Bazel. Bazel has been an increasingly adopted norm for medium and large mobile projects. Given the popularity of this repo as the de facto architecture standard, this enables wider adoption throughout the iOS community.

Verifications

bazel build :all
bazel test :all

Action items

  • Move the BUILD file declarations of third_party/ to their respective repos.
  • Add an example project that integrates with Bazel.
  • Consider migrating dependencies from WORKSPACE to MODULE.bazel
  • [ ] CI pipeline for the Bazel workflow.

DJBen avatar Jan 12 '24 06:01 DJBen

@DJBen Thanks for taking the time to PR this! We definitely know some of TCA's users would benefit from a Bazel workflow and would appreciate this. As project maintainers, we are not super familiar with Bazel, so I think there are a few concerns we have that we'd like to address before we can consider merging:

  • This PR adds eight new files to the root directory of the repo. This will end up being over 1/3 of all the files at the root, which makes it a bit noisier for folks browsing the source code, especially those who are not familiar with Bazel. Is there any way to quarantine things to a single place? Does Bazel require this structure?
  • While we aren't opposed to maintaining this code in the core repo, we do think it's necessary to establish continuous integration that proves it is working. That way, if anything breaks in the future, we can ping you or other Bazel users for help, or, if we can't maintain a working version of Bazel, we'll have a signal that it may be time for us to remove support from the repo till someone can address it. Can you please add a GitHub action to this PR that exercises Bazel?
  • We were also wondering if this code needs to be in the main repo at all. Is it possible to maintain a swift-composable-architecture-bazel support repo without having to fork our entire project? If so we'd be happy to link to the repo from the README and documentation. It'd also ensure that the code was in a place that is more likely to be maintained.

stephencelis avatar Jan 12 '24 18:01 stephencelis

Hi @stephencelis! Thank you for your quick feedback!

This PR adds eight new files to the root directory of the repo.

It is the convention and/or design of Bazel to have these files on the root level unfortunately. A prominent example with Bazel integration is https://github.com/apple/swift-syntax and I haven't heard from them bothering with the root level files :):

  • .bazelrc / .bazelversion: configs. They are hidden from Finder, e.g. .git.

  • WORKSPACE: this is more akin to the Package.swift's package declaration section.

  • MODULE.bazel: this is introduced in Bazel version 7 as a future substitution of WORKSPACE to declare package dependencies.

  • MODULE.bazel.lock: Like Podfile.lock, I can exclude them from source control.

  • BUILD.bazel: this is comparable to the declared target and product declaration of the Package.swift.

  • third_party/*: they are BUILD files we declare for the dependencies that do not yet have Bazel integration. If we get consensus on Bazel integration for all pointfreeco/ repos, we can merge them into their respective repos and eliminate this folder.

  • deps.bzl: Shared function to declare dependencies. I can move it to a bazel_integration folder for clarity.

  • [x] Move deps.bzl file to directory

  • [x] Remove MODULE.bazel.lock.

Can you please add a GitHub action to this PR that exercises Bazel?

Will do!

  • [ ] Establish CI for Bazel build pipeline

Is it possible to maintain a swift-composable-architecture-bazel support repo without having to fork our entire project?

It is technically possible for any third party repo to integrate with TCA using Bazel even files in this PR do not exist here - they would have done the same thing via http_archive(..., build_file: ...) and supply dependent BUILD files in third_party. That's what some big companies have already spent time and resources doing :)

Without Bazel presence in the main repo, it will also be a constant maintenance hassle to update the version and archive link.

As our iOS community getting more exposed to Bazel, more people will expect to see BUILD files in the main repo and volunteer to update them according to the needs.

Let me know if my action items and points make sense. Appreciate your input!

DJBen avatar Jan 12 '24 19:01 DJBen

Without Bazel presence in the main repo, it will also be a constant maintenance hassle to update the version and archive link.

One option would be to promote a designated Bazel repo from our main repo. That way there would be a documented default for folks to use, and the Bazel integration in the main repo wouldn't be a build system that could become brittle over time. The maintenance burden is for sure on that separate repo, but it would be contained. We're also a little unsure if supporting Bazel in one repo would require bleeding out to all of its dependencies.

After looking into SwiftSyntax's Bazel integration, I noted a few things:

  1. Keith Smiley used to maintain a separate repo: https://github.com/keith/swift-syntax-bazel/
  2. He suggested merging it upstream here: https://github.com/apple/swift-syntax/pull/1251
  3. Doug Gregor from the Swift team noted that they do not want to maintain it, so it'd be Keith's responsibility: https://github.com/apple/swift-syntax/pull/1251#pullrequestreview-1369329813
  4. Keith has been actively maintaining it, e.g.: https://github.com/apple/swift-syntax/pull/1796

While we're open to the approach Doug has taken, we'd require the same level of maintenance from you and other Bazel contributors in the future, and if CI does begin to fail we may have to remove support in the future if the issues aren't addressed. TCA also has an ongoing Observation beta that should hopefully be merged in the coming weeks. This adds new dependencies and is likely to break the Bazel build soon.

So stepping back, I do wonder if as a first step we could link out to a designated Bazel repo, instead. And then maybe we could start a GitHub discussion about what it would mean to move to support directly in this repo. I think we just want to best understand all the trade-offs.

stephencelis avatar Jan 12 '24 19:01 stephencelis

That sounds good. I'll provide a separate repo. In the future I am more than happy to take the maintenance burden of Bazel integration.

What is your stance regarding integrating Bazel directly into much smaller leaf dependencies like swift-custom-dump, swift-clocks, combine-schedulers? For example In my previous company we'd hoped to integrate with swift-custom-dump with Bazel directly but alas it didn't exist. It would be relatively smaller footprint and impact.

DJBen avatar Jan 12 '24 19:01 DJBen

I think our same concerns apply, and maybe more so, since it increases the number of maintenance requirements to potentially all of our libraries. While we understand the needs of large enterprise teams to use Bazel, we do wish Bazel made it easier to layer support onto a project without getting the project itself involved.

stephencelis avatar Jan 12 '24 20:01 stephencelis

I made the bazel integration available at https://github.com/DJBen/swift-composable-architecture-bazel. Thanks for your feedback! Feel free to close this PR.

DJBen avatar Jan 12 '24 21:01 DJBen

For others trying to build this project with Bazel, I went down this route, but then found https://github.com/cgrindel/rules_swift_package_manager, which has been working great. It uses Package.swift as the source for dependency versions and generates build definitions, so the versions should line up, without needing any custom forks. I'm not sure if there are any drawbacks yet, but it's been working so far for this and 30 or so different dependencies I have tested.

joprice avatar Jan 15 '24 19:01 joprice

@DJBen Wanna fix up this PR (or submit another) that links to your Bazel support library in the README?

Also side note but the README on your project has a small typo ("Crchitecture").

stephencelis avatar Jan 15 '24 21:01 stephencelis

I'll close this one; experiment with the approach from @joprice, and add another separate PR for the Bazel support in README separately

DJBen avatar Jan 17 '24 01:01 DJBen