From 3faae370b5cd40c435d0ee4c58686d162cb3e96c Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Mon, 30 Dec 2024 16:32:14 -0500 Subject: [PATCH 1/3] Pin our tracing-core dep due to binary size concerns. --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 5eb70f1718..ef8b7de212 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -499,6 +499,7 @@ thiserror = { version = "2", default-features = false } time = "0.3" toml_edit = "0.21" tracing = "0.1" +tracing-core = "=0.1.30" # pinned due to large binary size increase (~400kb) in later versions tracing-subscriber = "0.3.16" typed-path = "0.7" uefi = "0.32" From 872bc361ba1fa805d16f0aa54fa21172137ed5af Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Thu, 2 Jan 2025 14:34:57 -0500 Subject: [PATCH 2/3] feedback --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ef8b7de212..4bdf3b7a6b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -499,7 +499,7 @@ thiserror = { version = "2", default-features = false } time = "0.3" toml_edit = "0.21" tracing = "0.1" -tracing-core = "=0.1.30" # pinned due to large binary size increase (~400kb) in later versions +tracing-core = "=0.1.30" # Pin to avoid binary size increase https://github.com/tokio-rs/tracing/issues/3182 tracing-subscriber = "0.3.16" typed-path = "0.7" uefi = "0.32" From cb5337cdc350ee831b59c5c0cedd85943b67ca55 Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Thu, 2 Jan 2025 15:14:40 -0500 Subject: [PATCH 3/3] Add support for ignoring unused deps at the workspace level --- Cargo.toml | 4 ++ xtask/src/tasks/fmt/unused_deps.rs | 88 +++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4bdf3b7a6b..ad5f200c22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -522,6 +522,10 @@ xshell-macros = "0.2" # We add the derive feature here since the vast majority of our crates use it. zerocopy = { version = "0.7.32", features = ["derive"] } +[workspace.metadata.xtask.unused-deps] +# Pulled in through "tracing", but we need to pin the version +ignored = ["tracing-core"] + [profile.release] panic = 'abort' debug = true diff --git a/xtask/src/tasks/fmt/unused_deps.rs b/xtask/src/tasks/fmt/unused_deps.rs index df0145775a..e57046ebd4 100644 --- a/xtask/src/tasks/fmt/unused_deps.rs +++ b/xtask/src/tasks/fmt/unused_deps.rs @@ -107,6 +107,7 @@ impl Xtask for UnusedDeps { results.sort_by(|a, b| a.1.cmp(b.1)); let mut workspace = analyze_workspace(&ctx.root)?; + let full_deps = workspace.deps.clone(); // Display all the results. @@ -137,20 +138,32 @@ impl Xtask for UnusedDeps { workspace.deps.retain(|x| !analysis.deps.contains(x)); } - if !workspace.deps.is_empty() { - workspace.deps.sort(); + workspace.deps.sort(); + workspace.ignored.sort(); + if workspace.deps != workspace.ignored { found_something = true; + let mut unused_deps = Vec::new(); + println!("Workspace -- {}:", workspace.path.display()); for dep in &workspace.deps { - println!("\t{} is unused", dep); + if !workspace.ignored.contains(dep) { + println!("\t{} is unused", dep); + unused_deps.push(DepResult::Unused(dep.clone())); + } + } + for ign in &workspace.ignored { + if !workspace.deps.contains(ign) { + if full_deps.contains(ign) { + println!("\t{} is ignored, but being used", ign); + unused_deps.push(DepResult::IgnoredButUsed(ign.clone())); + } else { + println!("\t{} is ignored, but it's not even being depended on", ign); + unused_deps.push(DepResult::IgnoredAndMissing(ign.clone())); + } + } } if self.fix { - let unused_deps = workspace - .deps - .into_iter() - .map(DepResult::Unused) - .collect::>(); let fixed = remove_dependencies(&fs_err::read_to_string(&workspace.path)?, &unused_deps)?; fs_err::write(&workspace.path, fixed).context("Cargo.toml write error")?; @@ -212,11 +225,38 @@ fn remove_dependencies(manifest: &str, analysis_results: &[DepResult]) -> anyhow } } "workspace" => { - if let Some(t) = v - .get_mut("dependencies") - .and_then(|t| t.as_table_like_mut()) - { - dep_tables.push(t); + for (k2, v2) in v.iter_mut() { + let v2 = match v2 { + v2 if v2.is_table_like() => v2.as_table_like_mut().unwrap(), + _ => continue, + }; + + match k2.get() { + "dependencies" => dep_tables.push(v2), + "metadata" => { + // get_mut() seems to create a new table that wasn't previously + // there in some cases, so first check with the immutable + // accessors. + if v2 + .get("xtask") + .and_then(|x| x.get("unused-deps")) + .and_then(|u| u.get("ignored")) + .is_some() + { + ignored_array = v2 + .get_mut("metadata") + .unwrap() + .get_mut("xtask") + .unwrap() + .get_mut("unused-deps") + .unwrap() + .get_mut("ignored") + .unwrap() + .as_array_mut(); + } + } + _ => {} + } } } "package" => { @@ -337,6 +377,7 @@ enum DepResult { struct WorkspaceAnalysis { pub path: PathBuf, pub deps: Vec, + pub ignored: Vec, } fn make_regexp(name: &str) -> String { @@ -467,15 +508,22 @@ impl Search { fn analyze_workspace(root: &Path) -> anyhow::Result { let path = root.join("Cargo.toml"); let manifest = Manifest::from_path_with_metadata(&path)?; - - let deps = manifest + let workspace = manifest .workspace - .expect("workspace manifest must have a workspace section") - .dependencies - .into_keys() - .collect(); + .expect("workspace manifest must have a workspace section"); + + let deps = workspace.dependencies.into_keys().collect(); - Ok(WorkspaceAnalysis { deps, path }) + let ignored = workspace + .metadata + .and_then(|meta| meta.xtask.and_then(|x| x.unused_deps.map(|u| u.ignored))) + .unwrap_or_default(); + + Ok(WorkspaceAnalysis { + deps, + path, + ignored, + }) } fn analyze_crate(manifest_path: &Path) -> anyhow::Result> {