From cb819729d0d2d3f6f74b05168e17c34ca7f626b0 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Wed, 10 Sep 2025 16:56:36 +0100 Subject: [PATCH 1/5] tidy: Introduce `WorkspaceInfo` struct for deps information --- src/tools/tidy/src/deps.rs | 97 ++++++++++++++++++++++++----------- src/tools/tidy/src/extdeps.rs | 12 +++-- 2 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 60347b2ea6456..08c0d3ca78653 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -48,39 +48,74 @@ const LICENSES: &[&str] = &[ type ExceptionList = &'static [(&'static str, &'static str)]; +pub(crate) struct WorkspaceInfo<'a> { + /// Path to the directory containing the workspace root Cargo.toml file. + pub(crate) path: &'a str, + /// The list of license exceptions. + pub(crate) exceptions: ExceptionList, + /// Optionally: + /// * A list of crates for which dependencies need to be explicitly allowed. + /// * The list of allowed dependencies. + pub(crate) crates_and_deps: Option<(&'a [&'a str], &'a [&'a str])>, + /// Submodules required for the workspace + pub(crate) submodules: &'a [&'a str], +} + +impl<'a> WorkspaceInfo<'a> { + const fn new( + path: &'a str, + exceptions: ExceptionList, + crates_and_deps: Option<(&'a [&str], &'a [&str])>, + submodules: &'a [&str], + ) -> Self { + Self { path, exceptions, crates_and_deps, submodules } + } +} + /// The workspaces to check for licensing and optionally permitted dependencies. -/// -/// Each entry consists of a tuple with the following elements: -/// -/// * The path to the workspace root Cargo.toml file. -/// * The list of license exceptions. -/// * Optionally a tuple of: -/// * A list of crates for which dependencies need to be explicitly allowed. -/// * The list of allowed dependencies. -/// * Submodules required for the workspace. // FIXME auto detect all cargo workspaces -pub(crate) const WORKSPACES: &[(&str, ExceptionList, Option<(&[&str], &[&str])>, &[&str])] = &[ +pub(crate) const WORKSPACES: &[WorkspaceInfo<'static>] = &[ // The root workspace has to be first for check_rustfix to work. - (".", EXCEPTIONS, Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES)), &[]), - ("library", EXCEPTIONS_STDLIB, Some((&["sysroot"], PERMITTED_STDLIB_DEPENDENCIES)), &[]), + WorkspaceInfo::new(".", EXCEPTIONS, Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES)), &[]), + WorkspaceInfo::new( + "library", + EXCEPTIONS_STDLIB, + Some((&["sysroot"], PERMITTED_STDLIB_DEPENDENCIES)), + &[], + ), // Outside of the alphabetical section because rustfmt formats it using multiple lines. - ( + WorkspaceInfo::new( "compiler/rustc_codegen_cranelift", EXCEPTIONS_CRANELIFT, Some((&["rustc_codegen_cranelift"], PERMITTED_CRANELIFT_DEPENDENCIES)), &[], ), // tidy-alphabetical-start - ("compiler/rustc_codegen_gcc", EXCEPTIONS_GCC, None, &[]), - ("src/bootstrap", EXCEPTIONS_BOOTSTRAP, None, &[]), - ("src/tools/cargo", EXCEPTIONS_CARGO, None, &["src/tools/cargo"]), - //("src/tools/miri/test-cargo-miri", &[], None), // FIXME uncomment once all deps are vendored - //("src/tools/miri/test_dependencies", &[], None), // FIXME uncomment once all deps are vendored - ("src/tools/rust-analyzer", EXCEPTIONS_RUST_ANALYZER, None, &[]), - ("src/tools/rustbook", EXCEPTIONS_RUSTBOOK, None, &["src/doc/book", "src/doc/reference"]), - ("src/tools/rustc-perf", EXCEPTIONS_RUSTC_PERF, None, &["src/tools/rustc-perf"]), - ("src/tools/test-float-parse", EXCEPTIONS, None, &[]), - ("tests/run-make-cargo/uefi-qemu/uefi_qemu_test", EXCEPTIONS_UEFI_QEMU_TEST, None, &[]), + WorkspaceInfo::new("compiler/rustc_codegen_gcc", EXCEPTIONS_GCC, None, &[]), + WorkspaceInfo::new("src/bootstrap", EXCEPTIONS_BOOTSTRAP, None, &[]), + WorkspaceInfo::new("src/tools/cargo", EXCEPTIONS_CARGO, None, &["src/tools/cargo"]), + //WorkspaceInfo::new("src/tools/miri/test-cargo-miri", &[], None), // FIXME uncomment once all deps are vendored + //WorkspaceInfo::new("src/tools/miri/test_dependencies", &[], None), // FIXME uncomment once all deps are vendored + WorkspaceInfo::new("src/tools/rust-analyzer", EXCEPTIONS_RUST_ANALYZER, None, &[]), + WorkspaceInfo::new( + "src/tools/rustbook", + EXCEPTIONS_RUSTBOOK, + None, + &["src/doc/book", "src/doc/reference"], + ), + WorkspaceInfo::new( + "src/tools/rustc-perf", + EXCEPTIONS_RUSTC_PERF, + None, + &["src/tools/rustc-perf"], + ), + WorkspaceInfo::new("src/tools/test-float-parse", EXCEPTIONS, None, &[]), + WorkspaceInfo::new( + "tests/run-make-cargo/uefi-qemu/uefi_qemu_test", + EXCEPTIONS_UEFI_QEMU_TEST, + None, + &[], + ), // tidy-alphabetical-end ]; @@ -567,29 +602,29 @@ pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) { check_proc_macro_dep_list(root, cargo, bless, bad); - for &(workspace, exceptions, permitted_deps, submodules) in WORKSPACES { + for &WorkspaceInfo { path, exceptions, crates_and_deps, submodules } in WORKSPACES { if has_missing_submodule(root, submodules) { continue; } - if !root.join(workspace).join("Cargo.lock").exists() { - tidy_error!(bad, "the `{workspace}` workspace doesn't have a Cargo.lock"); + if !root.join(path).join("Cargo.lock").exists() { + tidy_error!(bad, "the `{path}` workspace doesn't have a Cargo.lock"); continue; } let mut cmd = cargo_metadata::MetadataCommand::new(); cmd.cargo_path(cargo) - .manifest_path(root.join(workspace).join("Cargo.toml")) + .manifest_path(root.join(path).join("Cargo.toml")) .features(cargo_metadata::CargoOpt::AllFeatures) .other_options(vec!["--locked".to_owned()]); let metadata = t!(cmd.exec()); - check_license_exceptions(&metadata, workspace, exceptions, bad); - if let Some((crates, permitted_deps)) = permitted_deps { - check_permitted_dependencies(&metadata, workspace, permitted_deps, crates, bad); + check_license_exceptions(&metadata, path, exceptions, bad); + if let Some((crates, permitted_deps)) = crates_and_deps { + check_permitted_dependencies(&metadata, path, permitted_deps, crates, bad); } - if workspace == "library" { + if path == "library" { check_runtime_license_exceptions(&metadata, bad); check_runtime_no_duplicate_dependencies(&metadata, bad); check_runtime_no_proc_macros(&metadata, bad); diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs index bc217a55cc199..2b212cfa67a4b 100644 --- a/src/tools/tidy/src/extdeps.rs +++ b/src/tools/tidy/src/extdeps.rs @@ -3,6 +3,8 @@ use std::fs; use std::path::Path; +use crate::deps::WorkspaceInfo; + /// List of allowed sources for packages. const ALLOWED_SOURCES: &[&str] = &[ r#""registry+https://github.com/rust-lang/crates.io-index""#, @@ -13,22 +15,22 @@ const ALLOWED_SOURCES: &[&str] = &[ /// Checks for external package sources. `root` is the path to the directory that contains the /// workspace `Cargo.toml`. pub fn check(root: &Path, bad: &mut bool) { - for &(workspace, _, _, submodules) in crate::deps::WORKSPACES { + for &WorkspaceInfo { path, submodules, .. } in crate::deps::WORKSPACES { if crate::deps::has_missing_submodule(root, submodules) { continue; } // FIXME check other workspaces too // `Cargo.lock` of rust. - let path = root.join(workspace).join("Cargo.lock"); + let lockfile = root.join(path).join("Cargo.lock"); - if !path.exists() { - tidy_error!(bad, "the `{workspace}` workspace doesn't have a Cargo.lock"); + if !lockfile.exists() { + tidy_error!(bad, "the `{path}` workspace doesn't have a Cargo.lock"); continue; } // Open and read the whole file. - let cargo_lock = t!(fs::read_to_string(&path)); + let cargo_lock = t!(fs::read_to_string(&lockfile)); // Process each line. for line in cargo_lock.lines() { From ba874d38a561ec59d0f8e6fdde3d830e84f6da98 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Wed, 10 Sep 2025 17:07:28 +0100 Subject: [PATCH 2/5] tidy: Print crate name on dependency error --- src/tools/tidy/src/deps.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 08c0d3ca78653..184f52a247f86 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -621,7 +621,8 @@ pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) { check_license_exceptions(&metadata, path, exceptions, bad); if let Some((crates, permitted_deps)) = crates_and_deps { - check_permitted_dependencies(&metadata, path, permitted_deps, crates, bad); + let descr = crates.get(0).unwrap_or(&path); + check_permitted_dependencies(&metadata, descr, permitted_deps, crates, bad); } if path == "library" { From adfb6ec089fbb41519acd530cbedd3477e84399c Mon Sep 17 00:00:00 2001 From: clubby789 Date: Wed, 10 Sep 2025 17:13:21 +0100 Subject: [PATCH 3/5] tidy: More accurate permitted dependencies location --- src/tools/tidy/src/deps.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 184f52a247f86..9acd445f3e238 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -261,7 +261,19 @@ const EXCEPTIONS_UEFI_QEMU_TEST: ExceptionList = &[ ("r-efi", "MIT OR Apache-2.0 OR LGPL-2.1-or-later"), // LGPL is not acceptable, but we use it under MIT OR Apache-2.0 ]; -const PERMITTED_DEPS_LOCATION: &str = concat!(file!(), ":", line!()); +struct ListLocation { + path: &'static str, + line: u32, +} + +/// Creates a [`ListLocation`] for the current location (with an additional offset to the actual list start); +macro_rules! location { + (+ $offset:literal) => { + ListLocation { path: file!(), line: line!() + $offset } + }; +} + +const PERMITTED_RUSTC_DEPS_LOCATION: ListLocation = location!(+6); /// Crates rustc is allowed to depend on. Avoid adding to the list if possible. /// @@ -930,7 +942,8 @@ fn check_permitted_dependencies( } if has_permitted_dep_error { - eprintln!("Go to `{PERMITTED_DEPS_LOCATION}` for the list."); + let ListLocation { path, line } = PERMITTED_RUSTC_DEPS_LOCATION; + eprintln!("Go to `{path}:{line}` for the list."); } } From 11a22b30904fb6e0fd41d9004b3a88e985ec8455 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Wed, 10 Sep 2025 17:19:51 +0100 Subject: [PATCH 4/5] tidy: Add specific line info for allowed dependencies --- src/tools/tidy/src/deps.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 9acd445f3e238..290f0ea313419 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -48,6 +48,7 @@ const LICENSES: &[&str] = &[ type ExceptionList = &'static [(&'static str, &'static str)]; +#[derive(Clone, Copy)] pub(crate) struct WorkspaceInfo<'a> { /// Path to the directory containing the workspace root Cargo.toml file. pub(crate) path: &'a str, @@ -56,7 +57,8 @@ pub(crate) struct WorkspaceInfo<'a> { /// Optionally: /// * A list of crates for which dependencies need to be explicitly allowed. /// * The list of allowed dependencies. - pub(crate) crates_and_deps: Option<(&'a [&'a str], &'a [&'a str])>, + /// * The source code location of the allowed dependencies list + crates_and_deps: Option<(&'a [&'a str], &'a [&'a str], ListLocation)>, /// Submodules required for the workspace pub(crate) submodules: &'a [&'a str], } @@ -65,7 +67,7 @@ impl<'a> WorkspaceInfo<'a> { const fn new( path: &'a str, exceptions: ExceptionList, - crates_and_deps: Option<(&'a [&str], &'a [&str])>, + crates_and_deps: Option<(&'a [&str], &'a [&str], ListLocation)>, submodules: &'a [&str], ) -> Self { Self { path, exceptions, crates_and_deps, submodules } @@ -76,18 +78,27 @@ impl<'a> WorkspaceInfo<'a> { // FIXME auto detect all cargo workspaces pub(crate) const WORKSPACES: &[WorkspaceInfo<'static>] = &[ // The root workspace has to be first for check_rustfix to work. - WorkspaceInfo::new(".", EXCEPTIONS, Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES)), &[]), + WorkspaceInfo::new( + ".", + EXCEPTIONS, + Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES, PERMITTED_RUSTC_DEPS_LOCATION)), + &[], + ), WorkspaceInfo::new( "library", EXCEPTIONS_STDLIB, - Some((&["sysroot"], PERMITTED_STDLIB_DEPENDENCIES)), + Some((&["sysroot"], PERMITTED_STDLIB_DEPENDENCIES, PERMITTED_STDLIB_DEPS_LOCATION)), &[], ), // Outside of the alphabetical section because rustfmt formats it using multiple lines. WorkspaceInfo::new( "compiler/rustc_codegen_cranelift", EXCEPTIONS_CRANELIFT, - Some((&["rustc_codegen_cranelift"], PERMITTED_CRANELIFT_DEPENDENCIES)), + Some(( + &["rustc_codegen_cranelift"], + PERMITTED_CRANELIFT_DEPENDENCIES, + PERMITTED_CRANELIFT_DEPS_LOCATION, + )), &[], ), // tidy-alphabetical-start @@ -261,6 +272,7 @@ const EXCEPTIONS_UEFI_QEMU_TEST: ExceptionList = &[ ("r-efi", "MIT OR Apache-2.0 OR LGPL-2.1-or-later"), // LGPL is not acceptable, but we use it under MIT OR Apache-2.0 ]; +#[derive(Clone, Copy)] struct ListLocation { path: &'static str, line: u32, @@ -499,6 +511,8 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ // tidy-alphabetical-end ]; +const PERMITTED_STDLIB_DEPS_LOCATION: ListLocation = location!(+2); + const PERMITTED_STDLIB_DEPENDENCIES: &[&str] = &[ // tidy-alphabetical-start "addr2line", @@ -540,6 +554,8 @@ const PERMITTED_STDLIB_DEPENDENCIES: &[&str] = &[ // tidy-alphabetical-end ]; +const PERMITTED_CRANELIFT_DEPS_LOCATION: ListLocation = location!(+2); + const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[ // tidy-alphabetical-start "allocator-api2", @@ -632,9 +648,9 @@ pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) { let metadata = t!(cmd.exec()); check_license_exceptions(&metadata, path, exceptions, bad); - if let Some((crates, permitted_deps)) = crates_and_deps { + if let Some((crates, permitted_deps, location)) = crates_and_deps { let descr = crates.get(0).unwrap_or(&path); - check_permitted_dependencies(&metadata, descr, permitted_deps, crates, bad); + check_permitted_dependencies(&metadata, descr, permitted_deps, crates, location, bad); } if path == "library" { @@ -882,6 +898,7 @@ fn check_permitted_dependencies( descr: &str, permitted_dependencies: &[&'static str], restricted_dependency_crates: &[&'static str], + permitted_location: ListLocation, bad: &mut bool, ) { let mut has_permitted_dep_error = false; @@ -942,8 +959,7 @@ fn check_permitted_dependencies( } if has_permitted_dep_error { - let ListLocation { path, line } = PERMITTED_RUSTC_DEPS_LOCATION; - eprintln!("Go to `{path}:{line}` for the list."); + eprintln!("Go to `{}:{}` for the list.", permitted_location.path, permitted_location.line); } } From 37d058fa048acd6bf385677f7cdb1e020ceb0769 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Sun, 14 Sep 2025 23:03:56 +0100 Subject: [PATCH 5/5] tidy: Remove `WorkspaceInfo` constructor --- src/tools/tidy/src/deps.rs | 150 ++++++++++++++++++++++--------------- 1 file changed, 90 insertions(+), 60 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 290f0ea313419..8092a8000cf58 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -63,71 +63,101 @@ pub(crate) struct WorkspaceInfo<'a> { pub(crate) submodules: &'a [&'a str], } -impl<'a> WorkspaceInfo<'a> { - const fn new( - path: &'a str, - exceptions: ExceptionList, - crates_and_deps: Option<(&'a [&str], &'a [&str], ListLocation)>, - submodules: &'a [&str], - ) -> Self { - Self { path, exceptions, crates_and_deps, submodules } - } -} - /// The workspaces to check for licensing and optionally permitted dependencies. // FIXME auto detect all cargo workspaces pub(crate) const WORKSPACES: &[WorkspaceInfo<'static>] = &[ // The root workspace has to be first for check_rustfix to work. - WorkspaceInfo::new( - ".", - EXCEPTIONS, - Some((&["rustc-main"], PERMITTED_RUSTC_DEPENDENCIES, PERMITTED_RUSTC_DEPS_LOCATION)), - &[], - ), - WorkspaceInfo::new( - "library", - EXCEPTIONS_STDLIB, - Some((&["sysroot"], PERMITTED_STDLIB_DEPENDENCIES, PERMITTED_STDLIB_DEPS_LOCATION)), - &[], - ), - // Outside of the alphabetical section because rustfmt formats it using multiple lines. - WorkspaceInfo::new( - "compiler/rustc_codegen_cranelift", - EXCEPTIONS_CRANELIFT, - Some(( - &["rustc_codegen_cranelift"], - PERMITTED_CRANELIFT_DEPENDENCIES, - PERMITTED_CRANELIFT_DEPS_LOCATION, + WorkspaceInfo { + path: ".", + exceptions: EXCEPTIONS, + crates_and_deps: Some(( + &["rustc-main"], + PERMITTED_RUSTC_DEPENDENCIES, + PERMITTED_RUSTC_DEPS_LOCATION, )), - &[], - ), - // tidy-alphabetical-start - WorkspaceInfo::new("compiler/rustc_codegen_gcc", EXCEPTIONS_GCC, None, &[]), - WorkspaceInfo::new("src/bootstrap", EXCEPTIONS_BOOTSTRAP, None, &[]), - WorkspaceInfo::new("src/tools/cargo", EXCEPTIONS_CARGO, None, &["src/tools/cargo"]), - //WorkspaceInfo::new("src/tools/miri/test-cargo-miri", &[], None), // FIXME uncomment once all deps are vendored - //WorkspaceInfo::new("src/tools/miri/test_dependencies", &[], None), // FIXME uncomment once all deps are vendored - WorkspaceInfo::new("src/tools/rust-analyzer", EXCEPTIONS_RUST_ANALYZER, None, &[]), - WorkspaceInfo::new( - "src/tools/rustbook", - EXCEPTIONS_RUSTBOOK, - None, - &["src/doc/book", "src/doc/reference"], - ), - WorkspaceInfo::new( - "src/tools/rustc-perf", - EXCEPTIONS_RUSTC_PERF, - None, - &["src/tools/rustc-perf"], - ), - WorkspaceInfo::new("src/tools/test-float-parse", EXCEPTIONS, None, &[]), - WorkspaceInfo::new( - "tests/run-make-cargo/uefi-qemu/uefi_qemu_test", - EXCEPTIONS_UEFI_QEMU_TEST, - None, - &[], - ), - // tidy-alphabetical-end + submodules: &[], + }, + WorkspaceInfo { + path: "library", + exceptions: EXCEPTIONS_STDLIB, + crates_and_deps: Some(( + &["sysroot"], + PERMITTED_STDLIB_DEPENDENCIES, + PERMITTED_STDLIB_DEPS_LOCATION, + )), + submodules: &[], + }, + { + WorkspaceInfo { + path: "compiler/rustc_codegen_cranelift", + exceptions: EXCEPTIONS_CRANELIFT, + crates_and_deps: Some(( + &["rustc_codegen_cranelift"], + PERMITTED_CRANELIFT_DEPENDENCIES, + PERMITTED_CRANELIFT_DEPS_LOCATION, + )), + submodules: &[], + } + }, + WorkspaceInfo { + path: "compiler/rustc_codegen_gcc", + exceptions: EXCEPTIONS_GCC, + crates_and_deps: None, + submodules: &[], + }, + WorkspaceInfo { + path: "src/bootstrap", + exceptions: EXCEPTIONS_BOOTSTRAP, + crates_and_deps: None, + submodules: &[], + }, + WorkspaceInfo { + path: "src/tools/cargo", + exceptions: EXCEPTIONS_CARGO, + crates_and_deps: None, + submodules: &["src/tools/cargo"], + }, + // FIXME uncomment once all deps are vendored + // WorkspaceInfo { + // path: "src/tools/miri/test-cargo-miri", + // crates_and_deps: None + // submodules: &[], + // }, + // WorkspaceInfo { + // path: "src/tools/miri/test_dependencies", + // crates_and_deps: None, + // submodules: &[], + // } + WorkspaceInfo { + path: "src/tools/rust-analyzer", + exceptions: EXCEPTIONS_RUST_ANALYZER, + crates_and_deps: None, + submodules: &[], + }, + WorkspaceInfo { + path: "src/tools/rustbook", + exceptions: EXCEPTIONS_RUSTBOOK, + crates_and_deps: None, + submodules: &["src/doc/book", "src/doc/reference"], + }, + WorkspaceInfo { + path: "src/tools/rustc-perf", + exceptions: EXCEPTIONS_RUSTC_PERF, + crates_and_deps: None, + submodules: &["src/tools/rustc-perf"], + }, + WorkspaceInfo { + path: "src/tools/test-float-parse", + exceptions: EXCEPTIONS, + crates_and_deps: None, + submodules: &[], + }, + WorkspaceInfo { + path: "tests/run-make-cargo/uefi-qemu/uefi_qemu_test", + exceptions: EXCEPTIONS_UEFI_QEMU_TEST, + crates_and_deps: None, + submodules: &[], + }, ]; /// These are exceptions to Rust's permissive licensing policy, and