From f5892f210e529c4826a7fc40ee1aba2e20cc39fa Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 5 Apr 2024 13:21:46 -0500 Subject: [PATCH] perf(toml): Avoid inferring when targets are known We read the file system to infer two different data points - Implicit targets - Implicit `path` values for targets I took a shortcut for this case and recognize the scenario where we can bypass both and do so. I went with a bypass, rather than this being integrating into the inferring code because the inferring code is complex and I didn't want to add to it further in isolating the inferring to only when its needed. --- src/cargo/util/toml/targets.rs | 183 ++++++++++++++++++++------------- 1 file changed, 110 insertions(+), 73 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 9d22f4cc10a..261ff67b320 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -261,48 +261,60 @@ pub fn resolve_bins( errors: &mut Vec, has_lib: bool, ) -> CargoResult> { - let inferred = inferred_bins(package_root, package_name); + if is_resolved(toml_bins, autodiscover) { + let toml_bins = toml_bins.cloned().unwrap_or_default(); + for bin in &toml_bins { + validate_bin_name(bin, warnings)?; + validate_bin_crate_types(bin, edition, warnings, errors)?; + validate_bin_proc_macro(bin, edition, warnings, errors)?; + } + Ok(toml_bins) + } else { + let inferred = inferred_bins(package_root, package_name); - let mut bins = toml_targets_and_inferred( - toml_bins, - &inferred, - package_root, - autodiscover, - edition, - warnings, - "binary", - "bin", - "autobins", - ); + let mut bins = toml_targets_and_inferred( + toml_bins, + &inferred, + package_root, + autodiscover, + edition, + warnings, + "binary", + "bin", + "autobins", + ); - for bin in &mut bins { - // Check early to improve error messages - validate_bin_name(bin, warnings)?; + for bin in &mut bins { + // Check early to improve error messages + validate_bin_name(bin, warnings)?; - validate_bin_crate_types(bin, edition, warnings, errors)?; - validate_bin_proc_macro(bin, edition, warnings, errors)?; + validate_bin_crate_types(bin, edition, warnings, errors)?; + validate_bin_proc_macro(bin, edition, warnings, errors)?; - let path = target_path(bin, &inferred, "bin", package_root, edition, &mut |_| { - if let Some(legacy_path) = legacy_bin_path(package_root, name_or_panic(bin), has_lib) { - warnings.push(format!( - "path `{}` was erroneously implicitly accepted for binary `{}`,\n\ + let path = target_path(bin, &inferred, "bin", package_root, edition, &mut |_| { + if let Some(legacy_path) = + legacy_bin_path(package_root, name_or_panic(bin), has_lib) + { + warnings.push(format!( + "path `{}` was erroneously implicitly accepted for binary `{}`,\n\ please set bin.path in Cargo.toml", - legacy_path.display(), - name_or_panic(bin) - )); - Some(legacy_path) - } else { - None - } - }); - let path = match path { - Ok(path) => path, - Err(e) => anyhow::bail!("{}", e), - }; - bin.path = Some(PathValue(path)); - } + legacy_path.display(), + name_or_panic(bin) + )); + Some(legacy_path) + } else { + None + } + }); + let path = match path { + Ok(path) => path, + Err(e) => anyhow::bail!("{}", e), + }; + bin.path = Some(PathValue(path)); + } - Ok(bins) + Ok(bins) + } } #[tracing::instrument(skip_all)] @@ -536,6 +548,19 @@ fn to_bench_targets( Ok(result) } +fn is_resolved(toml_targets: Option<&Vec>, autodiscover: Option) -> bool { + if autodiscover != Some(false) { + return false; + } + + let Some(toml_targets) = toml_targets else { + return true; + }; + toml_targets + .iter() + .all(|t| t.name.is_some() && t.path.is_some()) +} + fn resolve_targets( target_kind_human: &str, target_kind: &str, @@ -576,48 +601,60 @@ fn resolve_targets_with_legacy_path( legacy_path: &mut dyn FnMut(&TomlTarget) -> Option, autodiscover_flag_name: &str, ) -> CargoResult> { - let inferred = inferred(); - let toml_targets = toml_targets_and_inferred( - toml_targets, - &inferred, - package_root, - autodiscover, - edition, - warnings, - target_kind_human, - target_kind, - autodiscover_flag_name, - ); - - for target in &toml_targets { - // Check early to improve error messages - validate_target_name(target, target_kind_human, target_kind, warnings)?; - - validate_proc_macro(target, target_kind_human, edition, warnings)?; - validate_crate_types(target, target_kind_human, edition, warnings)?; - } - - let mut result = Vec::new(); - for mut target in toml_targets { - let path = target_path( - &target, + if is_resolved(toml_targets, autodiscover) { + let toml_targets = toml_targets.cloned().unwrap_or_default(); + for target in &toml_targets { + // Check early to improve error messages + validate_target_name(target, target_kind_human, target_kind, warnings)?; + + validate_proc_macro(target, target_kind_human, edition, warnings)?; + validate_crate_types(target, target_kind_human, edition, warnings)?; + } + Ok(toml_targets) + } else { + let inferred = inferred(); + let toml_targets = toml_targets_and_inferred( + toml_targets, &inferred, - target_kind, package_root, + autodiscover, edition, - legacy_path, + warnings, + target_kind_human, + target_kind, + autodiscover_flag_name, ); - let path = match path { - Ok(path) => path, - Err(e) => { - errors.push(e); - continue; - } - }; - target.path = Some(PathValue(path)); - result.push(target); + + for target in &toml_targets { + // Check early to improve error messages + validate_target_name(target, target_kind_human, target_kind, warnings)?; + + validate_proc_macro(target, target_kind_human, edition, warnings)?; + validate_crate_types(target, target_kind_human, edition, warnings)?; + } + + let mut result = Vec::new(); + for mut target in toml_targets { + let path = target_path( + &target, + &inferred, + target_kind, + package_root, + edition, + legacy_path, + ); + let path = match path { + Ok(path) => path, + Err(e) => { + errors.push(e); + continue; + } + }; + target.path = Some(PathValue(path)); + result.push(target); + } + Ok(result) } - Ok(result) } fn inferred_lib(package_root: &Path) -> Option {