chore: upgrade rules_rust to 0.71.3 - #10632
Conversation
- Drop bazel/rules_rust_package_alias.patch: the package_alias attribute is upstreamed in rules_rust 0.71.3. - Regenerate bazel/rules_rust_strip_level.patch for 0.71.3: still needed because the native rust.toolchain strip_level tag attribute is a flat string_dict that cannot express the per-compilation-mode config. - Repin Cargo Bazel lockfile (checksum only).
Two regressions surfaced by the rules_rust 0.69.0 -> 0.71.3 upgrade:
1. Cross-crate DEP_* env vars broke: 0.71.x redacts the producing build
script's out_dir to a generic ${out_dir} token in dep env files, but
those files are consumed by downstream crates' build scripts whose
--out-dir differs, so the token never resolves (libssh2-sys could not
find zlib.h from libz-sys's DEP_Z_INCLUDE). New
bazel/rules_rust_dep_env.patch restores exec-root-only redaction for
dep env files.
2. rustdoc doc tests failed to link with 'running strip: FileNotFound':
0.71.3 no longer leaks the compile action env (incl. PATH) into
CrateInfo.rustc_env (bazelbuild/rules_rust#3989), so the generated
doctest runner script clears the environment without exporting PATH,
and our zig-wrapper strip hook (hermetic_cc_toolchain_strip.patch)
could no longer resolve the bare 'strip' name. The wrapper now
resolves strip to an absolute path with a PATH-based fallback.
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Bazel rules_rust dependency to v0.71.3 and updates the repo’s patch set and Cargo Bazel lockfile to match, aiming to keep existing behavior (notably around strip levels) while adopting the upstreamed package_alias support.
Changes:
- Bump
rules_rustfrom0.69.0to0.71.3and update the archive integrity/URL. - Remove the now-upstreamed
rules_rust_package_alias.patch, regenerate/updaterules_rust_strip_level.patch, and add a newrules_rust_dep_env.patch. - Repin
Cargo.Bazel.json.lock(checksum-only change).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.Bazel.json.lock | Updates the pinned lock checksum after repinning. |
| bazel/rust.MODULE.bazel | Bumps rules_rust version/integrity and updates the applied patch list. |
| bazel/rules_rust_strip_level.patch | Updates the rules_rust patch that overrides strip level handling. |
| bazel/rules_rust_package_alias.patch | Removes the no-longer-needed patch (upstreamed). |
| bazel/rules_rust_dep_env.patch | Introduces a patch intended to fix cross-crate DEP_* env var handling in rules_rust 0.71.x. |
| bazel/hermetic_cc_toolchain_strip.patch | Adjusts hermetic toolchain stripping behavior (notably strip resolution). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add standard 'diff --git' header to rules_rust_dep_env.patch for easier auditing and patch-applicator compatibility. - zig-wrapper: fail with a clear error when no strip binary is found instead of falling back to a bare "strip" that cannot resolve without PATH. - Document the purpose of each rules_rust patch inline in rust.MODULE.bazel.
rules_rust 0.71.x (bazelbuild/rules_rust#4088) started prefixing every path expanded from the plural $(locations ...) / $(execpaths ...) forms with the ${pwd} substitution token, like it already did for the singular forms. That token is only substituted by the process wrapper during build actions; nothing substitutes it in a test's runtime environment (RunEnvironmentInfo), so tests saw literal '${pwd}/rs/...' paths and failed to open them. Use $(rootpaths ...) which rules_rust deliberately leaves unmodified for runtime use and which expands to the same runfiles-relative paths as $(locations ...) did before 0.71.x for these source files. Fixes: //rs/dogecoin/ckdoge/minter:unit_tests //rs/nns/governance:governance_integration_test //rs/nns/governance:merge_neurons_test //rs/nns/integration_tests:integration_tests_src/governance_neurons //rs/tla_instrumentation:args_test //rs/tla_instrumentation:multiple_calls_test //rs/tla_instrumentation:structs_test
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
|
✅ No security or compliance issues detected. Reviewed everything up to 082d8ae. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 082d8ae. Security Overview
Detected Code Changes
|
### The bug Since the `OUT_DIR` sanitization work (#4050 / #4011, first released in 0.71.x), `outputs_to_dep_env` redacts the producer's `out_dir` to the generic `${out_dir}` substitution token, the same way `outputs_to_env` does. That redaction is only correct for `_bs.env` files, which are consumed by the target that directly owns the build script (where `process_wrapper`'s `--out-dir` resolves the token to the right directory). Dep env (`DEP_*`) files, however, are consumed by *downstream* crates' build scripts: their runner only substitutes `${pwd}`, and their own `out_dir` points to a different directory. The token is therefore left unresolved, or would resolve to the wrong directory. Real-world failure: `libssh2-sys`'s build script fails to find `zlib.h` because `libz-sys`'s `DEP_Z_INCLUDE` contains a literal `${out_dir}` path component. Found while upgrading `rules_rust` to 0.71.3 in dfinity/ic (see dfinity/ic#10632, where this fix is currently carried as a patch). ### The fix Only substitute the exec root in dep env files and keep the real `out_dir` path. That path is valid for consumers: the producer's `out_dir` is a declared input of downstream build script actions. ### Tests * Unit test `out_dir_in_dep_env_value_is_not_redacted_to_substitution_token` in `cargo/private/cargo_build_script_runner/lib.rs`. * End-to-end regression test `//cargo/tests/dep_env:build_read_out_dir` mirroring the libz-sys → libssh2-sys scenario: a producer build script advertises `cargo:include=$OUT_DIR/include` and the consumer build script asserts `DEP_Z_INCLUDE` points at an existing directory. Fails without the fix, passes with it. --- Assisted-by: GitHub Copilot
Upgrades
rules_rustfrom 0.69.0 to 0.71.3.bazel/rules_rust_package_alias.patch: thepackage_aliasattribute is upstreamed in 0.71.3 (rules_rust#3742).bazel/rules_rust_strip_level.patchfor 0.71.3: still needed because the nativerust.toolchainstrip_leveltag attribute is a flatstring_dictthat cannot express the per-compilation-mode config (and is looked up by target triple downstream) (rules_rust#3730).bazel/rules_rust_dep_env.patch: rules_rust 0.71.x redacts the producer's out_dir to a generic${out_dir}token in dep env files, but those files are consumed by downstream crates' build scripts whose--out-dirdiffers, leaving the token unresolved (e.g.libssh2-sysfailing to findzlib.hfromlibz-sys'sDEP_Z_INCLUDE).bazel/hermetic_cc_toolchain_strip.patch: the zig-wrapper now invokesstripvia an absolute path (/usr/bin/stripor/bin/strip, failing with a clear error if neither exists), because rules_rust 0.71.x runs the wrapper at rustdoc doc-test time with a cleared environment (noPATH), where a barestripfails to resolve.envattributes from$(locations ...)to$(rootpaths ...): since rules_rust#4088 (0.71.x) the plural$(locations ...)/$(execpaths ...)forms are${pwd}-prefixed like the singular forms; that token is only substituted during build actions, so test runtime envs saw literal${pwd}/...paths.$(rootpaths ...)is deliberately left unmodified for runtime use.