From 37f2635a8565b9a3e196fa527221a22a4711cee2 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 22 Jul 2026 15:56:02 +1000 Subject: [PATCH] Move some shared source files into a new `shim_utils` crate Bootstrap's `rustc` and `rustdoc` shims are mostly self-contained, but they do share some code with each other or with other bootstrap tools. Historically that sharing was achieved using `#[path]` attributes pointing to the shared files. That's unusual, making the code harder to navigate, and making dependencies and warnings harder to manage. It also causes problems for rust-analyzer, which currently struggles with source files outside the crate root. This PR therefore takes three such source files, and moves them into a new `shim_utils` crate, which can then be used as a normal crate-path dependency. That new crate is separate from the existing `build_helper` crate, since that crate contains other code that is not of interest to the shims specifically. Former paths of the files that have been moved: - `src/bootstrap/src/utils/proc_macro_deps.rs` - `src/bootstrap/src/utils/shared_helpers.rs` - `src/build_helper/src/arg_file_command.rs` --- Cargo.lock | 10 +++++++++- Cargo.toml | 1 + src/bootstrap/Cargo.lock | 8 ++++++++ src/bootstrap/Cargo.toml | 6 +++++- src/bootstrap/src/bin/rustc.rs | 16 +++------------- src/bootstrap/src/bin/rustdoc.rs | 13 +++---------- src/bootstrap/src/utils/helpers.rs | 4 ++-- src/bootstrap/src/utils/mod.rs | 1 - src/bootstrap/src/utils/tests/mod.rs | 5 +---- ...red_helpers_tests.rs => shim_utils_tests.rs} | 9 ++------- src/shim_utils/Cargo.toml | 11 +++++++++++ src/shim_utils/README.md | 6 ++++++ .../src/arg_file_command.rs | 8 -------- src/shim_utils/src/lib.rs | 17 +++++++++++++++++ .../utils => shim_utils/src}/proc_macro_deps.rs | 0 .../utils => shim_utils/src}/shared_helpers.rs | 15 --------------- src/tools/compiletest/Cargo.toml | 2 +- src/tools/compiletest/src/util.rs | 6 +----- src/tools/tidy/Cargo.toml | 1 + src/tools/tidy/src/deps.rs | 12 ++++++------ 20 files changed, 77 insertions(+), 74 deletions(-) rename src/bootstrap/src/utils/tests/{shared_helpers_tests.rs => shim_utils_tests.rs} (69%) create mode 100644 src/shim_utils/Cargo.toml create mode 100644 src/shim_utils/README.md rename src/{build_helper => shim_utils}/src/arg_file_command.rs (90%) create mode 100644 src/shim_utils/src/lib.rs rename src/{bootstrap/src/utils => shim_utils/src}/proc_macro_deps.rs (100%) rename src/{bootstrap/src/utils => shim_utils/src}/shared_helpers.rs (88%) diff --git a/Cargo.lock b/Cargo.lock index f55daf9e01c5f..7b842fef4bae4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -879,7 +879,7 @@ dependencies = [ "semver", "serde", "serde_json", - "tempfile", + "shim_utils", "tracing", "tracing-subscriber", "unified-diff", @@ -5328,6 +5328,13 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shim_utils" +version = "0.0.0" +dependencies = [ + "tempfile", +] + [[package]] name = "shlex" version = "1.3.0" @@ -5662,6 +5669,7 @@ dependencies = [ "rustc-hash 2.1.1", "semver", "serde", + "shim_utils", "similar", "tempfile", "termcolor", diff --git a/Cargo.toml b/Cargo.toml index 67c7a9d67edc8..0d36c3f242d82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ members = [ "src/rustc-std-workspace/rustc-std-workspace-core", "src/rustc-std-workspace/rustc-std-workspace-std", "src/rustdoc-json-types", + "src/shim_utils", "src/tools/build-manifest", "src/tools/bump-stage0", "src/tools/cargotest", diff --git a/src/bootstrap/Cargo.lock b/src/bootstrap/Cargo.lock index c05eeb97829b3..773af5f202bb2 100644 --- a/src/bootstrap/Cargo.lock +++ b/src/bootstrap/Cargo.lock @@ -60,6 +60,7 @@ dependencies = [ "serde_derive", "serde_json", "sha2", + "shim_utils", "sysinfo", "tar", "tempfile", @@ -661,6 +662,13 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shim_utils" +version = "0.0.0" +dependencies = [ + "tempfile", +] + [[package]] name = "shlex" version = "1.3.0" diff --git a/src/bootstrap/Cargo.toml b/src/bootstrap/Cargo.toml index cb96af268a6aa..436bc3c342d16 100644 --- a/src/bootstrap/Cargo.toml +++ b/src/bootstrap/Cargo.toml @@ -35,6 +35,7 @@ test = false cc = "=1.2.28" cmake = "=0.1.54" +# tidy-alphabetical-start build_helper = { path = "../build_helper" } clap = { version = "4.4", default-features = false, features = ["std", "usage", "help", "derive", "error-context"] } clap_complete = "4.4" @@ -50,12 +51,15 @@ serde = "1.0" serde_derive = "1.0" serde_json = "1.0" sha2 = "0.10" +shim_utils = { path = "../shim_utils" } tar = { version = "0.4.45", default-features = false } +# Used by `feature = "tracing"` and by tests, but is also a dependency of `shim_utils` anyway. +tempfile = "3.23" termcolor = "1.4" toml = "0.5" walkdir = "2.4" xz2 = "0.1" -tempfile = "3.15.0" +# tidy-alphabetical-end # Dependencies needed by the build-metrics feature sysinfo = { version = "0.39.2", default-features = false, optional = true, features = ["system"] } diff --git a/src/bootstrap/src/bin/rustc.rs b/src/bootstrap/src/bin/rustc.rs index c8aff10b6355b..d5e8162516f03 100644 --- a/src/bootstrap/src/bin/rustc.rs +++ b/src/bootstrap/src/bin/rustc.rs @@ -20,21 +20,11 @@ use std::path::{Path, PathBuf}; use std::process::{Child, Command}; use std::time::Instant; -use arg_file_command::ArgFileCommand; -use shared_helpers::{ - collect_args, dylib_path, dylib_path_var, exe, maybe_dump, parse_rustc_stage, - parse_rustc_verbose, parse_value_from_args, +use shim_utils::{ + ArgFileCommand, collect_args, dylib_path, dylib_path_var, exe, maybe_dump, parse_rustc_stage, + parse_rustc_verbose, parse_value_from_args, proc_macro_deps, }; -#[path = "../utils/shared_helpers.rs"] -mod shared_helpers; - -#[path = "../../../build_helper/src/arg_file_command.rs"] -mod arg_file_command; - -#[path = "../utils/proc_macro_deps.rs"] -mod proc_macro_deps; - fn main() { let orig_args = collect_args(); let mut args = orig_args.clone(); diff --git a/src/bootstrap/src/bin/rustdoc.rs b/src/bootstrap/src/bin/rustdoc.rs index fd048d138e090..eba1e9ef1c5cf 100644 --- a/src/bootstrap/src/bin/rustdoc.rs +++ b/src/bootstrap/src/bin/rustdoc.rs @@ -5,18 +5,11 @@ use std::env; use std::path::PathBuf; -use arg_file_command::ArgFileCommand; -use shared_helpers::{ - collect_args, dylib_path, dylib_path_var, maybe_dump, parse_rustc_stage, parse_rustc_verbose, - parse_value_from_args, +use shim_utils::{ + ArgFileCommand, collect_args, dylib_path, dylib_path_var, maybe_dump, parse_rustc_stage, + parse_rustc_verbose, parse_value_from_args, }; -#[path = "../utils/shared_helpers.rs"] -mod shared_helpers; - -#[path = "../../../build_helper/src/arg_file_command.rs"] -mod arg_file_command; - fn main() { let args = collect_args(); diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 394e14c80a1ef..f4a9b5704a434 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -12,11 +12,11 @@ use std::{env, fs, io, panic, str}; use build_helper::ci::CiEnv; use object::read::archive::ArchiveFile; +pub(crate) use shim_utils::{dylib_path, dylib_path_var}; use crate::core::builder::Builder; use crate::core::config::{Config, TargetSelection}; use crate::utils::exec::{BootstrapCommand, command}; -pub use crate::utils::shared_helpers::{dylib_path, dylib_path_var}; use crate::{BootstrapOverrideLld, StepStack}; #[cfg(test)] @@ -68,7 +68,7 @@ macro_rules! t { pub use t; pub fn exe(name: &str, target: TargetSelection) -> String { - crate::utils::shared_helpers::exe(name, &target.triple) + shim_utils::exe(name, &target.triple) } /// Returns the path to the split debug info for the specified file if it exists. diff --git a/src/bootstrap/src/utils/mod.rs b/src/bootstrap/src/utils/mod.rs index 97d8d274e8fb6..75012f819b445 100644 --- a/src/bootstrap/src/utils/mod.rs +++ b/src/bootstrap/src/utils/mod.rs @@ -11,7 +11,6 @@ pub(crate) mod exec; pub(crate) mod helpers; pub(crate) mod job; pub(crate) mod render_tests; -pub(crate) mod shared_helpers; pub(crate) mod tarball; pub(crate) mod tracing; diff --git a/src/bootstrap/src/utils/tests/mod.rs b/src/bootstrap/src/utils/tests/mod.rs index 94dfa33455d80..ee180fbf224a8 100644 --- a/src/bootstrap/src/utils/tests/mod.rs +++ b/src/bootstrap/src/utils/tests/mod.rs @@ -7,10 +7,7 @@ use tempfile::TempDir; use crate::{Config, Flags}; pub mod git; - -// Note: tests for `shared_helpers` is separate here, as otherwise shim binaries that include the -// `shared_helpers` via `#[path]` would fail to find it, breaking `./x check bootstrap`. -mod shared_helpers_tests; +mod shim_utils_tests; /// Holds temporary state of a bootstrap test. /// Right now it is only used to redirect the build directory of the bootstrap diff --git a/src/bootstrap/src/utils/tests/shared_helpers_tests.rs b/src/bootstrap/src/utils/tests/shim_utils_tests.rs similarity index 69% rename from src/bootstrap/src/utils/tests/shared_helpers_tests.rs rename to src/bootstrap/src/utils/tests/shim_utils_tests.rs index c486e65007e5b..77958a5470c60 100644 --- a/src/bootstrap/src/utils/tests/shared_helpers_tests.rs +++ b/src/bootstrap/src/utils/tests/shim_utils_tests.rs @@ -1,11 +1,6 @@ -//! The `shared_helpers` module can't have its own tests submodule, because that would cause -//! problems for the shim binaries that include it via `#[path]`, so instead those unit tests live -//! here. -//! -//! To prevent tidy from complaining about this file not being named `tests.rs`, it lives inside a -//! submodule directory named `tests`. +//! Code in the `shim_utils` crate can be tested here as part of `./x test bootstrap`. -use crate::utils::shared_helpers::parse_value_from_args; +use shim_utils::parse_value_from_args; #[test] fn test_parse_value_from_args() { diff --git a/src/shim_utils/Cargo.toml b/src/shim_utils/Cargo.toml new file mode 100644 index 0000000000000..c4ff615f36bbd --- /dev/null +++ b/src/shim_utils/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "shim_utils" +version = "0.0.0" +edition = "2024" + +# Every dependency of this crate becomes a transitive dependency of bootstrap +# (and some other tools), so try to avoid dependencies that are slow to build. +[dependencies] +# tidy-alphabetical-start +tempfile = "3.23.0" +# tidy-alphabetical-end diff --git a/src/shim_utils/README.md b/src/shim_utils/README.md new file mode 100644 index 0000000000000..b7589d9cdb0ce --- /dev/null +++ b/src/shim_utils/README.md @@ -0,0 +1,6 @@ +`shim_utils` is a small library crate containing shared code that is used by +bootstrap's `rustc` and `rustdoc` shims, and also by bootstrap itself or other +bootstrap tools. + +Using a dedicated crate should be faster than trying to link `build_helper` +or `bootstrap` into the shims. diff --git a/src/build_helper/src/arg_file_command.rs b/src/shim_utils/src/arg_file_command.rs similarity index 90% rename from src/build_helper/src/arg_file_command.rs rename to src/shim_utils/src/arg_file_command.rs index 1d74558f472ab..616ac9ecd8e74 100644 --- a/src/build_helper/src/arg_file_command.rs +++ b/src/shim_utils/src/arg_file_command.rs @@ -1,11 +1,3 @@ -//! This module is explictly not `mod`ed as it's shared across multiple crates -//! like bootstrap and compiletest via `#[path]` moduled declarations. -//! It's important to keep this file isolated from the rest of build_helper so it can be compiled -//! without build_helper. - -// Roughly match the `std::process::Command` API -#![allow(dead_code, unreachable_pub)] - use std::ffi::{OsStr, OsString}; use std::io::Write; use std::path::Path; diff --git a/src/shim_utils/src/lib.rs b/src/shim_utils/src/lib.rs new file mode 100644 index 0000000000000..ba32d0e2fa009 --- /dev/null +++ b/src/shim_utils/src/lib.rs @@ -0,0 +1,17 @@ +//! Utility code in this crate can be used by bootstrap's `rustc` and `rustdoc` +//! shims, while also being shared by bootstrap and other bootstrap tools. +//! +//! Try to keep this crate small, to avoid bloating bootstrap build times. +//! +//! Ideally, any code in this crate should be used by at least one of the shims, +//! and at least one other crate (possibly the other shim). + +mod arg_file_command; +pub mod proc_macro_deps; +mod shared_helpers; + +pub use crate::arg_file_command::ArgFileCommand; +pub use crate::shared_helpers::{ + collect_args, dylib_path, dylib_path_var, exe, maybe_dump, parse_rustc_stage, + parse_rustc_verbose, parse_value_from_args, +}; diff --git a/src/bootstrap/src/utils/proc_macro_deps.rs b/src/shim_utils/src/proc_macro_deps.rs similarity index 100% rename from src/bootstrap/src/utils/proc_macro_deps.rs rename to src/shim_utils/src/proc_macro_deps.rs diff --git a/src/bootstrap/src/utils/shared_helpers.rs b/src/shim_utils/src/shared_helpers.rs similarity index 88% rename from src/bootstrap/src/utils/shared_helpers.rs rename to src/shim_utils/src/shared_helpers.rs index 78af70ddb0f42..fad245a6b69c9 100644 --- a/src/bootstrap/src/utils/shared_helpers.rs +++ b/src/shim_utils/src/shared_helpers.rs @@ -1,18 +1,3 @@ -//! This module serves two purposes: -//! -//! 1. It is part of the `utils` module and used in other parts of bootstrap. -//! 2. It is embedded inside bootstrap shims to avoid a dependency on the bootstrap library. -//! Therefore, this module should never use any other bootstrap module. This reduces binary size -//! and improves compilation time by minimizing linking time. - -// # Note on tests -// -// If we were to declare a tests submodule here, the shim binaries that include this module via -// `#[path]` would fail to find it, which breaks `./x check bootstrap`. So instead the unit tests -// for this module are in `super::tests::shared_helpers_tests`. - -#![allow(dead_code)] - use std::env; use std::ffi::OsString; use std::fs::OpenOptions; diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index 000dca8abfca5..09c19be3dafd6 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -31,7 +31,7 @@ rustfix = "0.8.1" semver = { version = "1.0.23", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -tempfile = "3.23.0" +shim_utils = { path = "../../shim_utils" } tracing = "0.1" tracing-subscriber = { version = "0.3.3", default-features = false, features = ["ansi", "env-filter", "fmt", "parking_lot", "smallvec"] } unified-diff = "0.2.1" diff --git a/src/tools/compiletest/src/util.rs b/src/tools/compiletest/src/util.rs index bb47bfa2d848d..53c675e7d015e 100644 --- a/src/tools/compiletest/src/util.rs +++ b/src/tools/compiletest/src/util.rs @@ -2,15 +2,11 @@ use std::env; use std::process::Command; use camino::{Utf8Path, Utf8PathBuf}; +pub(crate) use shim_utils::ArgFileCommand; #[cfg(test)] mod tests; -#[path = "../../../build_helper/src/arg_file_command.rs"] -mod arg_file_command; - -pub(crate) use arg_file_command::ArgFileCommand; - pub(crate) fn make_new_path(path: &str) -> String { assert!(cfg!(windows)); // Windows just uses PATH as the library search path, so we have to diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml index 4b38f9479fbc6..eecd46fded588 100644 --- a/src/tools/tidy/Cargo.toml +++ b/src/tools/tidy/Cargo.toml @@ -14,6 +14,7 @@ walkdir = "2" ignore = "0.4.18" semver = "1.0" serde = { version = "1.0.125", features = ["derive"], optional = true } +shim_utils = { path = "../../shim_utils" } termcolor = "1.1.3" rustc-hash = "2.0.0" similar = "2.5.0" diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index aeaea8ca9bb5b..286a0448089e0 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -8,12 +8,10 @@ use std::path::Path; use cargo_metadata::semver::Version; use cargo_metadata::{Metadata, Package, PackageId}; +use shim_utils::proc_macro_deps; use crate::diagnostics::{RunningCheck, TidyCtx}; -#[path = "../../../bootstrap/src/utils/proc_macro_deps.rs"] -mod proc_macro_deps; - #[derive(Clone, Copy)] struct ListLocation { path: &'static str, @@ -704,6 +702,8 @@ pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) { /// Ensure the list of proc-macro crate transitive dependencies is up to date fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, check: &mut RunningCheck) { + const PROC_MACRO_DEPS_RS_PATH: &str = "src/shim_utils/src/proc_macro_deps.rs"; + if std::env::var("RUSTC").is_err() { panic!("tidy must be run under bootstrap (./x test tidy), not as a standalone command"); } @@ -732,7 +732,7 @@ fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, check: &mut if needs_blessing && bless { let mut proc_macro_deps: Vec<_> = proc_macro_deps.into_iter().collect(); proc_macro_deps.sort(); - let mut file = File::create(root.join("src/bootstrap/src/utils/proc_macro_deps.rs")) + let mut file = File::create(root.join(PROC_MACRO_DEPS_RS_PATH)) .expect("`proc_macro_deps` should exist"); writeln!( &mut file, @@ -758,13 +758,13 @@ pub static CRATES: &[&str] = &[ for missing in proc_macro_deps.difference(&expected) { error_found = true; check.error(format!( - "proc-macro crate dependency `{missing}` is not registered in `src/bootstrap/src/utils/proc_macro_deps.rs`", + "proc-macro crate dependency `{missing}` is not registered in `{PROC_MACRO_DEPS_RS_PATH}`", )); } for extra in expected.difference(&proc_macro_deps) { error_found = true; check.error(format!( - "`{extra}` is registered in `src/bootstrap/src/utils/proc_macro_deps.rs`, but is not a proc-macro crate dependency", + "`{extra}` is registered in `{PROC_MACRO_DEPS_RS_PATH}`, but is not a proc-macro crate dependency", )); } if error_found {