Allow a no-cargo setup for bzlmod #2565

dzbarsky · 2024-03-19T21:24:34Z

Usage looks like so:

crate = use_extension("@rules_rust//crate_universe:extension.bzl", "crate")
crate.spec(package = "anyhow", version = "1.0.77")
....
crate.from_specs(name = "crates")

It might make sense to merge the annotation attributes into the spec so we don't have to duplicate things, but we can probably iterate on this in the future, this API is still experimental, yeah?

dzbarsky · 2024-03-19T21:32:42Z

@matts1 You're probably best-equipped to review this if you have the cycles

matts1

A few nitpicks, but generally looks good to me

matts1 · 2024-03-19T23:35:25Z

examples/bzlmod/hello_world_no_cargo/hello_world_test.sh

Nit: I've recently been made aware of a much better way to do this.

Can you add bazel_dep(name = "aspect_bazel_lib", version = "1.36.0") to MODULE.bazel, then add the following to the build file:

load("@aspect_bazel_lib//lib:run_binary.bzl", "run_binary) load("@bazel_skylib//rules:diff_test.bzl", "diff_test") run_binary( name = "run_hello_world", tool = ":hello_world", ) diff_test( name = "hello_world_diff_test", file1 = ":run_hello_world", file2 = "testdata/hello_world.out", )

This would need more changes to the binary to direct the output to a file, or another wrapper. But really I don't even need to run it, I'll just switch this to a build_test. As long as it compiles we're ok :)

That's a good point, sounds good to me. Don't know if we even need a build_test. bazel test :all actually builds all non-test targets as well IIRC.

that's true, I'll just leave the rust_binary

matts1 · 2024-03-19T23:36:59Z

examples/bzlmod/hello_world_no_cargo/src/main.rs

Nit: Can we make most of the files in the directory a symlink to examples/bzlmod/hello_world/*? That should help us keep them in sync - the only file that should actually be different is MODULE.bazel, but the rest should be the same.

I added --noenable_workspace to the .bazelrc, so now we only have BUILD.bazel, MODULE.bazel, .bazelrc and src/main.rs. The last one is the only one that is the same. I tried symlinking it but Bazel rust_binary seems to not pick up the symlinked source file. Given the source is pretty trivial, it didn't seem worth poking at it more, but let me know if you feel strongly!

That's very curious, but yeah, don't think it's worth digging into.

matts1 · 2024-03-19T23:42:20Z

crate_universe/extension.bzl

+            all_repos.append(cfg.name)
+            local_repos.append(cfg.name)
+
+        for cfg in mod.tags.from_specs:


Nit: Looks like a lot of duplicated code between this and the above section. Could we pull the common stuff into a function?

good call, I rearranged this a bit to share the repo validation. So the duplication now is the 3 lines for annotations. I'm hesitating a bit because sharing this either requires using a lambda or a function that takes module_annotations, repo_specific_annotations, and cfg.name. Alternately, it can look like this_repo_annotations = update_annotations(module_annotations, repo_specific_annotations.get(cfg.name, {})) or something.

Do you still think it's worth pulling out or keep it as-is?

I quite like the repo_specific_annotations idea. I think that'd probably be the cleanest.

I factored that out, PTAL

matts1 · 2024-03-20T22:34:25Z

crate_universe/extension.bzl

@@ -290,10 +325,39 @@ _annotation = tag_class(
    ),
 )

+_from_specs = tag_class(
+    doc = "Generates a repo @crates from a Cargo.toml / Cargo.lock pair",


Nit: This looks like it needs to be updated

yep good catch, ty!

dzbarsky · 2024-03-20T23:37:29Z

@UebelAndre @illicitonion can we merge this?

UebelAndre

Thanks! A couple of nits and can you add a build in CI that runs on this workspace? Or some kind of regression test somewhere else?

~~I'd also love to know if @matts1 approves too.~~ (They do!)

crate_universe/extension.bzl

examples/bzlmod/hello_world_no_cargo/.bazelrc

crate_universe/extension.bzl

dzbarsky · 2024-03-22T13:34:10Z

Thanks! A couple of nits and can you add a build in CI that runs on this workspace? Or some kind of regression test somewhere else?

Oh I had assumed we were already testing the examples in CI. Should we fix that instead?

UebelAndre · 2024-03-22T13:39:43Z

Oh I had assumed we were already testing the examples in CI. Should we fix that instead?

A new workspace requires a new job. Had this been an expansion of an existing case the it would have been picked up

dzbarsky · 2024-03-22T14:53:45Z

Oh I had assumed we were already testing the examples in CI. Should we fix that instead?

A new workspace requires a new job. Had this been an expansion of an existing case the it would have been picked up

Ah I see, the CI framework makes it hard to iterate over all the example workspaces, fair enough. Updated with all your comments, PTAL!

UebelAndre

Nice! I had just one more request to fix the .bazelignore file and then I think it's good to go!

examples/bzlmod/hello_world_no_cargo/MODULE.bazel

crate_universe/extension.bzl

UebelAndre

Thank you!

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [rules_rust](https://togithub.com/bazelbuild/rules_rust) | http_archive | minor | `0.40.0` -> `0.41.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_rust (rules_rust)</summary> ### [`v0.41.0`](https://togithub.com/bazelbuild/rules_rust/releases/tag/0.41.0) [Compare Source](https://togithub.com/bazelbuild/rules_rust/compare/0.40.0...0.41.0) ### 0.41.0 ```python load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "rules_rust", integrity = "sha256-Y4v6kjQQfXxh5tU6FQB6YXux/ODFGUq3IlpgBV4Bwj8=", urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.41.0/rules_rust-v0.41.0.tar.gz"], ) ``` Additional documentation can be found at: https://bazelbuild.github.io/rules_rust/#setup #### What's Changed - Add example of cross-compiling with musl by [@illicitonion](https://togithub.com/illicitonion) in [bazelbuild/rules_rust#2535 - Temporarily disable "Examples Clang with LLD" CI job by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2560 - Update cargo_toml to `0.19.2` by [@jun-sheaf](https://togithub.com/jun-sheaf) in [bazelbuild/rules_rust#2551 - Stop rustfmt versioning from overriding rust-analyzer versioning. by [@ReticentIris](https://togithub.com/ReticentIris) in [bazelbuild/rules_rust#2553 - Rust Analyzer added NixOS supported platforms by [@rickvanprim](https://togithub.com/rickvanprim) in [bazelbuild/rules_rust#2547 - Removed unused 'select_with_or' by [@dzbarsky](https://togithub.com/dzbarsky) in [bazelbuild/rules_rust#2562 - Added Rust 1.77.0 by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2568 - bzlmod: fix issue with nightly versions by [@QuentinPerez](https://togithub.com/QuentinPerez) in [bazelbuild/rules_rust#2545 - Allow a no-cargo setup for bzlmod by [@dzbarsky](https://togithub.com/dzbarsky) in [bazelbuild/rules_rust#2565 - Fixed genquery for rust targets by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2559 - Fix cargo-bazel recompile for MODULE.bazel by [@ericmcbride](https://togithub.com/ericmcbride) in [bazelbuild/rules_rust#2570 - Minor cleanup of bzl files by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2573 - Added starlark unit tests for `rust_toolchain.opt_level` by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2578 - Mark the bzlmod extension reproducible as appropriate by [@dzbarsky](https://togithub.com/dzbarsky) in [bazelbuild/rules_rust#2575 - Release 0.41.0 by [@UebelAndre](https://togithub.com/UebelAndre) in [bazelbuild/rules_rust#2569 #### New Contributors - [@jun-sheaf](https://togithub.com/jun-sheaf) made their first contribution in [bazelbuild/rules_rust#2551 - [@ReticentIris](https://togithub.com/ReticentIris) made their first contribution in [bazelbuild/rules_rust#2553 - [@QuentinPerez](https://togithub.com/QuentinPerez) made their first contribution in [bazelbuild/rules_rust#2545 - [@ericmcbride](https://togithub.com/ericmcbride) made their first contribution in [bazelbuild/rules_rust#2570 **Full Changelog**: bazelbuild/rules_rust@0.40.0...0.41.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/bazel-contrib/toolchains_llvm).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

dzbarsky force-pushed the bzlmod-crate-spec branch from 1b08f0f to 7b023d3 Compare March 19, 2024 21:32

dzbarsky force-pushed the bzlmod-crate-spec branch from 7b023d3 to 26d33c1 Compare March 19, 2024 21:39

matts1 suggested changes Mar 19, 2024

View reviewed changes

dzbarsky force-pushed the bzlmod-crate-spec branch 2 times, most recently from 90ce2ef to 95b563a Compare March 20, 2024 02:31

dzbarsky mentioned this pull request Mar 20, 2024

Bzlmod crate.from_cargo() annotations and packages #2391

Open

dzbarsky force-pushed the bzlmod-crate-spec branch from 95b563a to eca818d Compare March 20, 2024 14:54

matts1 approved these changes Mar 20, 2024

View reviewed changes

dzbarsky force-pushed the bzlmod-crate-spec branch from eca818d to 68fa808 Compare March 20, 2024 23:37

dzbarsky mentioned this pull request Mar 21, 2024

Release 0.41.0 #2569

Merged

UebelAndre requested changes Mar 22, 2024

View reviewed changes

crate_universe/extension.bzl Outdated Show resolved Hide resolved

crate_universe/extension.bzl Show resolved Hide resolved

examples/bzlmod/hello_world_no_cargo/.bazelrc Outdated Show resolved Hide resolved

crate_universe/extension.bzl Show resolved Hide resolved

dzbarsky force-pushed the bzlmod-crate-spec branch from 68fa808 to 9359e07 Compare March 22, 2024 14:53

dzbarsky requested a review from UebelAndre March 22, 2024 14:53

UebelAndre requested changes Mar 22, 2024

View reviewed changes

examples/bzlmod/hello_world_no_cargo/MODULE.bazel Outdated Show resolved Hide resolved

crate_universe/extension.bzl Show resolved Hide resolved

Bzlmod spec stuff

98d495a

dzbarsky force-pushed the bzlmod-crate-spec branch from 9359e07 to 98d495a Compare March 22, 2024 18:21

dzbarsky requested a review from UebelAndre March 22, 2024 18:22

UebelAndre approved these changes Mar 26, 2024

View reviewed changes

UebelAndre added this pull request to the merge queue Mar 26, 2024

UebelAndre removed this pull request from the merge queue due to a manual request Mar 26, 2024

UebelAndre added this pull request to the merge queue Mar 26, 2024

Merged via the queue into bazelbuild:main with commit 00a4bfb Mar 26, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow a no-cargo setup for bzlmod #2565

Allow a no-cargo setup for bzlmod #2565

dzbarsky commented Mar 19, 2024

dzbarsky commented Mar 19, 2024

matts1 left a comment

matts1 Mar 19, 2024

dzbarsky Mar 20, 2024

matts1 Mar 20, 2024

dzbarsky Mar 20, 2024

matts1 Mar 19, 2024

dzbarsky Mar 20, 2024

matts1 Mar 20, 2024

matts1 Mar 19, 2024

dzbarsky Mar 20, 2024 •

edited

matts1 Mar 20, 2024

dzbarsky Mar 20, 2024

matts1 Mar 20, 2024

dzbarsky Mar 20, 2024

dzbarsky commented Mar 20, 2024

UebelAndre left a comment •

edited

dzbarsky commented Mar 22, 2024

UebelAndre commented Mar 22, 2024

dzbarsky commented Mar 22, 2024

UebelAndre left a comment

UebelAndre left a comment

Allow a no-cargo setup for bzlmod #2565

Allow a no-cargo setup for bzlmod #2565

Conversation

dzbarsky commented Mar 19, 2024

dzbarsky commented Mar 19, 2024

matts1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dzbarsky Mar 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dzbarsky commented Mar 20, 2024

UebelAndre left a comment • edited

Choose a reason for hiding this comment

dzbarsky commented Mar 22, 2024

UebelAndre commented Mar 22, 2024

dzbarsky commented Mar 22, 2024

UebelAndre left a comment

Choose a reason for hiding this comment

UebelAndre left a comment

Choose a reason for hiding this comment

dzbarsky Mar 20, 2024 •

edited

UebelAndre left a comment •

edited