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

Migrate existing modules to the new toolchain registration API #127

Closed
Wyverald opened this issue Jul 8, 2022 · 6 comments
Closed

Migrate existing modules to the new toolchain registration API #127

Wyverald opened this issue Jul 8, 2022 · 6 comments
Labels
bounty-1000USD Bug Bounty: A contributor who completes this can be paid by the Rules Authors SIG

Comments

@Wyverald
Copy link
Member

Wyverald commented Jul 8, 2022

See bazelbuild/bazel#15829

This:

module(
  name = "foo",
  version = "2.0",
  toolchains_to_register = ["@toolchain_repo//:toolchain"],
)

ext = use_extension("//:extensions.bzl", "ext")
ext.some_tag()
use_repo(ext, "toolchain_repo")

should become this:

module(
  name = "foo",
  version = "2.0",
)

ext = use_extension("//:extensions.bzl", "ext")
ext.some_tag()
use_repo(ext, "toolchain_repo")
register_toolchains("@toolchain_repo//:toolchain")
@alexeagle
Copy link
Contributor

Is there some min,max version of Bazel where both ways are supported, to allow the transition?

@meteorcloudy
Copy link
Member

The support for the old API isn't deleted at HEAD yet (the BCR presubmit uses Bazel@HEAD), but we'll probably delete them before 6.0 cut. Since Bzlmod is still under an experimental flag, I think that should be enough?

@Wyverald Do you think we need to cherry-pick recent Bzlmod changes into 5.3?

@Wyverald
Copy link
Member Author

Some are cherry-pickable (like this one) but the flag parsing ones are probably harder. Hopefully people have been testing Bzlmod-related stuff with rolling releases (maybe we should've been clearer about that)

@alexeagle alexeagle added the bounty-1000USD Bug Bounty: A contributor who completes this can be paid by the Rules Authors SIG label Jul 12, 2022
@mattyclarkson
Copy link
Contributor

Hopefully people have been testing Bzlmod-related stuff with rolling releases (maybe we should've been clearer about that)

We've been testing across Bazel stable releases after 5.0.0.

This just broke our CI when testing against anything other than 5.3.0.

It's not a problem, we'll just drop support for 5.2.0 and below but certainly confused us for 15 minutes why the API didn't suddenly exist.

Looking forward to bzlmod becoming non-experimental: we really like it :)

@Wyverald
Copy link
Member Author

To be clear, we haven't removed the old API from HEAD yet. @meteorcloudy's change merely migrated existing modules to use the new API, which is what broke you since the new API wasn't available before. (But now that that's done, I can remove the old API)

Sorry about the inconvenience!

@meteorcloudy
Copy link
Member

@mattyclarkson Sorry for the confusing.
It's recommended to test Bzlmod with Bazel@HEAD, since we are making lots of improvements to it.
If you are using Bazelisk, just set USE_BAZEL_VERSION=last_green

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty-1000USD Bug Bounty: A contributor who completes this can be paid by the Rules Authors SIG
Projects
None yet
Development

No branches or pull requests

4 participants