search

bazel-ios / rules_ios

Bazel rules for building iOS applications and frameworks
Apache License 2.0
279 stars 85 forks source link

Feature request: Add support for Bzlmod #704

Closed luispadron closed 11 months ago

luispadron commented 1 year ago

Request

What?

We'd like for rules_ios to support bzlmod. In Bazel 6.0.0 bzlmod became non-experimental and much of the community (i.e. rules_apple, rules_swift, etc) have already enabled support for it.

Why?

Tasks

I've started a draft PR over in #703, I have very little experience with bzlmod but using this as a way to learn and help integrate it for rules_ios.

If you have ideas and thoughts, comment here and we can work together to design and implement this.

luispadron commented 1 year ago

I've gotten pretty far (i think) in the current PR. I'm currently stuck adding support for the cocoapods and carthage repos. There seems to be a starlark crash somewhere in the repository rule.

I've been testing with a simple bazel build //..., the .bazelrc has been modified to --enable_bzlmod (we'd remove this before merging) to make it easier to test.

thiagohmcruz commented 1 year ago

+1 for adding this in! All items in "Why?" make a lot of sense to me. I've never played with bzlmod so we're on the same boat haha.

Dumb questions:

  1. Besides making CI 🟢 in that PR is there a way to continuously test if it's working as expected and know on CI jobs if say a PR breaks this? Not sure if this is common pattern or even necessary.
  2. I assume we'll need to update docs for this, what comes to mind: main README, release instructions if anything changes there. I think that's it?
  3. Does this forces is to pick a more structured versioning system (e.g. SemVer) or is it flexible and we can solve that later on?
luispadron commented 1 year ago

@thiagohmcruz From what I know/understand:

  1. Besides making CI 🟢 in that PR is there a way to continuously test if it's working as expected and know on CI jobs if say a PR breaks this? Not sure if this is common pattern or even necessary.

I think we can keep the jobs as-is, they'd use the WORKSPACE and not bzlmod and we could look at adding another job which flips --enable_bzlmod to ensure both are supported. It sucks but I think while both are being supported by Bazel we'll need to maintain both a WORKSPACE and MODULE.bazel. In practice though this doesn't seem to bad since we already define most of our public deps in repositories.bzl which can be shared between both.

  1. I assume we'll need to update docs for this, what comes to mind: main README, release instructions if anything changes there. I think that's it?

Yeah totally, I've seen repos like rules_apple provide instructions for Bzlmod by default and then provide WORKSPACE instructions like we currently have today with the "legacy" tag. Additionally, with bzlmod we need to keep MODULE.bazel in-sync with the release version numbers so we'll need to update the release workflow to generate that snippet during releases.

  1. Does this forces is to pick a more structured versioning system (e.g. SemVer) or is it flexible and we can solve that later on?

Don't think this changes much, theres more info here on version format: https://bazel.build/external/module#version_format We've already started tagging semver like releases so we could just continue that.

luispadron commented 1 year ago

Got some tests going ✅ , taking a break to understand the current issues more.

Some things worth discussing about bringing over:

mattrobmattrob commented 1 year ago
  • Should we remove cocoapods and carthage rules? Are these in use? They require some custom local repos and dependencies

We do not use these in our project (instead just the cocoapods-bazel approach for static BUILD files from CocoaPods). I'd love to be able to get rid of them if they aren't in use. 👍

thiagohmcruz commented 1 year ago

@luispadron @mattrobmattrob

Should we remove cocoapods and carthage rules? Are these in use? They require some custom local repos and dependencies

I honestly don't know. I wouldn't drop the use cases without knowing if it's in use indeed. For CocoaPods might potentially be fine because of cocoapods-bazel as @mattrobmattrob said, but wouldn't know what to do with Carthage... Maybe make these optionally loaded instead so if there's someone using we can tell them to add the opt-in flag and if we don't see anyone complaining in a while we drop it?

Should we define the generator to use with bzlmod by default, otherwise we may need to create an extension to allow folks to include these in separately

By "generator" you mean the Xcode generator? If "by default" you mean loading it by default with rules_ios we definitely should, it is still the "default" generator and I'd keep it that way until more people are migrated over and using other generators in prod.

Nit: for both points above worth asking in the slack channels?

jerrymarino commented 1 year ago

@luispadron first off, thanks for sending this PR. I'm also still getting my feet wet with bzlmod so far so had 1-2 questions 🏃‍♂️

Does bzlmod help us deal with optional transitive dependencies? If you ask me, it'd be nice if Bazel didn't force you to add all of the extra rules_ios_** calls in the workspace or bzlmod variant of it:

# Ideally don't need this
rules_ios_deps_rbe_feature_1
rules_ios_deps_rbe_feature_2

If some tools want to do a bazel query ... and unknowing pull 100% of deps, I don't know if it's worth to continue to optimize / gut features for these tools with bzlmod.

I like the idea to simplify this but hopefully bzlmod can help us keep features used by a smaller number of folks. The carthage feature is a a good example of this IMO, not sure who is using it. Maybe bzlmod can help get stats on that..

luispadron commented 1 year ago

@acecilia Going off the framework_builder blame, what are your thoughts on dropping support for these rules? Do ya'll still use these, is there a need for them?

acecilia commented 1 year ago

I believe these rules are not used by anybody 🤞 Added them some time ago thinking they would be useful, but other better alternatives were created not short after. I would say you can safely remove them

luispadron commented 1 year ago

@jerrymarino

If you ask me, it'd be nice if Bazel didn't force you to add all of the extra rulesios** calls in the workspace or bzlmod variant of it:

For a direct consumer of rules_ios, they should just need to add this to their MODULE.bazel

bazel_dep(name = "rules_ios", version = "x.x.x", repo_name = "build_bazel_rules_ios")

If some tools want to do a bazel query ... and unknowing pull 100% of deps, I don't know if it's worth to continue to optimize / gut features for these tools with bzlmod.

bzlmod should evaluate when needed so it should only pull in the repos needed for the given targets. I believe this should make it faster than the existing WORKSPACE file, but this also depends on how how our non-bzl mod packages behave as MODULE.bazel macros are always called. Bazel 7.0 should fix this with a bzlmod lockfile.

brentleyjones commented 1 year ago

Bazel 7.0 should fix this with a bzlmod lockfile.

The lockfile is available (opt-in) fom Bazel 6.3.2 onward.

Just pinging here to see where adding bzlmod support is on the roadmap? Ideally the ruleset would have support before Bazel 7 flips the default on everyone. Also makes it easier for people starting to jump to rolling releases.

luispadron commented 1 year ago

@brentleyjones still blocked by the legacy Xcode generator issues requiring repository name during analysis (which is now unavailable in bzlmod).

I plan on spending more time this month hopefully wrapping it up

luispadron commented 1 year ago

Providing an update here, this is really close. Current status is that we have these in-flight PRs needed to merge:

Once those are merged I'll add the required files and setup the bcr release process. rules_ios version 3.0 will be the first one in the registry

luispadron commented 11 months ago

This is now completed