Add strip_level_select for per-target stripping - #4167
Open
nmattia wants to merge 1 commit into
Open
Conversation
Contributor
Author
|
@illicitonion I wasn't sure about the exact semantics of |
Closes bazelbuild#3730 This adds a new tag class, `strip_level_select`, which allows specifying the strip level for a collection of triples (inspired by `annotation_select` in the `crate` extension).
nmattia
force-pushed
the
nm-strip-level-select
branch
from
July 24, 2026 12:27
f23049b to
f8f382c
Compare
This was referenced Jul 24, 2026
pull Bot
pushed a commit
to bit-cook/ic
that referenced
this pull request
Jul 27, 2026
This drops our local rules_rust patch which we used to hardcode strip values, and instead use the new `strip_level_select` tag: bazelbuild/rules_rust#4167
illicitonion
self-requested a review
September 2, 2026 15:48
pull Bot
pushed a commit
to mikeyhodl/ic
that referenced
this pull request
Sep 11, 2026
Upgrades `rules_rust` from 0.71.3 to 0.74.0, stops depending on the `dfinity/rules_rust` fork, and drops two local patches that 0.74.0 makes redundant. ## Drops the fork The pin was an `archive_override` on `dfinity/rules_rust`, whose sole commit adds a `strip_level_select` tag class ([bazelbuild/rules_rust#4167](bazelbuild/rules_rust#4167), still open upstream). That tag's only caller was removed in 9e29205 when hermetic-llvm replaced zig cc — clang emits deterministic debug info, so the `strip_level_select(none)` workaround went away with it. `strip_level` now appears nowhere in the repo outside the patch file, so the fork is dead code. ```diff -bazel_dep(name = "rules_rust", version = "0.71.3") -archive_override( +bazel_dep(name = "rules_rust", version = "0.74.0") +single_version_override( module_name = "rules_rust", - integrity = "sha256-5GXwZtp5VlNxMI+dolYYxhyhG5f5ReZ5qz30gAbPsrg=", patch_strip = 1, patches = ["//bazel:rules_rust_static_runtime.patch"], - strip_prefix = "rules_rust-f8f382c7e351a797242ce4a92262342f8994e265", - type = "tar.gz", - urls = ["https://codeload.github.com/dfinity/rules_rust/tar.gz/f8f382c7e351a797242ce4a92262342f8994e265"], ) ``` No integrity hash, no `strip_prefix`, no URL, and no fork to rebase on every future release — a bump is now a one-line version change. Checked before switching: BCR's `rules_rust/0.74.0/source.json` carries no patches of its own, and `rules_rust` is depended on **only by the root module** (`bazel mod explain rules_rust`), so nothing else can pull MVS off 0.74.0. ## Regenerates `bazel/rules_rust_static_runtime.patch` The `rust_doc_test` half of the patch is **upstream as of 0.74.0** ([dfinity#4220](bazelbuild/rules_rust#4220), commit `ed7afd2fc`) and is a superset of ours — it threads the runtime libs through `rustdoc_compile_action`'s return value rather than recomputing them, adds an empty-lib-root guard we lacked (an empty root would emit `--strip_substring=/`, which the writer applies as a plain string replacement), and ships a unit test. So the patch no longer carries it. What remains is the dylib/cdylib static-runtime preference, which is still not upstream and has no replacement mechanism (no new `unwindlib`, toolchain or rule attribute controls C++ runtime linkage in this range). dfinity#4220 also introduced a **new** site in `rustdoc.bzl` that picks the dynamic runtime for dylib/cdylib, which the old patch would have missed — leaving a cdylib doc test with the static runtime in its action inputs but the dynamic runtime's names in its `-l` flags. The policy therefore now lives in a single exported `cc_runtime_libs` helper, shared by all three sites that choose a runtime (`collect_inputs`, `_add_native_link_flags`, and rustdoc's doc-test link flags), so inputs and link flags stay consistent by construction. The patch applies with **zero fuzz and zero offset** (`git apply --check` and `patch -F0`), verified against the release tarball specifically, not just the git tag. ## Drops `bazel/rustix.patch` That patch deleted rustix's `can_compile()` probe artifact from `OUT_DIR` — which rules_rust captures as a cacheable `CargoBuildScriptRun` output — because the artifact was not byte-reproducible. 0.74.0 makes it unnecessary: [dfinity#4202](bazelbuild/rules_rust#4202) adds `--remap-path-prefix=${pwd}=.` to the `CARGO_ENCODED_RUSTFLAGS` that `cargo_build_script` passes to build scripts, and rustix's `can_compile()` forwards all of them to its probe `rustc`. ## Follow-ups (not in this PR) - The remaining dylib/cdylib half of the static-runtime patch has no upstream issue. Opening one would let it retire the way the doc-test half just did — though upstream deliberately links the dynamic runtime for shared libraries (its own `runtime_libs_test/shared_library` asserts exactly that), so it would need a toolchain-level opt-in rather than a straight behaviour change. - `.claude/CLAUDE.md`'s repin trigger could mention the `rules_rust` pin, since a bump on its own requires `./bin/bazel-pin.sh --force`. - If the rustix probe ever regresses (a toolchain bump, or anything adding a remap-scope rustflag), the fix is rules_rust 0.74.0's new `remove_nondeterministic_out_dir_files`: appending `rustix_test_can_compile` to `--@rules_rust//cargo/settings:out_dir_volatile_file_basenames` (re-listing its seven defaults, since the flag replaces rather than extends) empties `OUT_DIR` again with no crate patch and no repin. That, not a resurrected `bazel/rustix.patch`. - Worth an upstream issue: `cargo_build_script` could pass the same `${output_base}`/`${exec_root}` remaps that `rustc.bzl` already passes, closing the symlink gap described above. - The `dfinity/rules_rust` fork branches can be deleted once this merges; nothing will reference the fork any more. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes #3730
This adds a new tag class,
strip_level_select, which allows specifying the strip level for a collection of triples (inspired byannotation_selectin thecrateextension).