[bazel] Move to rules_rs for crate management by dzbarsky · Pull Request #6125 · cloudflare/workerd

dzbarsky · 2026-02-20T20:48:54Z

No description provided.

github-actions · 2026-02-20T20:49:03Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

dzbarsky · 2026-02-20T20:49:13Z

I have read the CLA Document and I hereby sign the CLA

codecov-commenter · 2026-02-21T15:27:57Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.70%. Comparing base (cea70af) to head (3ad441a).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6125      +/-   ##
==========================================
- Coverage   70.80%   70.70%   -0.11%     
==========================================
  Files         424      424              
  Lines      115290   115450     +160     
  Branches    18751    18770      +19     
==========================================
- Hits        81629    81626       -3     
- Misses      22429    22590     +161     
- Partials    11232    11234       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fhanau

LGTM otherwise

fhanau · 2026-03-05T02:15:36Z

+
+crate = use_extension("@rules_rs//rs:extensions.bzl", "crate")
+crate.annotation(
+    additive_build_file_content = """exports_files(["include/lol_html.h"])""",


nit: It's easier to define a new target so that we don't need to manually import the lol_html_c_api repo – the branch I have just uses an annotation with extra_aliased_targets = {"lol_html_c_api_include_lol_html_h": "include/lol_html.h"} for that and updates the header target in deps/rust/BUILD.lolhtml. This will require a change in the downstream project, I can help with that later

Ended up doing this myself in #6302 – if that ends up landing first this PR will need some rebasing, but it makes lol-html much easier to maintain so it's something I wanted to do anyway.

Ok no problem! I've been focusing on rules_rs fixes for Patrick anyway and then the plan is workerdcxx so this will need a rebase either way:)

fhanau · 2026-03-05T02:17:19Z

+    name = "crates_vendor",
+    cargo_lock = "//deps/rust:Cargo.lock",
+    cargo_toml = "//deps/rust:Cargo.toml",
+    platform_triples = RUST_TARGET_TRIPLES,


Does rules_rs have the equivalent of strip_internal_dependencies_from_cargo_lockfile here? Not a big change but would make Cargo.lock slightly cleaner.

rules_rs doesn't have that, it was only added to rules_rust to try to make cargo splicing run less often, because it was a pain for other projects. But the folks who added it at Zipline are also using rules_rs now :)

but also how would strip_internal_dependencies_from_cargo_lockfile help? My understanding was it removed dependencies between first-party crates, when your Cargo.toml represents a workspace with multiple crates in it. That's not the setup here, so I think the lockfile is already refelcting only third-party deps. Am I missing something?

When I tried this with rules_rust's from_cargo, it ended up removing the direct-cargo-bazel-deps package definition from Cargo.lock – not a big change, but cleaner than before.

fhanau · 2026-03-05T02:19:39Z

    deps = [
        "//src/rust/cxx-integration",
-        "@crates_vendor//:encoding_rs",
+        "@crates_vendor//:encoding_rs-0.8.35",


This looks odd, and will break upon the next deps update – we should explicitly depend on encoding_rs in Cargo.toml instead, looks like that was missed when copying the dependencies over.

ack, will fix. I think the deps have also updated a bit since I generated the list, so I'll re-sync them and also carry over the comments/etc

fhanau · 2026-03-05T02:21:42Z


 # equivalent to `cargo update`; use `workspace` or <package> to limit update scope
 update-rust package="full":
+  # cd deps/rust && bazel run @rules_rust//tools/upstream_wrapper:cargo -- update


nit: Let's actually replace the old command instead of just adding a comment with the new one.

ah yes, this was a reminder for me to ask yall to double check this does what you expect, can you please check :)

fhanau · 2026-03-05T02:25:09Z

+path = "fake.rs"
+
+[dependencies]
+# Keep explicit features minimal to bound compile times and binary size.


nit: In Cargo.toml, let's copy existing comments instead of rephrasing them. If there turn out to be cases where the comment is outdated/misleading, we can change those.

fhanau · 2026-03-05T02:28:20Z

+lol_html_c_api = { git = "https://github.com/cloudflare/lol-html", tag = "v2.7.1" }
+nix = "0"
+pico-args = "0"
+proc-macro2 = { version = "1", features = ["span-locations"] }


This is newly added here – let's add a comment stating that this is for workerd-cxx since it is not used directly in workerd. Ideally, let's keep workerd-cxx dependencies separately at the end of the list so that we can more easily keep track of them.

Also, to facilitate usage of this list in the internal version and avoid duplicating this list: Is there a way to have Cargo.toml include another Cargo.toml file so that common dependencies can be shared between these? I guess we could set up a crate with the given Cargo.toml file and depend on that crate but that hardly sounds like the most elegant solution.

I can rearrange workerd-cxx bits to the end + add an explanation. I think the more proper fix is to make workerd's Cargo.toml depend on workerd-cxx's Cargo.toml as a crate, but I think that might require making similar Cargo.toml-ification change to workerd-cxx, so we can either fix that repo first and then circle back to this one, or do this hack as a stopgap, which would you prefer

Yeah, transitioning to rules_rs in workerd-cxx first might be the better approach

@fhanau OK, lets give that a shot. Do I need to be added as collaborator there as well so I can trigger CI?

I'd imagine that you already have the required permissions, otherwise I should be able to run the CI too

@fhanau looks like I don't have the permissions, mind unblocking cloudflare/workerd-cxx#96 and adding me as collaborator there so I can iterate over the weekend if needed?

Done, will start looking at that PR soon.

fhanau · 2026-03-05T02:36:22Z


 # rules_rust
-bazel_dep(name = "rules_rust", version = "0.68.1")
+bazel_dep(name = "rules_rs", version = "0.0.40")


not actionable, I'm just curious: What's the best way to see what rules_rust version a given rules_rs version maps to?

https://github.com/dzbarsky/rules_rs/blob/c5ae2dbfd8171d6c973a5bdc426f8bd1b295c0fe/rs/experimental/rules_rust.bzl#L32-L39 contains the pin, https://github.com/dzbarsky/rules_rust tracks upstream very closely (i'm usually rebasing every few days) but carries a lot of fixes on top as well. Not sure if that answers your question?

dzbarsky · 2026-03-06T14:42:35Z

Thanks for the review! I'll update over the weekend, cheers

fhanau · 2026-03-24T16:48:02Z

+
+[dependencies]
+# Import the correct versions of dependencies from workerd-cxx
+workerd-cxx = { package = "third-party", git = "https://github.com/cloudflare/workerd-cxx.git", rev = "c54ffa14aa9f0fc43e701f0ed01f1680be154be2" }


This will make workerd-cxx updates inconvenient as they will no longer work through the auto-update mechanism when there are crate dependency changes
@mikea can you think of a better approach here?

fhanau · 2026-03-24T16:52:02Z

I started porting this approach to the downstream repo. To make that easier, can you rebase this PR on main, use the latest commit on cloudflare/workerd-cxx#97 and update to rules_rs 0.44.0 here? The latter might require rebasing the patch, we'll need extra_aliased_targets support there too

fhanau · 2026-04-23T16:47:33Z

Landed as part of #6643, which gets rid of the crate files – we're using plain rules_rust for now, unfortunately I kept running into cases where rules_rs is incompatible which makes using it difficult (might revisit in the future, but for now we're mostly interested in not having the vendored crate files, which is easier to do with just rules_rust)

dzbarsky requested review from a team as code owners February 20, 2026 20:48

github-actions Bot added a commit that referenced this pull request Feb 20, 2026

@dzbarsky has signed the CLA in #6125

2e8dd7d

dzbarsky force-pushed the main branch 2 times, most recently from 26941d3 to dfd14a8 Compare February 21, 2026 15:18

dzbarsky force-pushed the main branch 2 times, most recently from 018135b to 355efd0 Compare February 21, 2026 16:39

jasnell requested review from fhanau and mikea February 21, 2026 16:46

dzbarsky force-pushed the main branch 7 times, most recently from 863b156 to 5857a4a Compare March 3, 2026 13:55

fhanau reviewed Mar 5, 2026

View reviewed changes

fhanau mentioned this pull request Mar 20, 2026

[build] Update deps, stop vendoring crates cloudflare/workerd-cxx#97

Merged

dzbarsky force-pushed the main branch 6 times, most recently from 6e24811 to 0dad305 Compare March 22, 2026 01:52

[bazel] Move to rules_rs for crate management

3ad441a

dzbarsky force-pushed the main branch from 0dad305 to 3ad441a Compare March 22, 2026 02:56

fhanau reviewed Mar 24, 2026

View reviewed changes

fhanau mentioned this pull request Apr 23, 2026

[build] Adopt build without vendored Rust files #6643

Merged

fhanau closed this Apr 23, 2026

Conversation

dzbarsky commented Feb 20, 2026

Uh oh!

github-actions Bot commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dzbarsky commented Feb 20, 2026

Uh oh!

codecov-commenter commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fhanau left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhanau Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzbarsky commented Mar 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhanau commented Mar 24, 2026

Uh oh!

fhanau commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Feb 20, 2026 •

edited

Loading

codecov-commenter commented Feb 21, 2026 •

edited

Loading

fhanau Mar 13, 2026 •

edited

Loading