Skip to content

[bazel] Swap to rules_rs for Rust build rules#17398

Merged
shs96c merged 3 commits into
SeleniumHQ:trunkfrom
dzbarsky:zbarsky/rules_rs
May 8, 2026
Merged

[bazel] Swap to rules_rs for Rust build rules#17398
shs96c merged 3 commits into
SeleniumHQ:trunkfrom
dzbarsky:zbarsky/rules_rs

Conversation

@dzbarsky

Copy link
Copy Markdown
Contributor

💥 What does this PR do?

This PR migrates to using rules_rs instead of rules_rust for our Cargo lockfile integration and toolchains. rules_rs is a redesign ruleset that can operate directly on Cargo.toml/Cargo.lock without the need for additional bazel lockfiles or a bazel-specific repin operation. The toolchains are also optimized to minimize downloads of duplicative bytes compared to the default rules_rust ones.

🔧 Implementation Notes

This was straightforward.

🤖 AI assistance

Codex 0-shot :) It followed the instructions in rules_rs README perfectly.

💡 Additional Considerations

N/a

🔄 Types of changes

Cleanup / build ergonomics improvement

@CLAassistant

CLAassistant commented Apr 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@selenium-ci selenium-ci added B-build Includes scripting, bazel and CI integrations C-rust Rust code is mostly Selenium Manager B-manager Selenium Manager labels Apr 27, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Migrate Rust build rules from rules_rust to rules_rs

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Migrate Rust build system from rules_rust to rules_rs
• Simplify Cargo dependency management without bazel lockfiles
• Remove manual cargo repinning task and workflow
• Add platform-specific constraints for Linux and Windows
• Update Bazel module dependencies and toolchain configuration

Grey Divider

File Changes

1. MODULE.bazel ⚙️ Configuration changes +44/-10

Update Bazel dependencies and Rust toolchain configuration

MODULE.bazel


2. BUILD.bazel ⚙️ Configuration changes +16/-0

Add platform definitions for Linux and Windows

BUILD.bazel


3. .bazelrc ⚙️ Configuration changes +3/-1

Update build configuration for platform-specific settings

.bazelrc


View more (6)
4. rust/private/rustfmt_wrapper.bzl Dependencies +4/-8

Update Rust rule imports to use rules_rs

rust/private/rustfmt_wrapper.bzl


5. rust/BUILD.bazel ✨ Enhancement +4/-1

Add aliases support for Rust binary and library targets

rust/BUILD.bazel


6. rust/tests/BUILD.bazel ✨ Enhancement +2/-1

Add aliases support for Rust test suite

rust/tests/BUILD.bazel


7. rake_tasks/rust.rake Cleanup +0/-16

Remove manual cargo repinning task

rake_tasks/rust.rake


8. README.md 📝 Documentation +1/-5

Simplify Rust dependency documentation

README.md


9. rust/AGENTS.md 📝 Documentation +0/-1

Remove cargo repinning command from documentation

rust/AGENTS.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1)

Grey Divider


Action required

1. Missing rust:update task 🐞 Bug ≡ Correctness
Description
The PR deletes the rust:update rake task, but Rakefile still invokes it in release_updates,
which will fail at runtime with an unknown task error. This breaks the release preparation workflow
when running rake release_updates.
Code

rake_tasks/rust.rake[L15-30]

-desc 'Update the rust lock files'
-task :update do
-  # The first repin after a version bump can fail due to a stale checksum in
-  # Cargo.Bazel.lock; a second run lets Bazel reconcile and succeeds.
-  puts 'pinning cargo versions'
-  begin
-    Bazel.execute('fetch', ['--repo_env=CARGO_BAZEL_REPIN=true'], '@crates//:all')
-  rescue RuntimeError
-    puts 'repin failed, retrying...'
-    Bazel.execute('fetch', ['--repo_env=CARGO_BAZEL_REPIN=true'], '@crates//:all')
-  end
-end
-
-desc 'Pin Rust dependencies'
-task pin: :update
-
Evidence
Rakefile still calls Rake::Task['rust:update'].invoke as part of release_updates, but
rake_tasks/rust.rake no longer defines any task :update (only
build/format/lint/changelogs/version remain). Therefore, rake release_updates will raise “Don't
know how to build task 'rust:update'”.

Rakefile[104-126]
rake_tasks/rust.rake[9-51]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rake release_updates` invokes `rust:update`, but this PR removed the `rust:update` task definition from `rake_tasks/rust.rake`, so the release workflow will now fail with an unknown-task error.

### Issue Context
With the switch to `rules_rs`, there is no longer a Bazel-specific Rust lockfile repin step, so the old `rust:update` task was deleted. The top-level release task still expects it to exist.

### Fix Focus Areas
- Rakefile[112-122]
- rake_tasks/rust.rake[9-25]

### Suggested fix
Remove `Rake::Task['rust:update'].invoke` from the `release_updates` task (or reintroduce a `rust:update` task as a backward-compatible no-op if you still want the task name to exist).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Removed rust:update rake task 📘 Rule violation ⚙ Maintainability
Description
The PR deletes the rust:update and rust:pin rake tasks without first providing a deprecation
shim/message pointing users to the new workflow. This can break existing contributor automation and
violates the requirement to deprecate public functionality before removal with clear alternatives.
Code

rake_tasks/rust.rake[L15-30]

-desc 'Update the rust lock files'
-task :update do
-  # The first repin after a version bump can fail due to a stale checksum in
-  # Cargo.Bazel.lock; a second run lets Bazel reconcile and succeeds.
-  puts 'pinning cargo versions'
-  begin
-    Bazel.execute('fetch', ['--repo_env=CARGO_BAZEL_REPIN=true'], '@crates//:all')
-  rescue RuntimeError
-    puts 'repin failed, retrying...'
-    Bazel.execute('fetch', ['--repo_env=CARGO_BAZEL_REPIN=true'], '@crates//:all')
-  end
-end
-
-desc 'Pin Rust dependencies'
-task pin: :update
-
Evidence
PR Compliance ID 7 requires public functionality to be deprecated (with a message pointing to
alternatives) before removal. The diff removes the desc 'Update the rust lock files' / `task
:update and desc 'Pin Rust dependencies' / task pin: :update` definitions entirely, with no
deprecation alias retained.

AGENTS.md
rake_tasks/rust.rake[15-30]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rake tasks `rust:update` and `rust:pin` were removed without a deprecation period or a message pointing users to the new rules_rs-based workflow.

## Issue Context
Some contributors/automation may still call these tasks. The compliance requirement expects a deprecation step that clearly communicates the replacement.

## Fix Focus Areas
- rake_tasks/rust.rake[15-30]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread rake_tasks/rust.rake
Comment thread MODULE.bazel
Comment thread rake_tasks/rust.rake
@AutomatedTester

Copy link
Copy Markdown
Member

@dzbarsky could you sign the CLA before we can look to merge it?

@dzbarsky

Copy link
Copy Markdown
Contributor Author

@dzbarsky could you sign the CLA before we can look to merge it?

yep, all good!

@shs96c shs96c left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the idea. Just a question.... Also, was this tested on macOS?

Comment thread rake_tasks/rust.rake
Comment thread .bazelrc
Comment thread rake_tasks/rust.rake
@dzbarsky

Copy link
Copy Markdown
Contributor Author

@cgoldberg thanks for pointing out the rust:update bit, I think I updated the callers but I'm not sure if I fully understand the templating that was going out there, mind sanity checking? I pushed those bits as a separate commit in this branch to make it easy!

@cgoldberg

Copy link
Copy Markdown
Member

@dzbarsky yea, I think your workflow changes look correct. thanks.

@dzbarsky

Copy link
Copy Markdown
Contributor Author

@cgoldberg I had a CI failure with the remote build using a platform without the constraint and thus failing toolchain resolution. Updated and repushed, could you help trigger CI? Would love a bit of help getting this in!

@cgoldberg

Copy link
Copy Markdown
Member

could you help trigger CI?

It's running now

@dzbarsky

Copy link
Copy Markdown
Contributor Author

could you help trigger CI?

It's running now

hmm i'm not sure how target selection is implemented but Rust job was skipped, that seems dangerous :)

@cgoldberg

Copy link
Copy Markdown
Member

Rust job was skipped

I'm not sure how the target selection works either, and I can't figure out how to trigger the Rust job manually on your branch :/

@cgoldberg

Copy link
Copy Markdown
Member

CI is failing all over the place because GitHub Actions is having issues right now. I'll try to figure it out once they get things stabilized.

@dzbarsky dzbarsky force-pushed the zbarsky/rules_rs branch 4 times, most recently from c0fbd58 to 26149f2 Compare May 6, 2026 14:03
@cgoldberg

Copy link
Copy Markdown
Member

@dzbarsky

dzbarsky commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

build is failing on Windows: https://github.com/SeleniumHQ/selenium/actions/runs/25440138960/job/74647344814?pr=17398#step:23:196

Yep I need to fix @llvm module. Mentioned to Simon on slack but should have posted here as well to keep you in the loop. My bad!

@dzbarsky dzbarsky force-pushed the zbarsky/rules_rs branch from 26149f2 to a371c75 Compare May 7, 2026 16:07
@dzbarsky dzbarsky force-pushed the zbarsky/rules_rs branch 2 times, most recently from 51e7a73 to 7a4953b Compare May 7, 2026 20:41
@dzbarsky dzbarsky force-pushed the zbarsky/rules_rs branch from 7a4953b to 3168a5d Compare May 7, 2026 20:59
@shs96c shs96c dismissed cgoldberg’s stale review May 8, 2026 08:58

It looks like David has responded to all the requested changes, and it also sounds like the rake workflow should work too.

@shs96c shs96c merged commit e53d91f into SeleniumHQ:trunk May 8, 2026
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations B-manager Selenium Manager C-rust Rust code is mostly Selenium Manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants