Skip to content

Commit

Permalink
Auto merge of #13729 - epage:slash, r=Muscraft
Browse files Browse the repository at this point in the history
fix(package): Normalize paths in `Cargo.toml`

### What does this PR try to resolve?

On the surface, this resolves problems that aren't too big of a deal
- Clean up redundant information in paths (e.g. `.////.//foo.rs` being `foo.rs`) which is just visual
- Switch paths with `\` in them to `/`

However, this is prep for #13713 where these will be a much bigger deal
- If the user does `./foo.rs`, we might fail to compare that with the list of files included in the package
- We'll generate paths with `\` and need to make sure the packages can still be used on Windows

### How should we test and review this PR?

### Additional information
  • Loading branch information
bors committed Apr 10, 2024
2 parents f99d24f + 8b593e5 commit f8a73f7
Show file tree
Hide file tree
Showing 2 changed files with 218 additions and 8 deletions.
101 changes: 93 additions & 8 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(normalize_path_string_sep(path)));
}
let current_resolver = package
.resolver
.as_ref()
Expand All @@ -2362,7 +2370,16 @@ 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(
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`
// will copy it into the root, so adjust the path to this location.
package.license_file = Some(manifest::InheritableField::Value(
Expand All @@ -2384,7 +2401,18 @@ 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(
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`
// will copy it into the root, so adjust the path to this location.
package.readme = Some(manifest::InheritableField::Value(
Expand All @@ -2402,16 +2430,27 @@ fn prepare_toml_for_publish(
manifest::StringOrBool::Bool(_) => {}
}
}

let lib = if let Some(target) = &me.lib {
Some(prepare_target_for_publish(target, "library")?)
} else {
None
};
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 {
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,
Expand Down Expand Up @@ -2555,3 +2594,49 @@ fn prepare_toml_for_publish(
.map(manifest::InheritableDependency::Value)
}
}

fn prepare_targets_for_publish(
targets: Option<&Vec<manifest::TomlTarget>>,
context: &str,
) -> CargoResult<Option<Vec<manifest::TomlTarget>>> {
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, context)?;
prepared.push(target);
}

Ok(Some(prepared))
}

fn prepare_target_for_publish(
target: &manifest::TomlTarget,
context: &str,
) -> CargoResult<manifest::TomlTarget> {
let mut target = target.clone();
if let Some(path) = target.path {
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<PathBuf> {
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
}
}
125 changes: 125 additions & 0 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
"#,
)],
);
}

0 comments on commit f8a73f7

Please # to comment.