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

Add support for bzlmod #703

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Add support for bzlmod #703

merged 1 commit into from
Oct 12, 2023

Conversation

luispadron
Copy link
Collaborator

@luispadron luispadron commented May 3, 2023

Adds initial support for bzlmod to rules_ios.

Major changes:

  • Update versions in repositories.bzl to use minimum supported bzlmod version
  • Add MODULE.bazel and required files to enable bzlmod by default in Bazel 6+
  • Enable bzlmod by default in the repository

Required workarounds:

  • Disable bzlmod when running legacy .xcodeproj tests. This rule collects workspace name in an aspect which now resolves to simply _main in bzlmod. This doesnt seem to cause any actual issues with the generated project besides the repository being named _main. Proper fix is to have this use runfiles but I do not have enough context to make that change.

Things to follow-up on:

  • Add bzlmod support for stardoc
    • This requires some more changes to bzl_library usage and update to Stardoc version, will do this in a separate PR before publishing.
  • Publish to Bazel Central Registry (and introduce files to required to do so)
  • Finalize documentation and update README (after publication)
  • Consider better support for legacy xcode project generator, the tests will continue to run without bzlmod as the aspect requires workspace name collection udpates.

Depends on: #716, #719

@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 8 times, most recently from 68d11a5 to 0641bad Compare May 9, 2023 20:17
@luispadron luispadron changed the base branch from master to lpadron/remove-carthage-cocoapod-rules May 9, 2023 20:17
@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 2 times, most recently from e526722 to 541c0d4 Compare May 10, 2023 00:56
@luispadron luispadron force-pushed the lpadron/remove-carthage-cocoapod-rules branch from b67df7e to dc5cae9 Compare May 10, 2023 00:59
@luispadron luispadron force-pushed the lpadron/remove-carthage-cocoapod-rules branch from dc5cae9 to da9a8b3 Compare May 10, 2023 04:15
@luispadron luispadron changed the base branch from lpadron/remove-carthage-cocoapod-rules to lpadron/add-ci-cache-configs May 10, 2023 04:16
@luispadron luispadron force-pushed the lpadron/add-ci-cache-configs branch from 47f9d6e to 31a3539 Compare May 10, 2023 04:21
@luispadron luispadron force-pushed the lpadron/add-ci-cache-configs branch from d4e6afb to ba1987c Compare May 10, 2023 15:27
@luispadron luispadron force-pushed the lpadron/add-ci-cache-configs branch from e5740af to dba6df6 Compare May 10, 2023 16:19
@luispadron luispadron force-pushed the lpadron/add-ci-cache-configs branch from dba6df6 to 452f68e Compare May 10, 2023 17:33
@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 2 times, most recently from 576d108 to c97dad7 Compare May 11, 2023 04:00
@luispadron luispadron force-pushed the lpadron/add-ci-cache-configs branch from 4ccb5b2 to 154e009 Compare May 11, 2023 04:19
@luispadron luispadron force-pushed the lpadron/add-ci-cache-configs branch from 154e009 to a8109a5 Compare May 11, 2023 04:39
@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 5 times, most recently from 7271b3d to dfe6561 Compare August 22, 2023 19:02
BUILD.bazel Show resolved Hide resolved
rules/module_extensions.bzl Show resolved Hide resolved
rules/module_extensions.bzl Show resolved Hide resolved
rules/repositories.bzl Show resolved Hide resolved
@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 3 times, most recently from 1138ba7 to 186c436 Compare August 22, 2023 19:27
@luispadron luispadron marked this pull request as ready for review August 22, 2023 19:28
@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 2 times, most recently from 75fe577 to ca4d59f Compare August 22, 2023 19:51
BUILD.bazel Show resolved Hide resolved
rules/repositories.bzl Show resolved Hide resolved
rules/repositories.bzl Show resolved Hide resolved
@luispadron
Copy link
Collaborator Author

Will be blocked by #773 if that merges.

Copy link
Contributor

@jerrymarino jerrymarino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luispadron I think it's time to land this and forward fix. For the rules_apple 3.x update - I will rebase the changes once we land it.

@luispadron luispadron force-pushed the lpadron/add-bzlmod-support branch 2 times, most recently from f4d13f5 to 3977cb8 Compare October 3, 2023 19:21
This commit adds support for bzlmod.
See the MODULE.bazel for changes and enable with `--enable_bzlmod`.
@luispadron luispadron merged commit bf647f1 into master Oct 12, 2023
8 checks passed
@luispadron luispadron deleted the lpadron/add-bzlmod-support branch October 12, 2023 17:43

def _non_module_deps_impl(_):
rules_ios_dependencies(
load_bzlmod_dependencies = False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI - This confused me, because I thought "load_bzlmod_dependencies" means "dependencies that bzlmod needs" rather than "dependencies that bzlmod has already supplied". Just putting this here in case anyone else stumbles across this, and as always, naming is hard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to update it the other day and I felt the same way lol

Happy to change this to something more clear

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.

5 participants