Repository navigation
Optional crate identity metadata for rules_rs to not duplicate rlibs - #57
Draft
genevieve-me wants to merge 100 commits into
Draft
genevieve-me wants to merge 100 commits into
genevieve-me wants to merge 100 commits into
Conversation
This reverts commit 59a507e.
Add support for tier 3 targets bpfeb-unknown-none and bpfel-unknown-none (see https://github.com/rust-lang/rust/blob/f5e2df7/src/doc/rustc/src/platform-support.md?plain=1#L311-L312). This is modeled after bazelbuild#3507 and should probably be updated if/when bazelbuild/platforms#131 is merged. (please use rebase merge when landing this as the proper commit message is in the commit, rather than the PR description) /cc @avrabe
…elbuild#3829)" This reverts commit f198dde.
…() (bazelbuild#3816)" This reverts commit 9586468.
…hermeticbuild#3) * 0 * Add rust analyzer test coverage
…hollow rlibs: the RustcMetadata action runs rustc to completion with -Zno-codegen, emitting a .rlib archive. This approach mirrors the one used by buck2 and avoids needing to kill rustc mid-output in order to produce metadata. While not fixing problems with SVH mismatches when non-determinism, this does simplify the codepath and uses a production tested technique that doesn't have any of the dangers associated with killing the rustc process while it's still active.
Port the sharding wrapper feature from bazelbuild#3774 into the hermeticbuild fork. The implementation wraps rust_test executables when experimental_enable_sharding is set while keeping rustc_compile_action's existing provider-list API for internal and extension callers. rust_test now scans the returned providers to replace DefaultInfo for the wrapper, so extensions such as prost and wasm-bindgen continue to consume rustc_compile_action without API churn. Co-authored-by: Brian Duff <bduff@linkedin.com> Co-authored-by: Codex <noreply@openai.com>
Rustc emits GNU-like Windows staticlibs as lib<crate>.a, but rules_rust was stripping the lib prefix for all Windows non-rlib library outputs. Keep the prefix for staticlib outputs when the target ABI is gnu or gnullvm so declared outputs match rustc.
Add documentation extraction targets for the public cargo and rust Starlark packages. Declare the bazel_features, selects, and cc_debug_helper_bzl dependencies required by those targets.
) Scan code-generating rustc outputs for the resolved ${pwd} value after successful compilation. Reject embedded CARGO_MANIFEST_DIR, OUT_DIR, and other sandbox paths while preserving compile-time include_str! and relative paths. Inspect cargo_build_script OUT_DIR through test runfiles instead of retaining compile-action paths. Assisted-by: OpenAI Codex
* rustc: stage cc dynamic libraries in test runfiles The cc dynamic libraries a target loads at runtime were collected into its runfiles only when the crate type was bin, cdylib or staticlib. A rust_test reports the type of the crate under test -- "lib" for rust_test(crate = ":foo") -- so tests were skipped entirely. The collection also walked only `deps`, never `crate`, so even a bin built that way would have missed them. A rust_test depending on a cc_import shared library therefore produced runfiles containing just the executable and the repo mapping. Its RUNPATH is $ORIGIN/../../_solib_<cpu>/..., so it runs only where that execroot directory happens to sit beside the binary. Locally it does, and the test passes. Under remote execution the input tree is exactly the declared runfiles, and the test dies at load time with `cannot open shared object file`. Gate on crate_info.is_test as well, and scan `crate` alongside `deps`. * test: cover imported dylibs in rust_test runfiles A cc_binary(linkshared = True) in `deps` already reached a consumer's runfiles through the generic deps traversal, so the existing check_runfiles cases passed regardless of the dynamic library collection. A cc_import contributes no runfiles of its own -- the dylib is only in its CcInfo linking context -- so it isolates that collection. Adds two cases over an imported dylib: a rust_binary (which reaches it through `deps`) and a rust_test (through `crate`). The rust_test case fails without the preceding commit and passes with it. * ci: bump llvm to 0.8.4 for web.archive.org id_ URL fix llvm 0.7.7 pins the macOS SDK archive at a web.archive.org URL without the id_ modifier (web/20260430051604/...), which now serves the Wayback HTML wrapper page instead of the raw pkg. The download checksum-mismatches and the build aborts during analysis of any target that transitively fetches @macos_sdk (e.g. //test/genquery:bar_deps), skipping every test. llvm 0.8.4+ uses the id_ modifier (web/20260430051604id_/...) which returns the raw file with the correct sha256. Bump the bazel_dep to unblock CI.
Allow disabling the default hermetic macOS SDKROOT globally. Preserve explicit SDKROOT values and C/C++ toolchain SDKROOT.
Use source_file_label to preserve filenames relative to the owning Bazel package when query_label_for builds the srcs query. Files such as src/adb.rs now match the target's source label instead of :adb.rs. Add a regression test for root packages, nested directories, and direct source files. The test fails with the old basename behavior. Based on bazelbuild#4238; addresses hermeticbuild/rules_rs#232. Validation: 10 Flycheck tests and 44 rust-analyzer library tests pass through rules_rs. --saved-file command checks pass for four package layouts.
…eticbuild#48) ``` On Windows, consolidate_dependency_search_paths (util/process_wrapper/main.rs:156) collapses every -Ldependency=<dir> rustc flag into one unified directory, to avoid Windows command-line size overruns (commit 3f613e4). It does so by read_dir-ing each search path and hardlinking (falling back to copying) every regular file it finds into that unified dir. However, builds failed with: Error: ProcessWrapperError("unable to copy .../ijent_util-subscriber_contract-test.exe.tmp8ceb5a4 into unified dependency dir ...: Access is denied. (os error 5)") .exe.tmpXXXXXXX file is an LLVM's atomic-output temp file. Every LLVM tool that writes output this way does it: lld-link / rust-lld (the PE image and the PDB), llvm-lib (import libs), llvm-objcopy. So this file is rust-lld writing the sibling target ijent_util-subscriber_contract-test.exe. It only occurs with toolchain.linker_preference == "rust" (rustc.bzl:445-448); MSVC's link.exe writes in place. Lifecycle of that temp file ┌────────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ │ Phase │ What happens │ ├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ Created │ At the start of the sibling's link step. TempFile::create passes OF_Delete, then immediately calls │ │ │ setDeleteDisposition(H, true) → DELETE_PENDING from birth. │ ├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ Alive │ For the entire duration of that link, while lld mmaps and writes the image into it. │ ├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ Deleted │ FileOutputBuffer::commit() → TempFile::keep(FinalPath): setDeleteDisposition(H, false), then rename_handle(H, │ │ (success) │ "…-test.exe"). The temp name vanishes by rename. │ ├────────────────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤ │ Deleted │ TempFile::discard() — the already-set delete disposition does it on close; RemoveFileOnSignal covers crashes. │ │ (failure) │ │ └────────────────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ Why os error 5 and not os error 2 Because of DELETE_PENDING. On Windows a delete-pending file: - still appears in directory enumeration — fs::read_dir yields it and entry.file_type() succeeds (metadata comes from the dirent); - but every CreateFile on it fails with STATUS_DELETE_PENDING → ERROR_ACCESS_DENIED (5). So fs::hard_link (CreateHardLinkW) fails with 5, the fs::copy (CopyFileExW) fallback fails with 5, and the wrapper escalates to a fatal ProcessWrapperError. LLVM's own createUniqueEntity retries on errc::permission_denied for exactly this reason. This is not a narrow race. The window is the whole link of the sibling target, and the failure is deterministic once read_dir observes the temp file. Why the temp file is in a -L dependency= directory at all rust/private/rustc.bzl:2994-2998 derives the search paths from the dirnames of dep_info.transitive_crate_outputs (_get_dirname, rustc.bzl:3433). A dirname is an entire Bazel package bin dir (bazel-out/<cfg>/bin/fleet/native/ijent/ijent_util/) containing every output of that package — sibling .exes, .pdbs, .os, .rustc-outputs, tree artifacts, in-flight linker temps — not a curated set of crate artifacts. Bazel has no sandboxing on Windows, so those scanned directories are the live bazel-out tree with every concurrent action's activity visible. ```
Remove cargo_toml_env_vars inputs from the determinism BUILD files and match RustdocTest in the C++ runtime analysis test. Restore the dynamic standard-library default for construct_arguments callers and include C++ linker files in Windows GNU library compile inputs for dlltool. Add dynamic-standard-library assertions for Clippy and rustdoc tests, and cover dlltool inputs for GNU, GNU LLVM, MSVC, and Linux targets.
Rust generates import libraries in-process for Windows GNU LLVM and MSVC. Restrict the dlltool argument and C++ linker-file inputs to Windows GNU, where raw-dylib compilation invokes dlltool. Update the rlib input test so GNU LLVM retains no C++ linker files.
Expand C++ linker paths and flags in Args.map_each callbacks, so Bazel can apply the Rustc action's path mapper. Preserve the eager get_linker_and_args interface for Cargo build scripts and bindgen, and retain Rust linker selection and flag filtering. Defer Windows GNU dlltool paths as well. Add an analysis regression with a generated C++ linker, linker script, and LIB-derived search directory. It passes with mapping off on Bazel 9.1.0 and 9.3.0-dzbarsky18, and with strip on a 9.3.0-dzbarsky18 build containing bazelbuild/bazel#30789. It fails with strip on unpatched Bazel. All 21 selected linker/native-dependency/SDKROOT analysis tests pass with mapping off and on. Codex at 52e73e3a builds //codex-rs/cli:codex with strip after backporting this change to its rules_rust pin, preserving its patches. Unmodified rules_rust fails to find Scrt1.o and libc with strip, including with the Bazel backport alone.
Default experimental_use_allocator_libraries_with_mangled_symbols to true so cc_common.link and C++ consumers of Rust libraries receive allocator symbols generated by the selected rustc. Preserve the explicit opt-out for custom and legacy C++ allocator libraries and document the nightly requirement for the global allocator implementation. Remove redundant allocator flags from integration fixtures and ThinLTO tests. Cover heap allocation through C++ and per-target cc_common.link, default allocator CcInfo, and explicit opt-out. Use the C++ bootstrap wrapper in the mock codegen-units toolchain to avoid a dependency cycle. Validated with Bazel 9.1.0 on macOS: 20 cc_common.link integration tests, 32 CcInfo/allocator/ThinLTO unit tests, and the pinned nightly global allocator test. Per-target and C++ allocation tests also pass with the global cc_common.link setting disabled; the C++ test passes with dynamic libstd. Dynamic-libstd Rust tests encounter a getopts link failure also reproduced on unchanged main. Buildifier and git diff --check pass. Assisted-by: OpenAI Codex
Bootstrap actions pass rustc flags through @params, but the C++ bootstrap process wrapper substitutes only direct arguments. The literal remap placeholders leave execution directories in tinyjson crate metadata. Use -Zremap-cwd-prefix for bootstrap compilation so rustc resolves its own working directory. Default RUSTC_BOOTSTRAP to the bootstrap crate's name, preserving explicit environment settings. Keep the existing response files and SDKROOT expansion without changing the C++ wrapper. Add a regression test that reads the actual bootstrap tinyjson archive. The test fails before the fix. Sandboxed and local builds in different output bases now produce identical archives without execution directories. Validation: Bazel 9.1.0, Rust 1.98.0, macOS arm64; bazel test //... passed 621 tests and skipped 37 platform-incompatible tests. Refs hermeticbuild/rules_rs#233 and hermeticbuild#46. Co-authored-by: Codex <noreply@openai.com>
…alidation Prep work to fix the upstream hermeticbuild/rules_rs issue 144. Motivation A crate can be pulled from two different registries or resolved differently across Bzlmod hubs, producing two identical-name rlibs in one binary. For crates with global/static state (e.g., log, tracing), that breaks shared-state assumptions. There should be at most one configured instance per source-qualified identity per link unit. Adds an opt-in crate_identity attribute to library-producing Rust rules and a RustCrateIdentityInfo/RustLinkClosureInfo provider pair so a configured Rust library can carry a source-qualified logical identity. This commit does NOT merge or deduplicate crate instances; it only carries the identity and enforces the per-link-unit invariant (at most one configured instance of each source-qualified identity per native link unit) at analysis time, intrinsically for Rust terminal link units and via an opt-in aspect or the rust_link_checked_* wrappers for native C/C++ links. Deduplication of compatible same-source packages across independent hubs is performed upstream by the consuming crate generator (rules_rs interner).
Align crate_identity docs with reality: - The crate generator emits a source-qualified identity (source, name, version) as 'cargo:' + canonical record, not the non-existent fully_qualified_cargo_package_id helper; the generator merges compatible same-source packages, this feature only carries/validates the identity. - State explicitly that cross-registry same-name is a distinct identity accepted by design (private-registry shadowing), and that the enforced invariant is at most one configured instance per source-qualified identity per link unit. - Make the opt-in nature of native link validation explicit.
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
11 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.
This is preliminary work to solve issue hermeticbuild/rules_rs#144. It doesn't do anything unless combined with a PR on rules_rs (which I will open shortly).
It adds support for optional crate identity metadata as initial prep work to let rules_rs not duplicate rlibs from different crate hubs, as described in the above issue. Ie, if my app A depends on library B (internal Bazel dependency) and library C (crates.io dependency) and both depend on
tracing, right now the tracing library will end up duplicated in the built app A. This adds some metadata that can be used by rules_rs when driving rules_rust in order to 'coalesce' these dependencies into one linked library wherever possible.When targets opt into this extra logical identity, an aspect validates Rust link units to make sure we don't end up with incompatible duplicate instances of the same Cargo package.
Note: this is a contribution as part of work done at my employer Xona Space and has been approved to open source, the CLA will fail but should be fixed very shortly.
Disclaimer: Generative AI was used heavily in the creation of this branch, but it has been tested and used on a real Rust codebase and all responsibility for the code and design rests with me, the author.
Overview
This adds:
Performance
The new internal aspect is attached to every Rust rule’s deps and link_deps in rust/private/rust.bzl.
I think this is justified but it does affect build performance. this does not apply to any local path dependencies/libraries, which are not deduped at all.
When tested with my rules_rs branch implementing this:
Synthetic analys time, from my rules_rs branch using these rules_rust changes vs pre-MR latest commit to rules_rs (and its pinned rules_rust),
So performance is technically eventually polynomial in dependency depth driven by rustc.bzl _link_closure_identities() calling dep_info.transitive_crates.to_list() at every rustc_compile and the aspect then aggregating that. By dependency depth, I mean A depends on B depends on C depends on...
Real apps have bounded depth and would never have transitive dependencies >1000 deep, so the cost stays <1 second in real apps I tried testing on like
zoxide. only unrealistically deep synthetic stacks blow up.