From 4eea907733bb55b6c27ebe809b531c0550a0ad33 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 9 Apr 2024 13:44:25 -0500 Subject: [PATCH 1/3] test(package): Show behavior with backslashes --- tests/testsuite/package.rs | 125 +++++++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 860d6962772..c0b50c1b01d 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3541,3 +3541,128 @@ See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for ) .run() } + +#[cargo_test] +#[cfg(windows)] +fn normalize_paths() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + description = "foo" + documentation = "docs.rs/foo" + authors = [] + readme = ".\\docs\\README.md" + license-file = ".\\docs\\LICENSE" + build = ".\\src\\build.rs" + + [lib] + path = ".\\src\\lib.rs" + + [[bin]] + name = "foo" + path = ".\\src\\bin\\foo\\main.rs" + + [[example]] + name = "example_foo" + path = ".\\examples\\example_foo.rs" + + [[test]] + name = "test_foo" + path = ".\\tests\\test_foo.rs" + + [[bench]] + name = "bench_foo" + path = ".\\benches\\bench_foo.rs" + "#, + ) + .file("src/lib.rs", "") + .file("docs/README.md", "") + .file("docs/LICENSE", "") + .file("src/build.rs", "fn main() {}") + .file("src/bin/foo/main.rs", "fn main() {}") + .file("examples/example_foo.rs", "fn main() {}") + .file("tests/test_foo.rs", "fn main() {}") + .file("benches/bench_foo.rs", "fn main() {}") + .build(); + + p.cargo("package") + .with_stdout("") + .with_stderr( + "\ +[PACKAGING] foo v0.0.1 ([CWD]) +[VERIFYING] foo v0.0.1 ([CWD]) +[COMPILING] foo v0.0.1 ([CWD][..]) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..] +[PACKAGED] 11 files, [..] ([..] compressed) +", + ) + .run(); + + let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap(); + validate_crate_contents( + f, + "foo-0.0.1.crate", + &[ + "Cargo.lock", + "Cargo.toml", + "Cargo.toml.orig", + "src/lib.rs", + "docs/README.md", + "docs/LICENSE", + "src/build.rs", + "src/bin/foo/main.rs", + "examples/example_foo.rs", + "tests/test_foo.rs", + "benches/bench_foo.rs", + ], + &[( + "Cargo.toml", + r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO +# +# When uploading crates to the registry Cargo will automatically +# "normalize" Cargo.toml files for maximal compatibility +# with all versions of Cargo and also rewrite `path` dependencies +# to registry (e.g., crates.io) dependencies. +# +# If you are reading this file be aware that the original Cargo.toml +# will likely look very different (and much more reasonable). +# See Cargo.toml.orig for the original contents. + +[package] +edition = "2015" +name = "foo" +version = "0.0.1" +authors = [] +build = './src/build.rs' +description = "foo" +documentation = "docs.rs/foo" +readme = './docs/README.md' +license-file = './docs/LICENSE' + +[lib] +path = './src/lib.rs' + +[[bin]] +name = "foo" +path = './src/bin/foo/main.rs' + +[[example]] +name = "example_foo" +path = './examples/example_foo.rs' + +[[test]] +name = "test_foo" +path = './tests/test_foo.rs' + +[[bench]] +name = "bench_foo" +path = './benches/bench_foo.rs' +"#, + )], + ); +} From 5539293cf6ab19a5e722460e921c42e890bb2a72 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 9 Apr 2024 16:08:21 -0500 Subject: [PATCH 2/3] fix(package): Normalize paths in published `Cargo.toml` For now, this is more for visual consistency. However, this blocks #13713 as we need to be able to make these paths comparable to what is included in the package. --- src/cargo/util/toml/mod.rs | 71 +++++++++++++++++++++++++++++++++----- tests/testsuite/package.rs | 16 ++++----- 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 73637038bcb..bdfb5d653bf 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -8,7 +8,7 @@ use std::str::{self, FromStr}; use crate::AlreadyPrintedError; use anyhow::{anyhow, bail, Context as _}; use cargo_platform::Platform; -use cargo_util::paths; +use cargo_util::paths::{self, normalize_path}; use cargo_util_schemas::manifest::{self, TomlManifest}; use cargo_util_schemas::manifest::{RustVersion, StringOrBool}; use itertools::Itertools; @@ -2336,6 +2336,14 @@ fn prepare_toml_for_publish( let mut package = me.package().unwrap().clone(); package.workspace = None; + if let Some(StringOrBool::String(path)) = &package.build { + let path = paths::normalize_path(Path::new(path)); + let path = path + .into_os_string() + .into_string() + .map_err(|_err| anyhow::format_err!("non-UTF8 `package.build`"))?; + package.build = Some(StringOrBool::String(path)); + } let current_resolver = package .resolver .as_ref() @@ -2362,7 +2370,14 @@ fn prepare_toml_for_publish( .context("license file should have been resolved before `prepare_for_publish()`")?; let license_path = Path::new(&license_file); let abs_license_path = paths::normalize_path(&package_root.join(license_path)); - if abs_license_path.strip_prefix(package_root).is_err() { + if let Ok(license_file) = abs_license_path.strip_prefix(package_root) { + package.license_file = Some(manifest::InheritableField::Value( + license_file + .to_str() + .ok_or_else(|| anyhow::format_err!("non-UTF8 `package.license-file`"))? + .to_owned(), + )); + } else { // This path points outside of the package root. `cargo package` // will copy it into the root, so adjust the path to this location. package.license_file = Some(manifest::InheritableField::Value( @@ -2384,7 +2399,14 @@ fn prepare_toml_for_publish( manifest::StringOrBool::String(readme) => { let readme_path = Path::new(&readme); let abs_readme_path = paths::normalize_path(&package_root.join(readme_path)); - if abs_readme_path.strip_prefix(package_root).is_err() { + if let Ok(readme_path) = abs_readme_path.strip_prefix(package_root) { + package.readme = Some(manifest::InheritableField::Value(StringOrBool::String( + readme_path + .to_str() + .ok_or_else(|| anyhow::format_err!("non-UTF8 `package.license-file`"))? + .to_owned(), + ))); + } else { // This path points outside of the package root. `cargo package` // will copy it into the root, so adjust the path to this location. package.readme = Some(manifest::InheritableField::Value( @@ -2402,16 +2424,27 @@ fn prepare_toml_for_publish( manifest::StringOrBool::Bool(_) => {} } } + + let lib = if let Some(target) = &me.lib { + Some(prepare_target_for_publish(target)) + } else { + None + }; + let bin = prepare_targets_for_publish(me.bin.as_ref()); + let example = prepare_targets_for_publish(me.example.as_ref()); + let test = prepare_targets_for_publish(me.test.as_ref()); + let bench = prepare_targets_for_publish(me.bench.as_ref()); + let all = |_d: &manifest::TomlDependency| true; let mut manifest = manifest::TomlManifest { package: Some(package), project: None, profile: me.profile.clone(), - lib: me.lib.clone(), - bin: me.bin.clone(), - example: me.example.clone(), - test: me.test.clone(), - bench: me.bench.clone(), + lib, + bin, + example, + test, + bench, dependencies: map_deps(gctx, me.dependencies.as_ref(), all)?, dev_dependencies: map_deps( gctx, @@ -2555,3 +2588,25 @@ fn prepare_toml_for_publish( .map(manifest::InheritableDependency::Value) } } + +fn prepare_targets_for_publish( + targets: Option<&Vec>, +) -> Option> { + let targets = targets?; + + let mut prepared = Vec::with_capacity(targets.len()); + for target in targets { + let target = prepare_target_for_publish(target); + prepared.push(target); + } + + Some(prepared) +} + +fn prepare_target_for_publish(target: &manifest::TomlTarget) -> manifest::TomlTarget { + let mut target = target.clone(); + if let Some(path) = target.path { + target.path = Some(manifest::PathValue(normalize_path(&path.0))); + } + target +} diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index c0b50c1b01d..93b0a295aec 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3638,30 +3638,30 @@ edition = "2015" name = "foo" version = "0.0.1" authors = [] -build = './src/build.rs' +build = 'src/build.rs' description = "foo" documentation = "docs.rs/foo" -readme = './docs/README.md' -license-file = './docs/LICENSE' +readme = 'docs/README.md' +license-file = 'docs/LICENSE' [lib] -path = './src/lib.rs' +path = 'src/lib.rs' [[bin]] name = "foo" -path = './src/bin/foo/main.rs' +path = 'src/bin/foo/main.rs' [[example]] name = "example_foo" -path = './examples/example_foo.rs' +path = 'examples/example_foo.rs' [[test]] name = "test_foo" -path = './tests/test_foo.rs' +path = 'tests/test_foo.rs' [[bench]] name = "bench_foo" -path = './benches/bench_foo.rs' +path = 'benches/bench_foo.rs' "#, )], ); From 8b593e5ba7442269ad89589921d3272565a903a4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 9 Apr 2024 16:57:37 -0500 Subject: [PATCH 3/3] fix(package): Normalize path separators A windows user could use `\` and no Linux or Mac user could use the package. This normalizes the separator to what works on all platforms. --- src/cargo/util/toml/mod.rs | 72 +++++++++++++++++++++++++++----------- tests/testsuite/package.rs | 16 ++++----- 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index bdfb5d653bf..9cf3bc9e957 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -2342,7 +2342,7 @@ fn prepare_toml_for_publish( .into_os_string() .into_string() .map_err(|_err| anyhow::format_err!("non-UTF8 `package.build`"))?; - package.build = Some(StringOrBool::String(path)); + package.build = Some(StringOrBool::String(normalize_path_string_sep(path))); } let current_resolver = package .resolver @@ -2372,10 +2372,12 @@ fn prepare_toml_for_publish( let abs_license_path = paths::normalize_path(&package_root.join(license_path)); if let Ok(license_file) = abs_license_path.strip_prefix(package_root) { package.license_file = Some(manifest::InheritableField::Value( - license_file - .to_str() - .ok_or_else(|| anyhow::format_err!("non-UTF8 `package.license-file`"))? - .to_owned(), + normalize_path_string_sep( + license_file + .to_str() + .ok_or_else(|| anyhow::format_err!("non-UTF8 `package.license-file`"))? + .to_owned(), + ), )); } else { // This path points outside of the package root. `cargo package` @@ -2401,10 +2403,14 @@ fn prepare_toml_for_publish( let abs_readme_path = paths::normalize_path(&package_root.join(readme_path)); if let Ok(readme_path) = abs_readme_path.strip_prefix(package_root) { package.readme = Some(manifest::InheritableField::Value(StringOrBool::String( - readme_path - .to_str() - .ok_or_else(|| anyhow::format_err!("non-UTF8 `package.license-file`"))? - .to_owned(), + normalize_path_string_sep( + readme_path + .to_str() + .ok_or_else(|| { + anyhow::format_err!("non-UTF8 `package.license-file`") + })? + .to_owned(), + ), ))); } else { // This path points outside of the package root. `cargo package` @@ -2426,14 +2432,14 @@ fn prepare_toml_for_publish( } let lib = if let Some(target) = &me.lib { - Some(prepare_target_for_publish(target)) + Some(prepare_target_for_publish(target, "library")?) } else { None }; - let bin = prepare_targets_for_publish(me.bin.as_ref()); - let example = prepare_targets_for_publish(me.example.as_ref()); - let test = prepare_targets_for_publish(me.test.as_ref()); - let bench = prepare_targets_for_publish(me.bench.as_ref()); + let bin = prepare_targets_for_publish(me.bin.as_ref(), "binary")?; + let example = prepare_targets_for_publish(me.example.as_ref(), "example")?; + let test = prepare_targets_for_publish(me.test.as_ref(), "test")?; + let bench = prepare_targets_for_publish(me.bench.as_ref(), "benchmark")?; let all = |_d: &manifest::TomlDependency| true; let mut manifest = manifest::TomlManifest { @@ -2591,22 +2597,46 @@ fn prepare_toml_for_publish( fn prepare_targets_for_publish( targets: Option<&Vec>, -) -> Option> { - let targets = targets?; + context: &str, +) -> CargoResult>> { + let Some(targets) = targets else { + return Ok(None); + }; let mut prepared = Vec::with_capacity(targets.len()); for target in targets { - let target = prepare_target_for_publish(target); + let target = prepare_target_for_publish(target, context)?; prepared.push(target); } - Some(prepared) + Ok(Some(prepared)) } -fn prepare_target_for_publish(target: &manifest::TomlTarget) -> manifest::TomlTarget { +fn prepare_target_for_publish( + target: &manifest::TomlTarget, + context: &str, +) -> CargoResult { let mut target = target.clone(); if let Some(path) = target.path { - target.path = Some(manifest::PathValue(normalize_path(&path.0))); + let path = normalize_path(&path.0); + target.path = Some(manifest::PathValue(normalize_path_sep(path, context)?)); + } + Ok(target) +} + +fn normalize_path_sep(path: PathBuf, context: &str) -> CargoResult { + let path = path + .into_os_string() + .into_string() + .map_err(|_err| anyhow::format_err!("non-UTF8 path for {context}"))?; + let path = normalize_path_string_sep(path); + Ok(path.into()) +} + +fn normalize_path_string_sep(path: String) -> String { + if std::path::MAIN_SEPARATOR != '/' { + path.replace(std::path::MAIN_SEPARATOR, "/") + } else { + path } - target } diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 93b0a295aec..6e072744e18 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3638,30 +3638,30 @@ edition = "2015" name = "foo" version = "0.0.1" authors = [] -build = 'src/build.rs' +build = "src/build.rs" description = "foo" documentation = "docs.rs/foo" -readme = 'docs/README.md' -license-file = 'docs/LICENSE' +readme = "docs/README.md" +license-file = "docs/LICENSE" [lib] -path = 'src/lib.rs' +path = "src/lib.rs" [[bin]] name = "foo" -path = 'src/bin/foo/main.rs' +path = "src/bin/foo/main.rs" [[example]] name = "example_foo" -path = 'examples/example_foo.rs' +path = "examples/example_foo.rs" [[test]] name = "test_foo" -path = 'tests/test_foo.rs' +path = "tests/test_foo.rs" [[bench]] name = "bench_foo" -path = 'benches/bench_foo.rs' +path = "benches/bench_foo.rs" "#, )], );