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

Support bzlmod #1528

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Support bzlmod #1528

merged 3 commits into from
Mar 29, 2023

Conversation

cameron-martin
Copy link
Contributor

@cameron-martin cameron-martin commented Aug 25, 2022

This makes rules_rust load dependencies via bzlmod. Currently only basic functionality is completed, such as registering rustc toolchains and compiling crates. Note that it cannot interact with cargo, wasm or load any other rust toolchains such as rustfmt.

There is one new module, examples/bzlmod/hello_world, that depends on the root rules_rust module. This example can be built and run using:

cd examples/bzlmod/hello_world
bazel run //:hello_world

To register toolchains in an ergonomic way, it defines a new "hub" repository that contains all the toolchain proxies, so they can all be registered like so:

register_toolchains("@rust_toolchains//:all")

closes #1493

@UebelAndre
Copy link
Collaborator

re #1493 (comment)

@scentini do you or anyone you have contact with at Google know anything about bzlmod?

@scentini
Copy link
Collaborator

I'm not familiar with bzlmod, in case we're doing something silly here it's not obvious to me. @Wyverald could you PTAL?

@scentini
Copy link
Collaborator

cc @meteorcloudy

@meteorcloudy
Copy link
Member

Bzlmod may have some bugs in Bazel 5.3.0, it's recommended to test with Bazel@HEAD, you can use Bazelisk:

export USE_BAZEL_VERSION=last_green
bazelisk build //:hello_world

I got the following toolchain error instead of the visibility error:

ERROR: /Users/pcloudy/workspace/rules_rust/examples/bzlmod/hello_world/BUILD.bazel:5:12: While resolving toolchains for target //:hello_world: No matching toolchains found for types @rules_rust~override//rust:toolchain_type.
To debug, rerun with --toolchain_resolution_debug='@rules_rust~override//rust:toolchain_type'
If platforms or toolchains are a new concept for you, we'd encourage reading https://bazel.build/concepts/platforms-intro.

rust/repositories.bzl Outdated Show resolved Hide resolved
@cameron-martin
Copy link
Contributor Author

I've updated the PR with a rough proof of concept that can compile and run a rust binary.

@meteorcloudy
Copy link
Member

Nice, can you also fix the buildifier errors?

@benbrittain
Copy link
Contributor

Is there any work I can take on here to help this get landed?

extensions.bzl Outdated Show resolved Hide resolved
@UebelAndre
Copy link
Collaborator

UebelAndre commented Jan 18, 2023

@cameron-martin hey, any updates? 😄

@cameron-martin
Copy link
Contributor Author

@cameron-martin hey, any updates? smile

Sorry I've been busy with other things. I may be able to get back to this in the next week or so.

@opicaud
Copy link

opicaud commented Feb 11, 2023

hello @cameron-martin, do you have any update ?
Thanks!

@matts1
Copy link
Contributor

matts1 commented Feb 16, 2023

I've got a prototype working for a module extension for bzlmod (changes stacked on top of this PR). It's still very much a WIP, and needs a lot of cleaning up, but you can at least build examples/bzlmod/crates_repository.

https://github.com/matts1/rules_rust/tree/module_extension_prototype

TLDR on how to use:

crates_vendor = use_extension("@rules_rust//:extensions.bzl", "crates_vendor")
crates_vendor.crates_vendor(
    manifests = ["//:Cargo.toml"],
    cargo_lockfile = "//:Cargo.lock",
    lockfile = "//:cargo-bazel-lock.json",
)
use_repo(
    crates_vendor,
    crates = "crates_crates_repository",
)

@cameron-martin
Copy link
Contributor Author

cameron-martin commented Feb 18, 2023

I've rebased this PR. Here are the main things that need finishing:

  • Support all options when registering toolchains via bzlmod.
  • Work out how to run the examples on CI

@cameron-martin
Copy link
Contributor Author

We also have to consider the API for the module extensions. How many module extensions do we want? Just a single one, called "rust" for everything including crates_universe, etc? One per top-level package, e.g. rust, crates_universe, cargo?

@cameron-martin cameron-martin force-pushed the bzlmod branch 2 times, most recently from d8a1fff to d4645d8 Compare February 18, 2023 16:52
MODULE.bazel Show resolved Hide resolved
@matts1 matts1 mentioned this pull request Feb 20, 2023
MODULE.bazel Outdated Show resolved Hide resolved
@cameron-martin cameron-martin marked this pull request as ready for review February 20, 2023 23:24
@cameron-martin cameron-martin force-pushed the bzlmod branch 2 times, most recently from d358c9b to dc2cf33 Compare February 23, 2023 21:57
@cameron-martin
Copy link
Contributor Author

This should now be ready to review.

@UebelAndre
Copy link
Collaborator

cc @scentini @krasimirgg Hoping one of you two know anything about bzlmod and would be able to review this 😅

MODULE.bazel Outdated Show resolved Hide resolved
rust/private/extensions.bzl Outdated Show resolved Hide resolved
@scentini
Copy link
Collaborator

I read up a bit on bzlmod and unless someone with in-depth knowledge offers to do the review, I'll claim the 'expert' title. This PR LGTM, and aside from rebasing and updating the version in MODULE.bazel, I'd try merging.

Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

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

had a brief pass, mostly looks good! I didn't read the actual implementation very deeply.

MODULE.bazel Outdated Show resolved Hide resolved
rust/private/extensions.bzl Outdated Show resolved Hide resolved
This makes rules_rust load dependencies via bzlmod. Currently only basic functionality is completed, such as registering rustc toolchains and compiling crates. Note that it cannot interact with cargo, wasm or load any other rust toolchains such as rustfmt.

There is one new module, `examples/bzlmod/hello_world`, that depends on the root `rules_rust` module. This example can be built and run using:

```
cd examples/bzlmod/hello_world
bazel run //:hello_world
```
@scentini
Copy link
Collaborator

Thank you @Wyverald !

@scentini scentini self-requested a review March 29, 2023 08:54
@scentini scentini merged commit eaf5138 into bazelbuild:main Mar 29, 2023
@meteorcloudy
Copy link
Member

Nice! Will there be the next step to integrate with cargo to actually fetch rust dependencies?

path = "../../..",
)

rust = use_extension("@rules_rust//rust:extensions.bzl", "rust")
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for being late here. Do we want rules_rust to register a default toolchain so that users don't have to do toolchain registration in their MODULE.bazel file at all? (of course, they can still do it to use their desired toolchain version).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently only the toolchains declared in the top level module are registered. There was an unresolved discussion on whether this was the correct thing to do here, but currently the toolchain that rules_rust defines would not get registered.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks!

@cameron-martin
Copy link
Contributor Author

Nice! Will there be the next step to integrate with cargo to actually fetch rust dependencies?

@matts1 currently has a PR for this here: #1910

illicitonion added a commit to illicitonion/rules_rust that referenced this pull request Jan 4, 2024
It looks like we register these as _tools not as _srcs, this looks like
it was maybe a typo in bazelbuild#1528
illicitonion added a commit that referenced this pull request Jan 5, 2024
It looks like we register these as _tools not as _srcs, this looks like
it was maybe a typo in
#1528

Fixes #2388
@cameron-martin cameron-martin deleted the bzlmod branch February 20, 2024 19:47
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.

Add support for bzlmod
10 participants