Fix unresolved ${out_dir} token in dep env files - #4124
Merged
UebelAndre merged 1 commit intoJul 5, 2026
Conversation
`outputs_to_dep_env` redacted the producer's out_dir to the generic
`${out_dir}` substitution token, the same way `outputs_to_env` does.
But dep env (`DEP_*`) files are consumed by *downstream* crates' build
scripts, whose runner only substitutes `${pwd}` and whose own out_dir
points to a different directory, so the token was left unresolved (or
would resolve to the wrong directory). For example, libssh2-sys failed
to find zlib.h from libz-sys's DEP_Z_INCLUDE.
Only substitute the exec root in dep env files and keep the real
out_dir path, which is a declared input of downstream build script
actions.
Assisted-by: GitHub Copilot
basvandijk
marked this pull request as ready for review
July 2, 2026 13:54
UebelAndre
self-requested a review
July 2, 2026 14:06
basvandijk
marked this pull request as draft
July 2, 2026 14:32
Contributor
Author
|
EDIT: False alarm. The bug was caused by something else and fixed in dfinity/ic@082d8ae. |
basvandijk
marked this pull request as ready for review
July 2, 2026 14:48
UebelAndre
enabled auto-merge
July 5, 2026 14:18
2 tasks
pull Bot
pushed a commit
to mikeyhodl/ic
that referenced
this pull request
Jul 27, 2026
…ity#10900) _Requested by **Nicolas Mattia** · [Slack thread](https://dfinity.slack.com/archives/CL7Q2RXUM/p1784901121254099)_ ## What Upgrades the `rules_rust` Bazel module pin in `bazel/rust.MODULE.bazel` from the `0.71.3` release tarball to a `codeload.github.com` snapshot of the latest commit on `bazelbuild/rules_rust`'s default branch, matching the `archive_override` + codeload pattern already used for other Bazel modules in this repo (e.g. `rules_motoko`, `rules_foreign_cc` in the root `MODULE.bazel`). - **Before:** `https://github.com/bazelbuild/rules_rust/releases/download/0.71.3/rules_rust-0.71.3.tar.gz` (release 0.71.3) - **After:** `https://codeload.github.com/bazelbuild/rules_rust/tar.gz/5f04c8c14c2ffacfe10ad14e55298be21454be04` (commit [`5f04c8c1`](bazelbuild/rules_rust@5f04c8c), latest on `main` as of 2026-07-24: "Add `cargo_build_script.use_cc_toolchain` (dfinity#4161)") - `strip_prefix` updated to `rules_rust-5f04c8c14c2ffacfe10ad14e55298be21454be04` and the `integrity` hash recomputed for the new tarball (sha256 of the codeload archive fetched from an unrestricted network, converted to the `sha256-<base64>` SRI form to match the field format already used by this override). ## Patches - **Removed** `bazel/rules_rust_dep_env.patch`: this worked around `DEP_*` cross-crate env vars being redacted to an unresolved `${out_dir}` token in build-script dep-env files (e.g. `libssh2-sys` failing to find `zlib.h` via `libz-sys`'s `DEP_Z_INCLUDE`). This is now fixed upstream by bazelbuild/rules_rust#4124 ("Fix unresolved `${out_dir}` token in dep env files"), which is included as of the new pinned commit — confirmed by diffing `cargo/private/cargo_build_script_runner/lib.rs` at the new commit against the patch: the upstream code now uses `redact_exec_root` instead of full `redact_paths` in `outputs_to_dep_env`, matching the patch's intent, and even includes the same libssh2-sys/libz-sys regression test. The patch no longer applies (`git apply --check` fails cleanly against the new file content) and its reference was removed from `rust.MODULE.bazel`. - **Kept** `bazel/rules_rust_strip_level.patch` unchanged: it configures per-compilation-mode strip levels, which the native `rust.toolchain` tag's flat `strip_level` `string_dict` still cannot express (bazelbuild/rules_rust#3730 is still open upstream — traced the full call chain through `rust/extensions.bzl` → `rust/private/repositories.bzl` → `rust/private/toolchain.bzl`'s `_rust_toolchain_impl`, which still requires a per-compilation-mode dict at the rule level that the bzlmod extension tag cannot produce). Verified the patch still applies cleanly (`git apply --check`, 3-line offset, no fuzz) against `rust/extensions.bzl` fetched at the new target commit. - No other patches reference `rules_rust` in this repo. - No `MODULE.bazel.lock` file exists in this repo, so there is nothing to regenerate there. ## Validation What I was able to run in my sandbox: - `buildifier -mode=check bazel/rust.MODULE.bazel` — clean, no reformatting needed (exit 0). - `buildifier -lint=warn bazel/rust.MODULE.bazel` — only pre-existing warnings unrelated to this change (canonical-repository `@@` warnings elsewhere in the file, far from the diff). - Fetched the exact files both patches touch at the exact target commit (`rust/extensions.bzl`, `cargo/private/cargo_build_script_runner/lib.rs`) and ran `git apply --check` against them locally, as described above. - The `sha256`/`integrity` value was computed from a real fetch of the codeload tarball for the target commit, done outside my sandbox (my sandbox's network egress blocks `codeload.github.com` for any repo other than `dfinity/ic`), then converted to SRI format and round-tripped back to hex to confirm the conversion was correct. What I could **not** run in my sandbox, and why: - `bazel build` / `bazel test` / `bazel fetch` for this change specifically: my sandbox's network egress blocks (a) `codeload.github.com` tarball fetches for non-`dfinity` repos, (b) the `ghcr.io`/`pkg-containers.githubusercontent.com` blob storage needed to pull the pinned `ic-dev` dev-container image, and (c) `releases.bazel.build` binary downloads needed to get a matching Bazel 9.2.0 outside the container. All three are independent, hard network-egress restrictions in that environment, not fixable from within it. Given that, **this PR's actual build/test signal will need to come from CI** once it runs here. Please treat a green CI run (in particular anything that exercises `@rules_rust` toolchains/`cargo_build_script`) as the real confirmation that the new pin, hash, and patch set are correct before merging this out of draft. ## Testing - [ ] CI build/test is green (this is the validation this sandbox could not produce locally — see above) - [ ] Confirm no other in-repo patches or docs reference `bazel/rules_rust_dep_env.patch` --- _Generated by [Claude Code](https://claude.ai/code/session_01WYQSaxC3QcWKWQi7rDHe27)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org> Co-authored-by: Nicolas Mattia <nicolas.mattia@dfinity.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Since the
OUT_DIRsanitization work (#4050 / #4011, first released in 0.71.x),outputs_to_dep_envredacts the producer'sout_dirto the generic${out_dir}substitution token, the same wayoutputs_to_envdoes.That redaction is only correct for
_bs.envfiles, which are consumed by the target that directly owns the build script (whereprocess_wrapper's--out-dirresolves 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 ownout_dirpoints 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 findzlib.hbecauselibz-sys'sDEP_Z_INCLUDEcontains a literal${out_dir}path component. Found while upgradingrules_rustto 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_dirpath. That path is valid for consumers: the producer'sout_diris a declared input of downstream build script actions.Tests
out_dir_in_dep_env_value_is_not_redacted_to_substitution_tokenincargo/private/cargo_build_script_runner/lib.rs.//cargo/tests/dep_env:build_read_out_dirmirroring the libz-sys → libssh2-sys scenario: a producer build script advertisescargo:include=$OUT_DIR/includeand the consumer build script assertsDEP_Z_INCLUDEpoints at an existing directory. Fails without the fix, passes with it.Assisted-by: GitHub Copilot