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

Explicitly tracked lists of generated dependencies in MODULE.bazel #2368

Closed
UebelAndre opened this issue Dec 28, 2023 · 6 comments · Fixed by #3177
Closed

Explicitly tracked lists of generated dependencies in MODULE.bazel #2368

UebelAndre opened this issue Dec 28, 2023 · 6 comments · Fixed by #3177

Comments

@UebelAndre
Copy link
Collaborator

UebelAndre commented Dec 28, 2023

I just came across https://github.com/bazelbuild/rules_rust/blob/0.35.0/MODULE.bazel#L35-L124 and think this is a pretty egregious regression in maintainability for me. Any dependency here that's generated by a crate_universe rule needs to be updatable by the same rule or some other .bzl file rendered to contain the appropriate list.

@UebelAndre UebelAndre added the bug label Dec 28, 2023
@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Dec 28, 2023

cc @illicitonion @matts1 is this something that will be addressed soon?

@illicitonion
Copy link
Collaborator

From #2314:

Note that the MODULE.bazel changes in use_repo are autogenerated, and if you change the third party crates you depend on, you should get an error message saying something like "run this command to fix it" on your bzlmod presubmit environment.

I didn't test this out - did you run into this? What happened?

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Dec 28, 2023

I didn't test this out - did you run into this? What happened?

(13:20:44) WARNING: /workdir/MODULE.bazel:35:30: The module extension internal_deps defined in @rules_rust//rust/private:extensions.bzl reported incorrect imports of repositories via use_repo():
 
Imported, but not created by the extension (will cause the build to fail):
    rules_rust_bindgen__bindgen-0.65.1, rules_rust_bindgen__bindgen-cli-0.65.1
 
Not imported, but reported as direct dependencies by the extension (may cause the build to fail):
    rules_rust_bindgen__bindgen-0.69.1, rules_rust_bindgen__bindgen-cli-0.69.1
 
 ** You can use the following buildozer command to fix these issues:
 
buildozer 'use_repo_add @rules_rust//rust/private:extensions.bzl internal_deps rules_rust_bindgen__bindgen-0.69.1 rules_rust_bindgen__bindgen-cli-0.69.1' 'use_repo_remove @rules_rust//rust/private:extensions.bzl internal_deps rules_rust_bindgen__bindgen-0.65.1 rules_rust_bindgen__bindgen-cli-0.65.1' //MODULE.bazel:all

I don't have buildozer installed. I also don't think relying on buildozer is a good solution. I intended to update dependencies by running a crates_vendor target and that should have been all I needed to do. Can this list be rendered into another bzl file and loaded in MODULE.bazel instead (I genuinely have no idea what you can do with bzlmod)?

@illicitonion
Copy link
Collaborator

I'm pretty sure MODULE.bazel isn't allowed to load :( It's one of the pain-points the bzlmod folks are trying to work out how to improve without opening up too much power/complexity.

Personally I'm ok to rely on buildozer... I think we could probably automate fetching/running buildozer as part of running crates_vendor?

But I'll let @matts1 chime in (and also cc @meteorcloudy and @Wyverald)

@UebelAndre
Copy link
Collaborator Author

The lack of load feels like an annoying limitation but I'd be happy with buildozer. I care most that there's only one action needed to update dependencies using crate_universe. So bazel run //crate_universe/3rdparty:crates_vendor -- --repin should be all that's necessary if I wanted to update the dependencies of crate_universe itself.

@matts1
Copy link
Contributor

matts1 commented Dec 29, 2023

In order to solve that issue, the only way to do it is to use the hub-and-spoke model, like I use in #1910.

That also means you don't need to update the MODULE.bazel, so you don't need to use buildozer.

Essentially, we would generate @crate_universe_anyhow//:anyhow and @crate_universe_clap, then create a repository @crate_universe for which the BUILD.bazel contains

alias(name = "anyhow", actual = "@crate_universe_anyhow//:anyhow")
alias(name = "clap", actual = "@crate_universe_anyhow//:clap")

Then instead of referring to @crate_universe_anyhow//:anyhow, you'd have to refer to @crate_universe//:anyhow. This gets around the visibility issues because the main repo doesn't need to be able to see the @crate_universe_anyhow repo.

Pretty sure that once someone implements proper vendored crates for bzlmod, we should get this for free.

github-merge-queue bot pushed a commit that referenced this issue Jan 8, 2025
…#3169)

This would lead to regular failures as new versions could be picked up
that would no longer match what's hard-coded in the MODULE.bazel.

Relates to #2368
github-merge-queue bot pushed a commit that referenced this issue Jan 9, 2025
`bazel mod tidy` will now run at the end of any `crates_vendor`
executions if the current workspace contains a `MODULE.bazel` file and
the current Bazel version is greater than `7.0.0`.

closes #2368
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants