From 71e362fb177031f47673fd37ac35dd758dd5af15 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 3 Apr 2024 11:21:42 -0500 Subject: [PATCH 01/23] test(package): Clean up a build.rs test --- tests/testsuite/package.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 8f3ab93ade0..860d6962772 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3479,24 +3479,25 @@ fn build_script_outside_pkg_root() { ) .file("src/main.rs", "fn main() {}") .build(); - let mut expect_msg = String::from("\ + // custom_build.rs does not exist + p.cargo("package -l") + .with_status(101) + .with_stderr("\ warning: manifest has no documentation, homepage or repository. See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. error: the source file of build script doesn't appear to exist. This may cause issue during packaging, as modules resolution and resources included via macros are often relative to the path of source files. Please update the `build` setting in the manifest at `[CWD]/Cargo.toml` and point to a path inside the root of the package. -"); - // custom_build.rs does not exist - p.cargo("package -l") - .with_status(101) - .with_stderr(&expect_msg) +") .run(); // custom_build.rs outside the package root let custom_build_root = paths::root().join("t_custom_build"); _ = fs::create_dir(&custom_build_root).unwrap(); _ = fs::write(&custom_build_root.join("custom_build.rs"), "fn main() {}"); - expect_msg = format!( + p.cargo("package -l") + .with_status(101) + .with_stderr(&format!( "\ warning: manifest has no documentation, homepage or repository. See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. @@ -3504,10 +3505,7 @@ error: the source file of build script doesn't appear to be a path inside of the It is at `{}/t_custom_build/custom_build.rs`, whereas the root the package is `[CWD]`. This may cause issue during packaging, as modules resolution and resources included via macros are often relative to the path of source files. Please update the `build` setting in the manifest at `[CWD]/Cargo.toml` and point to a path inside the root of the package. -", paths::root().display()); - p.cargo("package -l") - .with_status(101) - .with_stderr(&expect_msg) +", paths::root().display())) .run(); } From cc9710e9575429bed38bfccedd607bbee0055187 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 3 Apr 2024 13:07:52 -0500 Subject: [PATCH 02/23] refactor(toml): Be consistent in readme resolve name --- src/cargo/util/toml/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 954f594a0d4..d97c07d861b 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -476,7 +476,7 @@ fn resolve_package_toml<'a>( .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) .transpose()? .map(manifest::InheritableField::Value), - readme: readme_for_package( + readme: resolve_package_readme( package_root, original_package .readme @@ -530,7 +530,7 @@ fn resolve_package_toml<'a>( } /// Returns the name of the README file for a [`manifest::TomlPackage`]. -fn readme_for_package( +fn resolve_package_readme( package_root: &Path, readme: Option<&manifest::StringOrBool>, ) -> Option { @@ -764,7 +764,7 @@ impl InheritableFields { /// Gets the field `workspace.package.readme`. fn readme(&self, package_root: &Path) -> CargoResult { - let Some(readme) = readme_for_package( + let Some(readme) = resolve_package_readme( self._ws_root.as_path(), self.package.as_ref().and_then(|p| p.readme.as_ref()), ) else { From d3cfa7fc75f871bed31b1333f3c9bb585f59bbb2 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 3 Apr 2024 20:48:56 -0500 Subject: [PATCH 03/23] refactor(toml): Clarify which manifest is being used --- src/cargo/util/toml/targets.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 1d956e2bb9e..40f52b283b8 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -35,7 +35,7 @@ const DEFAULT_BIN_DIR_NAME: &'static str = "bin"; #[tracing::instrument(skip_all)] pub(super) fn targets( features: &Features, - manifest: &TomlManifest, + resolved_toml: &TomlManifest, package_name: &str, package_root: &Path, edition: Edition, @@ -49,7 +49,7 @@ pub(super) fn targets( let has_lib; if let Some(target) = clean_lib( - manifest.lib.as_ref(), + resolved_toml.lib.as_ref(), package_root, package_name, edition, @@ -61,15 +61,14 @@ pub(super) fn targets( has_lib = false; } - let package = manifest + let package = resolved_toml .package .as_ref() - .or_else(|| manifest.project.as_ref()) .ok_or_else(|| anyhow::format_err!("manifest has no `package` (or `project`)"))?; targets.extend(clean_bins( features, - manifest.bin.as_ref(), + resolved_toml.bin.as_ref(), package_root, package_name, edition, @@ -80,7 +79,7 @@ pub(super) fn targets( )?); targets.extend(clean_examples( - manifest.example.as_ref(), + resolved_toml.example.as_ref(), package_root, edition, package.autoexamples, @@ -89,7 +88,7 @@ pub(super) fn targets( )?); targets.extend(clean_tests( - manifest.test.as_ref(), + resolved_toml.test.as_ref(), package_root, edition, package.autotests, @@ -98,7 +97,7 @@ pub(super) fn targets( )?); targets.extend(clean_benches( - manifest.bench.as_ref(), + resolved_toml.bench.as_ref(), package_root, edition, package.autobenches, @@ -126,7 +125,7 @@ pub(super) fn targets( } if let Some(metabuild) = metabuild { // Verify names match available build deps. - let bdeps = manifest.build_dependencies.as_ref(); + let bdeps = resolved_toml.build_dependencies.as_ref(); for name in &metabuild.0 { if !bdeps.map_or(false, |bd| bd.contains_key(name.as_str())) { anyhow::bail!( @@ -146,14 +145,14 @@ pub(super) fn targets( } fn clean_lib( - toml_lib: Option<&TomlLibTarget>, + resolved_lib: Option<&TomlLibTarget>, package_root: &Path, package_name: &str, edition: Edition, warnings: &mut Vec, ) -> CargoResult> { let inferred = inferred_lib(package_root); - let lib = match toml_lib { + let lib = match resolved_lib { Some(lib) => { if let Some(ref name) = lib.name { if name.contains('-') { @@ -264,7 +263,7 @@ fn clean_lib( let mut target = Target::lib_target(name_or_panic(lib), crate_types, path, edition); configure(lib, &mut target)?; - target.set_name_inferred(toml_lib.map_or(true, |v| v.name.is_none())); + target.set_name_inferred(resolved_lib.map_or(true, |v| v.name.is_none())); Ok(Some(target)) } From b86b4b4fe52e2b2e43b961a1365c95d80f7b777f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:05:04 -0500 Subject: [PATCH 04/23] refactor(toml): Clarify conversion nature of target fn's --- src/cargo/util/toml/mod.rs | 4 ++-- src/cargo/util/toml/targets.rs | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index d97c07d861b..73637038bcb 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -32,7 +32,7 @@ use crate::util::{self, context::ConfigRelativePath, GlobalContext, IntoUrl, Opt mod embedded; mod targets; -use self::targets::targets; +use self::targets::to_targets; /// See also `bin/cargo/commands/run.rs`s `is_manifest_command` pub fn is_embedded(path: &Path) -> bool { @@ -1065,7 +1065,7 @@ fn to_real_manifest( // If we have no lib at all, use the inferred lib, if available. // If we have a lib with a path, we're done. // If we have a lib with no path, use the inferred lib or else the package name. - let targets = targets( + let targets = to_targets( &features, &resolved_toml, package_name, diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 40f52b283b8..7b557b434e0 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -33,7 +33,7 @@ const DEFAULT_EXAMPLE_DIR_NAME: &'static str = "examples"; const DEFAULT_BIN_DIR_NAME: &'static str = "bin"; #[tracing::instrument(skip_all)] -pub(super) fn targets( +pub(super) fn to_targets( features: &Features, resolved_toml: &TomlManifest, package_name: &str, @@ -48,7 +48,7 @@ pub(super) fn targets( let has_lib; - if let Some(target) = clean_lib( + if let Some(target) = to_lib_target( resolved_toml.lib.as_ref(), package_root, package_name, @@ -66,7 +66,7 @@ pub(super) fn targets( .as_ref() .ok_or_else(|| anyhow::format_err!("manifest has no `package` (or `project`)"))?; - targets.extend(clean_bins( + targets.extend(to_bin_targets( features, resolved_toml.bin.as_ref(), package_root, @@ -78,7 +78,7 @@ pub(super) fn targets( has_lib, )?); - targets.extend(clean_examples( + targets.extend(to_example_targets( resolved_toml.example.as_ref(), package_root, edition, @@ -87,7 +87,7 @@ pub(super) fn targets( errors, )?); - targets.extend(clean_tests( + targets.extend(to_test_targets( resolved_toml.test.as_ref(), package_root, edition, @@ -96,7 +96,7 @@ pub(super) fn targets( errors, )?); - targets.extend(clean_benches( + targets.extend(to_bench_targets( resolved_toml.bench.as_ref(), package_root, edition, @@ -144,7 +144,7 @@ pub(super) fn targets( Ok(targets) } -fn clean_lib( +fn to_lib_target( resolved_lib: Option<&TomlLibTarget>, package_root: &Path, package_name: &str, @@ -267,7 +267,7 @@ fn clean_lib( Ok(Some(target)) } -fn clean_bins( +fn to_bin_targets( features: &Features, toml_bins: Option<&Vec>, package_root: &Path, @@ -390,7 +390,7 @@ fn clean_bins( } } -fn clean_examples( +fn to_example_targets( toml_examples: Option<&Vec>, package_root: &Path, edition: Edition, @@ -435,7 +435,7 @@ fn clean_examples( Ok(result) } -fn clean_tests( +fn to_test_targets( toml_tests: Option<&Vec>, package_root: &Path, edition: Edition, @@ -472,7 +472,7 @@ fn clean_tests( Ok(result) } -fn clean_benches( +fn to_bench_targets( toml_benches: Option<&Vec>, package_root: &Path, edition: Edition, From 74e8d760f03cab21d445d84c7403b2fd11cb58b3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 21:37:34 -0500 Subject: [PATCH 05/23] refactor(toml): Simplify lib resolving --- src/cargo/util/toml/targets.rs | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 7b557b434e0..601b35978a1 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -152,29 +152,21 @@ fn to_lib_target( warnings: &mut Vec, ) -> CargoResult> { let inferred = inferred_lib(package_root); - let lib = match resolved_lib { - Some(lib) => { - if let Some(ref name) = lib.name { - if name.contains('-') { - anyhow::bail!("library target names cannot contain hyphens: {}", name) - } - } - Some(TomlTarget { - name: lib - .name - .clone() - .or_else(|| Some(package_name.replace("-", "_"))), - ..lib.clone() - }) - } - None => inferred.as_ref().map(|lib| TomlTarget { - name: Some(package_name.replace("-", "_")), + let lib = resolved_lib.cloned().or_else(|| { + inferred.as_ref().map(|lib| TomlTarget { path: Some(PathValue(lib.clone())), ..TomlTarget::new() - }), - }; + }) + }); + let Some(mut lib) = lib else { return Ok(None) }; + let name = lib + .name + .get_or_insert_with(|| package_name.replace("-", "_")); + if name.contains('-') { + anyhow::bail!("library target names cannot contain hyphens: {}", name) + } - let Some(ref lib) = lib else { return Ok(None) }; + let lib = &lib; validate_proc_macro(lib, "library", warnings); validate_crate_types(lib, "library", warnings); From 6ef9779752cda4f3c1cc4861c90fd2b277873234 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 21:46:42 -0500 Subject: [PATCH 06/23] refactor(toml): Abstract out lib name validation --- src/cargo/util/toml/targets.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 601b35978a1..99095a7f683 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -159,18 +159,14 @@ fn to_lib_target( }) }); let Some(mut lib) = lib else { return Ok(None) }; - let name = lib - .name + lib.name .get_or_insert_with(|| package_name.replace("-", "_")); - if name.contains('-') { - anyhow::bail!("library target names cannot contain hyphens: {}", name) - } let lib = &lib; validate_proc_macro(lib, "library", warnings); validate_crate_types(lib, "library", warnings); - validate_target_name(lib, "library", "lib", warnings)?; + validate_lib_name(lib, warnings)?; let path = match (lib.path.as_ref(), inferred) { (Some(path), _) => package_root.join(&path.0), @@ -769,6 +765,16 @@ fn inferred_to_toml_targets(inferred: &[(String, PathBuf)]) -> Vec { .collect() } +fn validate_lib_name(target: &TomlTarget, warnings: &mut Vec) -> CargoResult<()> { + validate_target_name(target, "library", "lib", warnings)?; + let name = name_or_panic(target); + if name.contains('-') { + anyhow::bail!("library target names cannot contain hyphens: {}", name) + } + + Ok(()) +} + fn validate_target_name( target: &TomlTarget, target_kind_human: &str, From 019bae2efb375aa39c484aecd88bfc2755ad7999 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 21:48:56 -0500 Subject: [PATCH 07/23] refactor(toml): Consolidate lib discovery --- src/cargo/util/toml/targets.rs | 42 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 99095a7f683..140a527f1be 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -161,37 +161,37 @@ fn to_lib_target( let Some(mut lib) = lib else { return Ok(None) }; lib.name .get_or_insert_with(|| package_name.replace("-", "_")); - - let lib = &lib; - validate_proc_macro(lib, "library", warnings); - validate_crate_types(lib, "library", warnings); - - validate_lib_name(lib, warnings)?; - - let path = match (lib.path.as_ref(), inferred) { - (Some(path), _) => package_root.join(&path.0), - (None, Some(path)) => path, - (None, None) => { - let legacy_path = package_root - .join("src") - .join(format!("{}.rs", name_or_panic(lib))); - if edition == Edition::Edition2015 && legacy_path.exists() { + // Check early to improve error messages + validate_lib_name(&lib, warnings)?; + + if lib.path.is_none() { + if let Some(inferred) = inferred { + lib.path = Some(PathValue(inferred)); + } else { + let name = name_or_panic(&lib); + let legacy_path = Path::new("src").join(format!("{name}.rs")); + if edition == Edition::Edition2015 && package_root.join(&legacy_path).exists() { warnings.push(format!( - "path `{}` was erroneously implicitly accepted for library `{}`,\n\ + "path `{}` was erroneously implicitly accepted for library `{name}`,\n\ please rename the file to `src/lib.rs` or set lib.path in Cargo.toml", legacy_path.display(), - name_or_panic(lib) )); - legacy_path + lib.path = Some(PathValue(legacy_path)); } else { anyhow::bail!( - "can't find library `{}`, \ + "can't find library `{name}`, \ rename file to `src/lib.rs` or specify lib.path", - name_or_panic(lib) ) } } - }; + } + + let lib = &lib; + validate_proc_macro(lib, "library", warnings); + validate_crate_types(lib, "library", warnings); + + let path = lib.path.as_ref().expect("previously resolved"); + let path = package_root.join(&path.0); if lib.plugin == Some(true) { warnings.push(format!( From f900b3f776a9a3bfdbd38d4af7cdc51d1d1d1d32 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 21:52:33 -0500 Subject: [PATCH 08/23] refactor(toml): Split lib discovery from Target creation --- src/cargo/util/toml/targets.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 140a527f1be..08c9ce2e03b 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -144,15 +144,15 @@ pub(super) fn to_targets( Ok(targets) } -fn to_lib_target( - resolved_lib: Option<&TomlLibTarget>, +fn resolve_lib( + original_lib: Option<&TomlLibTarget>, package_root: &Path, package_name: &str, edition: Edition, warnings: &mut Vec, -) -> CargoResult> { +) -> CargoResult> { let inferred = inferred_lib(package_root); - let lib = resolved_lib.cloned().or_else(|| { + let lib = original_lib.cloned().or_else(|| { inferred.as_ref().map(|lib| TomlTarget { path: Some(PathValue(lib.clone())), ..TomlTarget::new() @@ -186,7 +186,21 @@ fn to_lib_target( } } - let lib = &lib; + Ok(Some(lib)) +} + +fn to_lib_target( + original_lib: Option<&TomlLibTarget>, + package_root: &Path, + package_name: &str, + edition: Edition, + warnings: &mut Vec, +) -> CargoResult> { + let lib = resolve_lib(original_lib, package_root, package_name, edition, warnings)?; + + let Some(lib) = &lib else { + return Ok(None); + }; validate_proc_macro(lib, "library", warnings); validate_crate_types(lib, "library", warnings); @@ -251,7 +265,7 @@ fn to_lib_target( let mut target = Target::lib_target(name_or_panic(lib), crate_types, path, edition); configure(lib, &mut target)?; - target.set_name_inferred(resolved_lib.map_or(true, |v| v.name.is_none())); + target.set_name_inferred(original_lib.map_or(true, |v| v.name.is_none())); Ok(Some(target)) } From b08d19779c5bd70233013b9e7122695cfe56f238 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 21:54:48 -0500 Subject: [PATCH 09/23] refactor(toml): Move lib resolving up a level --- src/cargo/util/toml/targets.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 08c9ce2e03b..800a8aeae7f 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -48,12 +48,19 @@ pub(super) fn to_targets( let has_lib; - if let Some(target) = to_lib_target( + let lib = resolve_lib( resolved_toml.lib.as_ref(), package_root, package_name, edition, warnings, + )?; + if let Some(target) = to_lib_target( + resolved_toml.lib.as_ref(), + lib.as_ref(), + package_root, + edition, + warnings, )? { targets.push(target); has_lib = true; @@ -191,14 +198,12 @@ fn resolve_lib( fn to_lib_target( original_lib: Option<&TomlLibTarget>, + resolved_lib: Option<&TomlLibTarget>, package_root: &Path, - package_name: &str, edition: Edition, warnings: &mut Vec, ) -> CargoResult> { - let lib = resolve_lib(original_lib, package_root, package_name, edition, warnings)?; - - let Some(lib) = &lib else { + let Some(lib) = resolved_lib else { return Ok(None); }; validate_proc_macro(lib, "library", warnings); From f2b0678436594464a6181046aacb5aed43dad7bd Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 12:17:55 -0500 Subject: [PATCH 10/23] refactor(toml): Pull out legacy_bin_path --- src/cargo/util/toml/targets.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 800a8aeae7f..b76454a481a 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -372,29 +372,29 @@ fn to_bin_targets( configure(bin, &mut target)?; result.push(target); } - return Ok(result); + Ok(result) +} - fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option { - if !has_lib { - let path = package_root.join("src").join(format!("{}.rs", name)); - if path.exists() { - return Some(path); - } - } - let path = package_root.join("src").join("main.rs"); +fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option { + if !has_lib { + let path = package_root.join("src").join(format!("{}.rs", name)); if path.exists() { return Some(path); } + } + let path = package_root.join("src").join("main.rs"); + if path.exists() { + return Some(path); + } - let path = package_root - .join("src") - .join(DEFAULT_BIN_DIR_NAME) - .join("main.rs"); - if path.exists() { - return Some(path); - } - None + let path = package_root + .join("src") + .join(DEFAULT_BIN_DIR_NAME) + .join("main.rs"); + if path.exists() { + return Some(path); } + None } fn to_example_targets( From b061bc5e8317a2b776f793cb445a988814c50133 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 13:52:58 -0500 Subject: [PATCH 11/23] refactor(toml): Abstract out bin name validation --- src/cargo/util/toml/targets.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index b76454a481a..d90dac656f2 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -301,18 +301,17 @@ fn to_bin_targets( // This loop performs basic checks on each of the TomlTarget in `bins`. for bin in &bins { + validate_bin_name(bin, warnings)?; + // For each binary, check if the `filename` parameter is populated. If it is, // check if the corresponding cargo feature has been activated. if bin.filename.is_some() { features.require(Feature::different_binary_name())?; } - validate_target_name(bin, "binary", "bin", warnings)?; - - let name = name_or_panic(bin).to_owned(); - if let Some(crate_types) = bin.crate_types() { if !crate_types.is_empty() { + let name = name_or_panic(bin); errors.push(format!( "the target `{}` is a binary and can't have any \ crate-types set (currently \"{}\")", @@ -323,20 +322,13 @@ fn to_bin_targets( } if bin.proc_macro() == Some(true) { + let name = name_or_panic(bin); errors.push(format!( "the target `{}` is a binary and can't have `proc-macro` \ set `true`", name )); } - - if restricted_names::is_conflicting_artifact_name(&name) { - anyhow::bail!( - "the binary target name `{}` is forbidden, \ - it conflicts with cargo's build directory names", - name - ) - } } validate_unique_names(&bins, "binary")?; @@ -794,6 +786,19 @@ fn validate_lib_name(target: &TomlTarget, warnings: &mut Vec) -> CargoRe Ok(()) } +fn validate_bin_name(bin: &TomlTarget, warnings: &mut Vec) -> CargoResult<()> { + validate_target_name(bin, "binary", "bin", warnings)?; + let name = name_or_panic(bin).to_owned(); + if restricted_names::is_conflicting_artifact_name(&name) { + anyhow::bail!( + "the binary target name `{name}` is forbidden, \ + it conflicts with cargo's build directory names", + ) + } + + Ok(()) +} + fn validate_target_name( target: &TomlTarget, target_kind_human: &str, From 72ee1405489059c173de05028042d24e6602d2d4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 13:29:27 -0500 Subject: [PATCH 12/23] refactor(toml): Split bin discovery from Target creation --- src/cargo/util/toml/targets.rs | 76 +++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index d90dac656f2..272d27ddb4e 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -274,20 +274,18 @@ fn to_lib_target( Ok(Some(target)) } -fn to_bin_targets( - features: &Features, +fn resolve_bins( toml_bins: Option<&Vec>, package_root: &Path, package_name: &str, edition: Edition, autodiscover: Option, warnings: &mut Vec, - errors: &mut Vec, has_lib: bool, -) -> CargoResult> { +) -> CargoResult> { let inferred = inferred_bins(package_root, package_name); - let bins = toml_targets_and_inferred( + let mut bins = toml_targets_and_inferred( toml_bins, &inferred, package_root, @@ -299,10 +297,55 @@ fn to_bin_targets( "autobins", ); - // This loop performs basic checks on each of the TomlTarget in `bins`. - for bin in &bins { + for bin in &mut bins { validate_bin_name(bin, warnings)?; + 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)); + } + + Ok(bins) +} + +fn to_bin_targets( + features: &Features, + toml_bins: Option<&Vec>, + package_root: &Path, + package_name: &str, + edition: Edition, + autodiscover: Option, + warnings: &mut Vec, + errors: &mut Vec, + has_lib: bool, +) -> CargoResult> { + let bins = resolve_bins( + toml_bins, + package_root, + package_name, + edition, + autodiscover, + warnings, + has_lib, + )?; + + // This loop performs basic checks on each of the TomlTarget in `bins`. + for bin in &bins { // For each binary, check if the `filename` parameter is populated. If it is, // check if the corresponding cargo feature has been activated. if bin.filename.is_some() { @@ -335,24 +378,7 @@ fn to_bin_targets( let mut result = Vec::new(); for bin in &bins { - 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), - }; - + let path = bin.path.clone().expect("previously resolved").0; let mut target = Target::bin_target( name_or_panic(bin), bin.filename.clone(), From c699941d13d365c387413e01b6de6fd5d6b6af2a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:01:22 -0500 Subject: [PATCH 13/23] refactor(toml): Move bin resolving up a level --- src/cargo/util/toml/targets.rs | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 272d27ddb4e..492b67dae06 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -73,17 +73,16 @@ pub(super) fn to_targets( .as_ref() .ok_or_else(|| anyhow::format_err!("manifest has no `package` (or `project`)"))?; - targets.extend(to_bin_targets( - features, + let bins = resolve_bins( resolved_toml.bin.as_ref(), package_root, package_name, edition, package.autobins, warnings, - errors, has_lib, - )?); + )?; + targets.extend(to_bin_targets(features, &bins, edition, errors)?); targets.extend(to_example_targets( resolved_toml.example.as_ref(), @@ -325,27 +324,12 @@ fn resolve_bins( fn to_bin_targets( features: &Features, - toml_bins: Option<&Vec>, - package_root: &Path, - package_name: &str, + bins: &[TomlBinTarget], edition: Edition, - autodiscover: Option, - warnings: &mut Vec, errors: &mut Vec, - has_lib: bool, ) -> CargoResult> { - let bins = resolve_bins( - toml_bins, - package_root, - package_name, - edition, - autodiscover, - warnings, - has_lib, - )?; - // This loop performs basic checks on each of the TomlTarget in `bins`. - for bin in &bins { + for bin in bins { // For each binary, check if the `filename` parameter is populated. If it is, // check if the corresponding cargo feature has been activated. if bin.filename.is_some() { @@ -377,7 +361,7 @@ fn to_bin_targets( validate_unique_names(&bins, "binary")?; let mut result = Vec::new(); - for bin in &bins { + for bin in bins { let path = bin.path.clone().expect("previously resolved").0; let mut target = Target::bin_target( name_or_panic(bin), From 943f9bc8711951f86954d201f02337bc0896d070 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:18:32 -0500 Subject: [PATCH 14/23] refactor(toml): Track paths in TomlTarget, rather than next to it --- src/cargo/util/toml/targets.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 492b67dae06..565d75ba68b 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -423,7 +423,8 @@ fn to_example_targets( )?; let mut result = Vec::new(); - for (path, toml) in targets { + for toml in targets { + let path = toml.path.clone().expect("previously resolved").0; validate_crate_types(&toml, "example", warnings); let crate_types = match toml.crate_types() { Some(kinds) => kinds.iter().map(|s| s.into()).collect(), @@ -468,7 +469,8 @@ fn to_test_targets( )?; let mut result = Vec::new(); - for (path, toml) in targets { + for toml in targets { + let path = toml.path.clone().expect("previously resolved").0; let mut target = Target::test_target( name_or_panic(&toml), path, @@ -526,7 +528,8 @@ fn to_bench_targets( warnings.append(&mut legacy_warnings); let mut result = Vec::new(); - for (path, toml) in targets { + for toml in targets { + let path = toml.path.clone().expect("previously resolved").0; let mut target = Target::bench_target( name_or_panic(&toml), path, @@ -551,7 +554,7 @@ fn clean_targets( warnings: &mut Vec, errors: &mut Vec, autodiscover_flag_name: &str, -) -> CargoResult> { +) -> CargoResult> { clean_targets_with_legacy_path( target_kind_human, target_kind, @@ -579,7 +582,7 @@ fn clean_targets_with_legacy_path( errors: &mut Vec, legacy_path: &mut dyn FnMut(&TomlTarget) -> Option, autodiscover_flag_name: &str, -) -> CargoResult> { +) -> CargoResult> { let toml_targets = toml_targets_and_inferred( toml_targets, inferred, @@ -598,7 +601,7 @@ fn clean_targets_with_legacy_path( validate_unique_names(&toml_targets, target_kind)?; let mut result = Vec::new(); - for target in toml_targets { + for mut target in toml_targets { let path = target_path( &target, inferred, @@ -614,7 +617,8 @@ fn clean_targets_with_legacy_path( continue; } }; - result.push((path, target)); + target.path = Some(PathValue(path)); + result.push(target); } Ok(result) } From 36891bacfd50595cdec8438233c8f658c989134c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:27:23 -0500 Subject: [PATCH 15/23] refactor(toml): Move unique name validation out of resolving --- src/cargo/util/toml/targets.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 565d75ba68b..c47c25171eb 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -422,6 +422,8 @@ fn to_example_targets( "autoexamples", )?; + validate_unique_names(&targets, "example")?; + let mut result = Vec::new(); for toml in targets { let path = toml.path.clone().expect("previously resolved").0; @@ -468,6 +470,8 @@ fn to_test_targets( "autotests", )?; + validate_unique_names(&targets, "test")?; + let mut result = Vec::new(); for toml in targets { let path = toml.path.clone().expect("previously resolved").0; @@ -524,9 +528,10 @@ fn to_bench_targets( "autobenches", )? }; - warnings.append(&mut legacy_warnings); + validate_unique_names(&targets, "bench")?; + let mut result = Vec::new(); for toml in targets { let path = toml.path.clone().expect("previously resolved").0; @@ -599,7 +604,6 @@ fn clean_targets_with_legacy_path( validate_target_name(target, target_kind_human, target_kind, warnings)?; } - validate_unique_names(&toml_targets, target_kind)?; let mut result = Vec::new(); for mut target in toml_targets { let path = target_path( From 8439a4b21ac3c81142a63ee3729ba5c9f84488e2 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:32:52 -0500 Subject: [PATCH 16/23] refactor(toml): Split target discovery from Target creation --- src/cargo/util/toml/targets.rs | 127 ++++++++++++++++++++++++--------- 1 file changed, 92 insertions(+), 35 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index c47c25171eb..b7668867eb2 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -399,14 +399,14 @@ fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option>, package_root: &Path, edition: Edition, autodiscover: Option, warnings: &mut Vec, errors: &mut Vec, -) -> CargoResult> { +) -> CargoResult> { let inferred = infer_from_directory(&package_root.join(DEFAULT_EXAMPLE_DIR_NAME)); let targets = clean_targets( @@ -422,6 +422,26 @@ fn to_example_targets( "autoexamples", )?; + Ok(targets) +} + +fn to_example_targets( + toml_examples: Option<&Vec>, + package_root: &Path, + edition: Edition, + autodiscover: Option, + warnings: &mut Vec, + errors: &mut Vec, +) -> CargoResult> { + let targets = resolve_examples( + toml_examples, + package_root, + edition, + autodiscover, + warnings, + errors, + )?; + validate_unique_names(&targets, "example")?; let mut result = Vec::new(); @@ -447,14 +467,14 @@ fn to_example_targets( Ok(result) } -fn to_test_targets( +fn resolve_tests( toml_tests: Option<&Vec>, package_root: &Path, edition: Edition, autodiscover: Option, warnings: &mut Vec, errors: &mut Vec, -) -> CargoResult> { +) -> CargoResult> { let inferred = infer_from_directory(&package_root.join(DEFAULT_TEST_DIR_NAME)); let targets = clean_targets( @@ -470,6 +490,26 @@ fn to_test_targets( "autotests", )?; + Ok(targets) +} + +fn to_test_targets( + toml_tests: Option<&Vec>, + package_root: &Path, + edition: Edition, + autodiscover: Option, + warnings: &mut Vec, + errors: &mut Vec, +) -> CargoResult> { + let targets = resolve_tests( + toml_tests, + package_root, + edition, + autodiscover, + warnings, + errors, + )?; + validate_unique_names(&targets, "test")?; let mut result = Vec::new(); @@ -487,49 +527,66 @@ fn to_test_targets( Ok(result) } -fn to_bench_targets( +fn resolve_benches( toml_benches: Option<&Vec>, package_root: &Path, edition: Edition, autodiscover: Option, warnings: &mut Vec, errors: &mut Vec, -) -> CargoResult> { +) -> CargoResult> { let mut legacy_warnings = vec![]; - - let targets = { - let mut legacy_bench_path = |bench: &TomlTarget| { - let legacy_path = package_root.join("src").join("bench.rs"); - if !(name_or_panic(bench) == "bench" && legacy_path.exists()) { - return None; - } - legacy_warnings.push(format!( - "path `{}` was erroneously implicitly accepted for benchmark `{}`,\n\ + let mut legacy_bench_path = |bench: &TomlTarget| { + let legacy_path = package_root.join("src").join("bench.rs"); + if !(name_or_panic(bench) == "bench" && legacy_path.exists()) { + return None; + } + legacy_warnings.push(format!( + "path `{}` was erroneously implicitly accepted for benchmark `{}`,\n\ please set bench.path in Cargo.toml", - legacy_path.display(), - name_or_panic(bench) - )); - Some(legacy_path) - }; + legacy_path.display(), + name_or_panic(bench) + )); + Some(legacy_path) + }; - let inferred = infer_from_directory(&package_root.join("benches")); + let inferred = infer_from_directory(&package_root.join("benches")); - clean_targets_with_legacy_path( - "benchmark", - "bench", - toml_benches, - &inferred, - package_root, - edition, - autodiscover, - warnings, - errors, - &mut legacy_bench_path, - "autobenches", - )? - }; + let targets = clean_targets_with_legacy_path( + "benchmark", + "bench", + toml_benches, + &inferred, + package_root, + edition, + autodiscover, + warnings, + errors, + &mut legacy_bench_path, + "autobenches", + )?; warnings.append(&mut legacy_warnings); + Ok(targets) +} + +fn to_bench_targets( + toml_benches: Option<&Vec>, + package_root: &Path, + edition: Edition, + autodiscover: Option, + warnings: &mut Vec, + errors: &mut Vec, +) -> CargoResult> { + let targets = resolve_benches( + toml_benches, + package_root, + edition, + autodiscover, + warnings, + errors, + )?; + validate_unique_names(&targets, "bench")?; let mut result = Vec::new(); From e8f56df032bd9b3191ed44ad7f909622ba02e245 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:33:37 -0500 Subject: [PATCH 17/23] refactor(toml): Clarify meaning of target 'clean' fn's --- src/cargo/util/toml/targets.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index b7668867eb2..66d2df18821 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -409,7 +409,7 @@ fn resolve_examples( ) -> CargoResult> { let inferred = infer_from_directory(&package_root.join(DEFAULT_EXAMPLE_DIR_NAME)); - let targets = clean_targets( + let targets = resolve_targets( "example", "example", toml_examples, @@ -477,7 +477,7 @@ fn resolve_tests( ) -> CargoResult> { let inferred = infer_from_directory(&package_root.join(DEFAULT_TEST_DIR_NAME)); - let targets = clean_targets( + let targets = resolve_targets( "test", "test", toml_tests, @@ -552,7 +552,7 @@ fn resolve_benches( let inferred = infer_from_directory(&package_root.join("benches")); - let targets = clean_targets_with_legacy_path( + let targets = resolve_targets_with_legacy_path( "benchmark", "bench", toml_benches, @@ -605,7 +605,7 @@ fn to_bench_targets( Ok(result) } -fn clean_targets( +fn resolve_targets( target_kind_human: &str, target_kind: &str, toml_targets: Option<&Vec>, @@ -617,7 +617,7 @@ fn clean_targets( errors: &mut Vec, autodiscover_flag_name: &str, ) -> CargoResult> { - clean_targets_with_legacy_path( + resolve_targets_with_legacy_path( target_kind_human, target_kind, toml_targets, @@ -632,7 +632,7 @@ fn clean_targets( ) } -fn clean_targets_with_legacy_path( +fn resolve_targets_with_legacy_path( target_kind_human: &str, target_kind: &str, toml_targets: Option<&Vec>, From f243c91f68252c0d158cd7644d67369a835ac4f0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:41:01 -0500 Subject: [PATCH 18/23] refactor(toml): Move target resolving up a level --- src/cargo/util/toml/targets.rs | 65 +++++++--------------------------- 1 file changed, 12 insertions(+), 53 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 66d2df18821..0ccb90a4b37 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -84,32 +84,35 @@ pub(super) fn to_targets( )?; targets.extend(to_bin_targets(features, &bins, edition, errors)?); - targets.extend(to_example_targets( + let toml_examples = resolve_examples( resolved_toml.example.as_ref(), package_root, edition, package.autoexamples, warnings, errors, - )?); + )?; + targets.extend(to_example_targets(&toml_examples, edition, warnings)?); - targets.extend(to_test_targets( + let toml_tests = resolve_tests( resolved_toml.test.as_ref(), package_root, edition, package.autotests, warnings, errors, - )?); + )?; + targets.extend(to_test_targets(&toml_tests, edition)?); - targets.extend(to_bench_targets( + let toml_benches = resolve_benches( resolved_toml.bench.as_ref(), package_root, edition, package.autobenches, warnings, errors, - )?); + )?; + targets.extend(to_bench_targets(&toml_benches, edition)?); // processing the custom build script if let Some(custom_build) = maybe_custom_build(custom_build, package_root) { @@ -426,22 +429,10 @@ fn resolve_examples( } fn to_example_targets( - toml_examples: Option<&Vec>, - package_root: &Path, + targets: &[TomlExampleTarget], edition: Edition, - autodiscover: Option, warnings: &mut Vec, - errors: &mut Vec, ) -> CargoResult> { - let targets = resolve_examples( - toml_examples, - package_root, - edition, - autodiscover, - warnings, - errors, - )?; - validate_unique_names(&targets, "example")?; let mut result = Vec::new(); @@ -493,23 +484,7 @@ fn resolve_tests( Ok(targets) } -fn to_test_targets( - toml_tests: Option<&Vec>, - package_root: &Path, - edition: Edition, - autodiscover: Option, - warnings: &mut Vec, - errors: &mut Vec, -) -> CargoResult> { - let targets = resolve_tests( - toml_tests, - package_root, - edition, - autodiscover, - warnings, - errors, - )?; - +fn to_test_targets(targets: &[TomlTestTarget], edition: Edition) -> CargoResult> { validate_unique_names(&targets, "test")?; let mut result = Vec::new(); @@ -570,23 +545,7 @@ fn resolve_benches( Ok(targets) } -fn to_bench_targets( - toml_benches: Option<&Vec>, - package_root: &Path, - edition: Edition, - autodiscover: Option, - warnings: &mut Vec, - errors: &mut Vec, -) -> CargoResult> { - let targets = resolve_benches( - toml_benches, - package_root, - edition, - autodiscover, - warnings, - errors, - )?; - +fn to_bench_targets(targets: &[TomlBenchTarget], edition: Edition) -> CargoResult> { validate_unique_names(&targets, "bench")?; let mut result = Vec::new(); From b2ca0655520af97ef88eae301275a3dce08ec716 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 5 Apr 2024 10:36:30 -0500 Subject: [PATCH 19/23] refactor(toml): Prefer common constant for bench dir name --- src/cargo/util/toml/targets.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 0ccb90a4b37..0d19fcbd758 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -525,7 +525,7 @@ fn resolve_benches( Some(legacy_path) }; - let inferred = infer_from_directory(&package_root.join("benches")); + let inferred = infer_from_directory(&package_root.join(DEFAULT_BENCH_DIR_NAME)); let targets = resolve_targets_with_legacy_path( "benchmark", From b6c7544bd9f183b250676cdf32e5f26439c92978 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 5 Apr 2024 10:37:08 -0500 Subject: [PATCH 20/23] refactor(toml): Avoid incomplete constant for bin dir name --- src/cargo/util/toml/targets.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 0d19fcbd758..9cad18e308c 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -30,7 +30,6 @@ use crate::util::toml::warn_on_deprecated; const DEFAULT_TEST_DIR_NAME: &'static str = "tests"; const DEFAULT_BENCH_DIR_NAME: &'static str = "benches"; const DEFAULT_EXAMPLE_DIR_NAME: &'static str = "examples"; -const DEFAULT_BIN_DIR_NAME: &'static str = "bin"; #[tracing::instrument(skip_all)] pub(super) fn to_targets( @@ -392,10 +391,7 @@ fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option Vec<(String, PathBu if main.exists() { result.push((package_name.to_string(), main)); } - result.extend(infer_from_directory( - &package_root.join("src").join(DEFAULT_BIN_DIR_NAME), - )); + result.extend(infer_from_directory(&package_root.join("src").join("bin"))); result } @@ -938,7 +932,7 @@ fn target_path_not_found_error_message( ("example", false) => target_path.push(DEFAULT_EXAMPLE_DIR_NAME), ("bin", false) => { target_path.push("src"); - target_path.push(DEFAULT_BIN_DIR_NAME); + target_path.push("bin"); } _ => unreachable!("invalid target kind: {}", kind), } From f38136eeca19b131698375ca2209b92b378e0c71 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 14:56:54 -0500 Subject: [PATCH 21/23] refactor(toml): Give higher level info to target inference --- src/cargo/util/toml/targets.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 9cad18e308c..1e81beb290f 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -406,7 +406,7 @@ fn resolve_examples( warnings: &mut Vec, errors: &mut Vec, ) -> CargoResult> { - let inferred = infer_from_directory(&package_root.join(DEFAULT_EXAMPLE_DIR_NAME)); + let inferred = infer_from_directory(&package_root, Path::new(DEFAULT_EXAMPLE_DIR_NAME)); let targets = resolve_targets( "example", @@ -462,7 +462,7 @@ fn resolve_tests( warnings: &mut Vec, errors: &mut Vec, ) -> CargoResult> { - let inferred = infer_from_directory(&package_root.join(DEFAULT_TEST_DIR_NAME)); + let inferred = infer_from_directory(&package_root, Path::new(DEFAULT_TEST_DIR_NAME)); let targets = resolve_targets( "test", @@ -521,7 +521,7 @@ fn resolve_benches( Some(legacy_path) }; - let inferred = infer_from_directory(&package_root.join(DEFAULT_BENCH_DIR_NAME)); + let inferred = infer_from_directory(&package_root, Path::new(DEFAULT_BENCH_DIR_NAME)); let targets = resolve_targets_with_legacy_path( "benchmark", @@ -654,12 +654,14 @@ fn inferred_bins(package_root: &Path, package_name: &str) -> Vec<(String, PathBu if main.exists() { result.push((package_name.to_string(), main)); } - result.extend(infer_from_directory(&package_root.join("src").join("bin"))); + let default_bin_dir_name = Path::new("src").join("bin"); + result.extend(infer_from_directory(&package_root, &default_bin_dir_name)); result } -fn infer_from_directory(directory: &Path) -> Vec<(String, PathBuf)> { +fn infer_from_directory(package_root: &Path, relpath: &Path) -> Vec<(String, PathBuf)> { + let directory = package_root.join(relpath); let entries = match fs::read_dir(directory) { Err(_) => return Vec::new(), Ok(dir) => dir, From f1f7bbf6acfa8bc75f31884d0fe3305766de1632 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 15:05:41 -0500 Subject: [PATCH 22/23] refactor(toml): Simplify file/dir target inference --- src/cargo/util/toml/targets.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 1e81beb290f..0f293f27520 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -686,18 +686,18 @@ fn infer_any(entry: &DirEntry) -> Option<(String, PathBuf)> { fn infer_file(entry: &DirEntry) -> Option<(String, PathBuf)> { let path = entry.path(); - path.file_stem() - .and_then(|p| p.to_str()) - .map(|p| (p.to_owned(), path.clone())) + let stem = path.file_stem()?.to_str()?.to_owned(); + Some((stem, path)) } fn infer_subdirectory(entry: &DirEntry) -> Option<(String, PathBuf)> { let path = entry.path(); let main = path.join("main.rs"); - let name = path.file_name().and_then(|n| n.to_str()); - match (name, main.exists()) { - (Some(name), true) => Some((name.to_owned(), main)), - _ => None, + let name = path.file_name()?.to_str()?.to_owned(); + if main.exists() { + Some((name, main)) + } else { + None } } From ff4186836c51baf55d6381222dc76fec287f20c8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 4 Apr 2024 16:14:33 -0500 Subject: [PATCH 23/23] refactor(toml): Use relative paths in resolved targets --- src/cargo/util/toml/targets.rs | 98 ++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 33 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 0f293f27520..937177c9583 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -81,7 +81,13 @@ pub(super) fn to_targets( warnings, has_lib, )?; - targets.extend(to_bin_targets(features, &bins, edition, errors)?); + targets.extend(to_bin_targets( + features, + &bins, + package_root, + edition, + errors, + )?); let toml_examples = resolve_examples( resolved_toml.example.as_ref(), @@ -91,7 +97,12 @@ pub(super) fn to_targets( warnings, errors, )?; - targets.extend(to_example_targets(&toml_examples, edition, warnings)?); + targets.extend(to_example_targets( + &toml_examples, + package_root, + edition, + warnings, + )?); let toml_tests = resolve_tests( resolved_toml.test.as_ref(), @@ -101,7 +112,7 @@ pub(super) fn to_targets( warnings, errors, )?; - targets.extend(to_test_targets(&toml_tests, edition)?); + targets.extend(to_test_targets(&toml_tests, package_root, edition)?); let toml_benches = resolve_benches( resolved_toml.bench.as_ref(), @@ -111,7 +122,7 @@ pub(super) fn to_targets( warnings, errors, )?; - targets.extend(to_bench_targets(&toml_benches, edition)?); + targets.extend(to_bench_targets(&toml_benches, package_root, edition)?); // processing the custom build script if let Some(custom_build) = maybe_custom_build(custom_build, package_root) { @@ -327,6 +338,7 @@ fn resolve_bins( fn to_bin_targets( features: &Features, bins: &[TomlBinTarget], + package_root: &Path, edition: Edition, errors: &mut Vec, ) -> CargoResult> { @@ -364,7 +376,7 @@ fn to_bin_targets( let mut result = Vec::new(); for bin in bins { - let path = bin.path.clone().expect("previously resolved").0; + let path = package_root.join(&bin.path.as_ref().expect("previously resolved").0); let mut target = Target::bin_target( name_or_panic(bin), bin.filename.clone(), @@ -381,19 +393,21 @@ fn to_bin_targets( fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option { if !has_lib { - let path = package_root.join("src").join(format!("{}.rs", name)); - if path.exists() { - return Some(path); + let rel_path = Path::new("src").join(format!("{}.rs", name)); + if package_root.join(&rel_path).exists() { + return Some(rel_path); } } - let path = package_root.join("src").join("main.rs"); - if path.exists() { - return Some(path); + + let rel_path = Path::new("src").join("main.rs"); + if package_root.join(&rel_path).exists() { + return Some(rel_path); } - let path = package_root.join("src").join("bin").join("main.rs"); - if path.exists() { - return Some(path); + let default_bin_dir_name = Path::new("src").join("bin"); + let rel_path = default_bin_dir_name.join("main.rs"); + if package_root.join(&rel_path).exists() { + return Some(rel_path); } None } @@ -426,6 +440,7 @@ fn resolve_examples( fn to_example_targets( targets: &[TomlExampleTarget], + package_root: &Path, edition: Edition, warnings: &mut Vec, ) -> CargoResult> { @@ -433,7 +448,7 @@ fn to_example_targets( let mut result = Vec::new(); for toml in targets { - let path = toml.path.clone().expect("previously resolved").0; + let path = package_root.join(&toml.path.as_ref().expect("previously resolved").0); validate_crate_types(&toml, "example", warnings); let crate_types = match toml.crate_types() { Some(kinds) => kinds.iter().map(|s| s.into()).collect(), @@ -480,12 +495,16 @@ fn resolve_tests( Ok(targets) } -fn to_test_targets(targets: &[TomlTestTarget], edition: Edition) -> CargoResult> { +fn to_test_targets( + targets: &[TomlTestTarget], + package_root: &Path, + edition: Edition, +) -> CargoResult> { validate_unique_names(&targets, "test")?; let mut result = Vec::new(); for toml in targets { - let path = toml.path.clone().expect("previously resolved").0; + let path = package_root.join(&toml.path.as_ref().expect("previously resolved").0); let mut target = Target::test_target( name_or_panic(&toml), path, @@ -508,8 +527,8 @@ fn resolve_benches( ) -> CargoResult> { let mut legacy_warnings = vec![]; let mut legacy_bench_path = |bench: &TomlTarget| { - let legacy_path = package_root.join("src").join("bench.rs"); - if !(name_or_panic(bench) == "bench" && legacy_path.exists()) { + let legacy_path = Path::new("src").join("bench.rs"); + if !(name_or_panic(bench) == "bench" && package_root.join(&legacy_path).exists()) { return None; } legacy_warnings.push(format!( @@ -541,12 +560,16 @@ fn resolve_benches( Ok(targets) } -fn to_bench_targets(targets: &[TomlBenchTarget], edition: Edition) -> CargoResult> { +fn to_bench_targets( + targets: &[TomlBenchTarget], + package_root: &Path, + edition: Edition, +) -> CargoResult> { validate_unique_names(&targets, "bench")?; let mut result = Vec::new(); for toml in targets { - let path = toml.path.clone().expect("previously resolved").0; + let path = package_root.join(&toml.path.as_ref().expect("previously resolved").0); let mut target = Target::bench_target( name_or_panic(&toml), path, @@ -640,8 +663,8 @@ fn resolve_targets_with_legacy_path( } fn inferred_lib(package_root: &Path) -> Option { - let lib = package_root.join("src").join("lib.rs"); - if lib.exists() { + let lib = Path::new("src").join("lib.rs"); + if package_root.join(&lib).exists() { Some(lib) } else { None @@ -649,13 +672,14 @@ fn inferred_lib(package_root: &Path) -> Option { } fn inferred_bins(package_root: &Path, package_name: &str) -> Vec<(String, PathBuf)> { - let main = package_root.join("src").join("main.rs"); + let main = "src/main.rs"; let mut result = Vec::new(); - if main.exists() { + if package_root.join(main).exists() { + let main = PathBuf::from(main); result.push((package_name.to_string(), main)); } let default_bin_dir_name = Path::new("src").join("bin"); - result.extend(infer_from_directory(&package_root, &default_bin_dir_name)); + result.extend(infer_from_directory(package_root, &default_bin_dir_name)); result } @@ -670,31 +694,39 @@ fn infer_from_directory(package_root: &Path, relpath: &Path) -> Vec<(String, Pat entries .filter_map(|e| e.ok()) .filter(is_not_dotfile) - .filter_map(|d| infer_any(&d)) + .filter_map(|d| infer_any(package_root, &d)) .collect() } -fn infer_any(entry: &DirEntry) -> Option<(String, PathBuf)> { +fn infer_any(package_root: &Path, entry: &DirEntry) -> Option<(String, PathBuf)> { if entry.file_type().map_or(false, |t| t.is_dir()) { - infer_subdirectory(entry) + infer_subdirectory(package_root, entry) } else if entry.path().extension().and_then(|p| p.to_str()) == Some("rs") { - infer_file(entry) + infer_file(package_root, entry) } else { None } } -fn infer_file(entry: &DirEntry) -> Option<(String, PathBuf)> { +fn infer_file(package_root: &Path, entry: &DirEntry) -> Option<(String, PathBuf)> { let path = entry.path(); let stem = path.file_stem()?.to_str()?.to_owned(); + let path = path + .strip_prefix(package_root) + .map(|p| p.to_owned()) + .unwrap_or(path); Some((stem, path)) } -fn infer_subdirectory(entry: &DirEntry) -> Option<(String, PathBuf)> { +fn infer_subdirectory(package_root: &Path, entry: &DirEntry) -> Option<(String, PathBuf)> { let path = entry.path(); let main = path.join("main.rs"); let name = path.file_name()?.to_str()?.to_owned(); if main.exists() { + let main = main + .strip_prefix(package_root) + .map(|p| p.to_owned()) + .unwrap_or(main); Some((name, main)) } else { None @@ -997,7 +1029,7 @@ fn target_path( ) -> Result { if let Some(ref path) = target.path { // Should we verify that this path exists here? - return Ok(package_root.join(&path.0)); + return Ok(path.0.clone()); } let name = name_or_panic(target).to_owned();