Migrate Rust crate dependencies to rules_rs

[dzbarsky]

I wasn't sure of the intention behind the nonhermetic autoconfig cc toolchains, but it would be easy enough to remove those and use the hermetic-llvm (@llvm) toolchain for everything, since it's already pulled in.

All vendored crate build files can now be removed, they are unused. I figured we could do that in a followup commit but happy to do it here if folks prefer.

Fixes #24797

---

This change is 

[jwnimmer-tri]

FYI We use the "Reviewable" website for our code review discussions. (Click the icon in the PR overview to open the review page.) For new discussions I'll open them in that tool, but for your existing discussion I'll reply in the GitHub threads.

All vendored crate build files can now be removed, they are unused. I figured we could do that in a followup commit but happy to do it here if folks prefer.

Yes, to the extent any files are unused they should be deleted in the same pull request that obsoletes them.

I wasn't sure of the intention behind the nonhermetic autoconfig cc toolchains, but it would be easy enough to remove those and use the hermetic-llvm toolchain for everything, since it's already pulled in.

Drake is primarily a library (not application) used by robotics. In that world, Bazel and hermetic builds are not yet pervasive. Drake inter-operates with build systems and precompiled libraries outside of the Bazel ecosystem. For example, see https://drake.mit.edu/from_source.html for our CMake wrapper that transmogrifies CMake options into Bazel options and installs a compiled form of Drake's libraries and headers into the user's CMAKE_INSTALL_PREFIX using Bazel as the back-end.

For robotics ecosystem cases like those, the autodetecting toolchain must remain our default. Does rules_rs allow using (only) the autodetected toolchain? Assuming yes, why are any C++ toolchains being modified here / new LLVM module dependency / etc? The changes here that go outside of the Rust-specific parts of our code don't make sense to me yet, and are the main sticking point right now.

@jwnimmer-tri reviewed 4 files, made 6 comments, and resolved 2 discussions.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on dzbarsky).

---

MODULE.bazel line 31 at r1 (raw file):

# Configure the Rust toolchain.

rules_rust = use_extension("@rules_rs//rs:rules_rust.bzl", "rules_rust")

I see that this module extension is a forked copy of rules_rust.

There are two problems here:

(1) Mirroring. All externals must have at least two distinct websites serving them.

By default, we use the BCR cloudflare mirror of BCR modules so all BCR things in the MODULE are fine by default. (Served by both github and cloudflare.)

For module exensions we write ourselves, we provide our own S3-backed mirror of them. For example, the https://github.com/oxfordcontrol/Clarabel.cpp/archive/refs/tags/v0.11.1.tar.gz external is mirrored at https://drake-mirror.csail.mit.edu/github/oxfordcontrol/Clarabel.cpp/v0.11.1.tar.gz.

However, the rules_rust source archive that implements this extension uses an unstable github URL and has no mirrors. If github goes down, our build fails. If github changes their git-sha export-on-the-fly compression version, our build fails.

(2) Unstable / unclear version of rules_rust.

From a quick glance at the forked repo, I don't understand the merge policy versus bazelbuild/rules_rust and I don't understand what version of rules_rust it's based against. Is the main branch force-pushed when you want to rebase onto a newer rules_rust?

I would feel more comfortable if the lifecycle here were at least more clearly explained. Or even more ideally, if the pin was against an official numbered release of rules_rust plus a set of patches. (You could still use the git fork to maintain your changelist, but export it into patch files when used by rules_rs.) Possibly that's annoying, though. I could live with a more clear pinning / upgrade strategy and documentation.

(Using the upstream rules_rust tgz + patches would solve the mirroring problem, since rules_rs could specify both github and cloudflare as rules_rust.tgz urls.)

---

tools/workspace/crate_universe/upgrade.sh line 1 at r1 (raw file):

#!/bin/bash

nit At this point, it probably makes more sense to write this whole program in Python (with /usr/bin/env python3 as the shebang), and just use subprocess.check_call to run cargo? It's not required to be in bash.

[dzbarsky]

Ah, thanks for explaining. You have an interesting setup :)

In terms of what is happening in this PR - I replaced the toolchains_llvm module with llvm because it provides a much smaller download - it's only 30MB for the entire suite of LLVM tools. These tools are only being used for clang-format etc, we do not require the actual cc toolchain registered for rules_rs to work. (I had registered it as a dev_dep toolchain because I don't have a local cc toolchain, but removed it now). Just realized toolchains_llvm was itself marked dev_dependency = True so I'll do that for @llvm as well (I guess we don't care about @drake//tools working for downstream module consumers?)

Does that address your concerns?

@dzbarsky made 4 comments and resolved 1 discussion.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on jwnimmer-tri).

---

MODULE.bazel line 31 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I see that this module extension is a forked copy of rules_rust.

There are two problems here:

(1) Mirroring. All externals must have at least two distinct websites serving them.

By default, we use the BCR cloudflare mirror of BCR modules so all BCR things in the MODULE are fine by default. (Served by both github and cloudflare.)

For module exensions we write ourselves, we provide our own S3-backed mirror of them. For example, the https://github.com/oxfordcontrol/Clarabel.cpp/archive/refs/tags/v0.11.1.tar.gz external is mirrored at https://drake-mirror.csail.mit.edu/github/oxfordcontrol/Clarabel.cpp/v0.11.1.tar.gz.

However, the rules_rust source archive that implements this extension uses an unstable github URL and has no mirrors. If github goes down, our build fails. If github changes their git-sha export-on-the-fly compression version, our build fails.

(2) Unstable / unclear version of rules_rust.

From a quick glance at the forked repo, I don't understand the merge policy versus bazelbuild/rules_rust and I don't understand what version of rules_rust it's based against. Is the main branch force-pushed when you want to rebase onto a newer rules_rust?

I would feel more comfortable if the lifecycle here were at least more clearly explained. Or even more ideally, if the pin was against an official numbered release of rules_rust plus a set of patches. (You could still use the git fork to maintain your changelist, but export it into patch files when used by rules_rs.) Possibly that's annoying, though. I could live with a more clear pinning / upgrade strategy and documentation.

(Using the upstream rules_rust tgz + patches would solve the mirroring problem, since rules_rs could specify both github and cloudflare as rules_rust.tgz urls.)

I think multi-sourcing downloads is good for build reliability, but note that many external downloads currently do not have multiple URLs. It is not enough to simply have a BCR mirror, BCR modules issue plenty of their own downloads. For example even the current upstream rules_rust fetches tinyjson directly from crates.io and that's a requirement for even running the rust toolchain. I had an agent poke around and here's a list of unmirrored externals it found:

IMO an organization that cares about making builds reliable to network outages should be using some combination of Bazel's downloader/remote asset API (so downloads also get cached in remote cache) and/or a pull-through cache like Artifactory. Rulesets providing multiple URLs is admirable but ultimately very very hard to enforce 100% and should be treated as best effort in my experience. WDYT?

Regarding the rules_rust source archives - yep, you're totally right, thanks for reminding me to clean this up. I switched the pin to point at explicit source releases now, this is much better!

For the merge policy - it's essentially a combination of fixes we carry that upstream doesn't want or is slow to review. We often "backport" fixes from Google's rules_rust (backport in quotes because typically we are releasing rules_rs with bumped rules_rust pin before Google releases, so we are typically "ahead"). Unfortunately using a list of patch files against the upstream release is suboptimal for 2 reasons:

• We are 85 commits ahead of Google. That would be a bit silly to encode in a patch list (though doable)
• More importantly, some of our commits remove huge amounts of unneeded code and data from the rules_rust releases. The google https://github.com/bazelbuild/rules_rust/releases/download/0.73.0/rules_rust-0.73.0.tar.gz is 64 MB compressed! Our source archive is is 2.86MB and should actually shrink to the kilobytes ranges once I remove a 4.6MB starlark file from the repo

I don't think it makes sense to force users to download 64MB and then unpack it and use up even more space, especially with multi-worktree/multi-output-base workflows becoming so common these days :/

That said, I'm happy to change the README on https://github.com/hermeticbuild/rules_rust to make it clearer what is going on, do you have any suggestion for phrasing there that you would have wanted to see?

---

tools/workspace/crate_universe/upgrade.sh line 1 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit At this point, it probably makes more sense to write this whole program in Python (with /usr/bin/env python3 as the shebang), and just use subprocess.check_call to run cargo? It's not required to be in bash.

haha yeah fair enough, i'll swap this

[jwnimmer-tri]

BTW thanks for working on all of this!

I pushed a few small fixes. Feel free to squash and force-push if you want to keep a single commit in your tree. In any case Drake will squash it during merge into master.

Does that address your concerns?

Yep! My main concern originally was with the new toolchain registration and its effects on bazelrc, but that's all reverted now.

However, I would like to submit/test the llvm change separately. It makes it easier for me to understand and validate, is more localized in case we need to revert or bisect, etc. I carved it out into #24832 to get that into other reviewers' hands and land sooner.

@jwnimmer-tri partially reviewed 95 files and all commit messages, made 5 comments, and resolved 2 discussions.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on dzbarsky and jwnimmer-tri).

---

MODULE.bazel line 31 at r1 (raw file):

Previously, dzbarsky (David Zbarsky) wrote…

I think multi-sourcing downloads is good for build reliability, but note that many external downloads currently do not have multiple URLs. It is not enough to simply have a BCR mirror, BCR modules issue plenty of their own downloads. For example even the current upstream rules_rust fetches tinyjson directly from crates.io and that's a requirement for even running the rust toolchain. I had an agent poke around and here's a list of unmirrored externals it found:

IMO an organization that cares about making builds reliable to network outages should be using some combination of Bazel's downloader/remote asset API (so downloads also get cached in remote cache) and/or a pull-through cache like Artifactory. Rulesets providing multiple URLs is admirable but ultimately very very hard to enforce 100% and should be treated as best effort in my experience. WDYT?

Regarding the rules_rust source archives - yep, you're totally right, thanks for reminding me to clean this up. I switched the pin to point at explicit source releases now, this is much better!

For the merge policy - it's essentially a combination of fixes we carry that upstream doesn't want or is slow to review. We often "backport" fixes from Google's rules_rust (backport in quotes because typically we are releasing rules_rs with bumped rules_rust pin before Google releases, so we are typically "ahead"). Unfortunately using a list of patch files against the upstream release is suboptimal for 2 reasons:

• We are 85 commits ahead of Google. That would be a bit silly to encode in a patch list (though doable)
• More importantly, some of our commits remove huge amounts of unneeded code and data from the rules_rust releases. The google https://github.com/bazelbuild/rules_rust/releases/download/0.73.0/rules_rust-0.73.0.tar.gz is 64 MB compressed! Our source archive is is 2.86MB and should actually shrink to the kilobytes ranges once I remove a 4.6MB starlark file from the repo

I don't think it makes sense to force users to download 64MB and then unpack it and use up even more space, especially with multi-worktree/multi-output-base workflows becoming so common these days :/

That said, I'm happy to change the README on https://github.com/hermeticbuild/rules_rust to make it clearer what is going on, do you have any suggestion for phrasing there that you would have wanted to see?

The switch to release attachments is a key improvement, thanks. Not relying on the unstable github export compression makes me much more comfortable.

I'll circle back tomorrow on the other points.

---

tools/workspace/crate_universe/README.md line 13 at r3 (raw file):

Keep lock/Cargo.toml synchronized with Clarabel.cpp's rust_wrapper/Cargo.toml.

For things like this, we always try to have mechanical enforcement with some kind of linter.

Is it possible to have the upgrade.py automatically refer to Clarabel.cpp's toml file, without keeping a separate copy of it here? There should be a runfiles path to it available. (Edit: Ah, the MODULE.bazel need this copy. That's fine then.)

If not possible to remove it, then it's fine to have a vendored copy committed to git like we have here now, but we need some kind of diff_test or similar that will (1) fail bazel test //... --config=lint when the vendored copy is out of date and (2) upon failure, print a message/command saying how to mechanically copy/resync the file.

---

tools/workspace/crate_universe/defs.bzl line 17 at r3 (raw file):

    license_file = None
    preferred_basenames = ["LICENSE-APACHE", "LICENSE", "LICENSE.md", "LICENSE-MIT"]
    accepted_basenames = sorted([

I appreciate your (or your coding assistant's) thoroughness here to try to find all license files, but it seems a bit overkill -- more code mass then we want to maintain long-term.

Looking back at rules_rust, it seems that all it was doing for its rules_license declarations was a scan for file_name.to_uppercase().starts_with("LICENSE"). Could we do something here more along those lines, so as not to couple it with the LICENSE_LITERALS or LICENSE_PREFIXES lists? In the rust ecosystem it seems a strong convention to name things that way.

---

tools/install/installer.py line 81 at r3 (raw file):

@functools.cache

nit Before this PR lands, the changes to this file (and related BUILD) will need to be reverted.

I imagine it's necessary on your machine if you don't have a local Xcode install, but this file is specifically intended for CMake to run against the user's local Xcode, so we can't actually change it to use the hermetic one.

I'm OK just ignoring it for now in case carrying this change during review makes your development easier; we just need to revert at the end.

[tyler-yankee]

@tyler-yankee partially reviewed 18 files and all commit messages, and made 1 comment.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on dzbarsky and jwnimmer-tri).

---

MODULE.bazel line 39 at r4 (raw file):

rust_toolchains.toolchain(
    edition = "2021",
    version = "1.96.0",

BTW with this change we may need a better handle on automated upgrades of module extensions, but since Drake doesn't use Rust directly this one isn't too too big a deal.

[jwnimmer-tri]

FYI the LLVM change has landed on master now, so you can rebase this anytime (squashing or at least retaining on top the commits I pushed).

@jwnimmer-tri made 2 comments and resolved 1 discussion.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on dzbarsky).

---

MODULE.bazel line 31 at r1 (raw file):
My only remaining sticking point is clarity around the git relationship between bazelbuild/rules_rust main and hermeticbuild/rules_rust main.

The hermeticbuild main seems like a linear history, starting at some branching point from bazelbuild main around the 0.71 era. What is the plan for how to incorporate ongoing changes from the bazelbuild/rules_rust into hermeticbuild/rules_rust? Will bazelbuild main be merged into the hermeticbuild main, or will the hermeticbuild main be rebased and force-pushed?

Relatedly, I think it would help with clarity if the base revision of bazelbuild rules_rust used by hermeticbuild were more clearly communicated somewhere. As in "the hermeticbuild fork is based off of bazelbuild rules_rust as of v0.71.3" or whatever. And then when hermeticbuild upgrades to a newer base, the documentation changes to reflect that. Obviously there are a lot of fixes and even cherry-picks happening above the base revision, but having at least a rough sense of "how fresh (or stale)" the fork is would be help build confidence to switch to hermeticbuild.

Final small suggestions: it https://github.com/hermeticbuild/rules_rust, perhaps replace the README with one better aimed that at details of the fork. (I realize this might be annoying to maintain if upstream rules_rust changes their README frequently.)

IMO an organization that cares about making builds reliable to network outages should be using some combination

Yeah, I know and I don't disagree. Inside the company we have more tools, but for this open-source Drake portion I try to do my best to provide reliable builds from source for our individual one-off users who don't have access to such things.

In any case, now that it's a stabilized release attachment I can live with it.

(For some of our other dependencies, upstreams have force-pushed to tags (OMG) and overwritten their unstable-url artifacts from time to time over the years, so I have a built-in hestitation to using direct github export urls for anything.)

[jwnimmer-tri]

I am about to sign off for the day, but the new changes look good. I'll come back tomorrow and finish up looking.

@jwnimmer-tri reviewed 12 files and all commit messages, and made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on dzbarsky).

[dzbarsky]

thanks, done

@dzbarsky made 4 comments.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on jwnimmer-tri and tyler-yankee).

---

MODULE.bazel line 31 at r1 (raw file):
I rebase on top of upstream main ~weekly (depending how fast bazelbuild/rulesrust lands commits) and force-push, that makes it easier IMO to see the patches we are carrying on top. So basically the answer to "how fresh/stale" is the fork - it's typically within 20 commits of HEAD or so :) But I'm happy to include the "min released watermark" in the README if that makes things clearer!

upstreams have force-pushed to tags
💀

---

tools/workspace/crate_universe/defs.bzl line 17 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I appreciate your (or your coding assistant's) thoroughness here to try to find all license files, but it seems a bit overkill -- more code mass then we want to maintain long-term.

Looking back at rules_rust, it seems that all it was doing for its rules_license declarations was a scan for file_name.to_uppercase().starts_with("LICENSE"). Could we do something here more along those lines, so as not to couple it with the LICENSE_LITERALS or LICENSE_PREFIXES lists? In the rust ecosystem it seems a strong convention to name things that way.

Done.

---

tools/install/installer.py line 81 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit Before this PR lands, the changes to this file (and related BUILD) will need to be reverted.

I imagine it's necessary on your machine if you don't have a local Xcode install, but this file is specifically intended for CMake to run against the user's local Xcode, so we can't actually change it to use the hermetic one.

I'm OK just ignoring it for now in case carrying this change during review makes your development easier; we just need to revert at the end.

ok lets leave this thread open as a reminder to ourselves

[dzbarsky]

@dzbarsky made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on jwnimmer-tri and tyler-yankee).

---

tools/workspace/crate_universe/README.md line 13 at r3 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Keep lock/Cargo.toml synchronized with Clarabel.cpp's rust_wrapper/Cargo.toml.

For things like this, we always try to have mechanical enforcement with some kind of linter.

Is it possible to have the upgrade.py automatically refer to Clarabel.cpp's toml file, without keeping a separate copy of it here? There should be a runfiles path to it available. (Edit: Ah, the MODULE.bazel need this copy. That's fine then.)

If not possible to remove it, then it's fine to have a vendored copy committed to git like we have here now, but we need some kind of diff_test or similar that will (1) fail bazel test //... --config=lint when the vendored copy is out of date and (2) upon failure, print a message/command saying how to mechanically copy/resync the file.

i rearranged the repo rules a bit to avoid the duplication

[dzbarsky]

@dzbarsky resolved 1 discussion.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on jwnimmer-tri and tyler-yankee).

[jwnimmer-tri]

feature. +a:@rpoyner-tri for platform review, please.

@jwnimmer-tri reviewed 3 files and all commit messages, made 3 comments, and resolved 3 discussions.
Reviewable status: 1 unresolved discussion, LGTM missing from assignees tyler-yankee,rpoyner-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on dzbarsky and rpoyner-tri).

---

MODULE.bazel line 31 at r1 (raw file):

Previously, dzbarsky (David Zbarsky) wrote…

I rebase on top of upstream main ~weekly (depending how fast bazelbuild/rulesrust lands commits) and force-push, that makes it easier IMO to see the patches we are carrying on top. So basically the answer to "how fresh/stale" is the fork - it's typically within 20 commits of HEAD or so :) But I'm happy to include the "min released watermark" in the README if that makes things clearer!

upstreams have force-pushed to tags
💀

Great. Nothing in this discussions is blocking the pull request, but I do think some README changes that communicate the lifecycle (for both co-maintainers and users) would help out.

---

a discussion (no related file):
+a:@tyler-yankee please review from the point of view of an upgrade.py user. I played with it locally and it seemed good, but would like a double-check.

[rpoyner-tri]

@rpoyner-tri partially reviewed 95 files and all commit messages, and made 2 comments.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee tyler-yankee, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on dzbarsky).

---

tools/workspace/crate_universe/upgrade.py line 25 at r6 (raw file):

        )

    from python import runfiles

nit is it worth writing down why we need to delay this import?

[jwnimmer-tri]

@jwnimmer-tri reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee tyler-yankee, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on dzbarsky).

---

tools/workspace/crate_universe/upgrade.py line 25 at r6 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit is it worth writing down why we need to delay this import?

Done

[rpoyner-tri]

@rpoyner-tri reviewed 1 file and all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee tyler-yankee, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on dzbarsky).

[tyler-yankee]

@tyler-yankee reviewed 7 files and all commit messages, and made 2 comments.
Reviewable status: 1 unresolved discussion, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on jwnimmer-tri).

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

+a:@tyler-yankee please review from the point of view of an upgrade.py user. I played with it locally and it seemed good, but would like a double-check.

I checked out a sufficiently old version of tools/workspace/crate_universe/lock/Cargo.toml.lock (v1.45.0), renamed it to Cargo.lock to match what this PR is looking for, and ran bazel run //tools/workspace:new_release -- crate_universe. The upgrade restored state from the downgrades to HEAD, and printed the appropriate message with file(s) modified / commit.

[jwnimmer-tri]

@jwnimmer-tri resolved 1 discussion.
Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on dzbarsky).

[dzbarsky]

Great. Nothing in this discussions is blocking the pull request, but I do think some README changes that communicate the lifecycle (for both co-maintainers and users) would help out.

OK, I will follow up with README changes when I get home (currently traveling). Thanks for helping drive this in, super appreciate it!