diff --git a/crates/cargo-util/Cargo.toml b/crates/cargo-util/Cargo.toml index c7848831f67..0238fd06360 100644 --- a/crates/cargo-util/Cargo.toml +++ b/crates/cargo-util/Cargo.toml @@ -16,6 +16,7 @@ hex = "0.4.2" jobserver = "0.1.21" libc = "0.2.88" log = "0.4.6" +normpath = "0.2" same-file = "1.0.6" shell-escape = "0.1.4" tempfile = "3.1.0" diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index ea39273a82d..7505ad41898 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -2,6 +2,7 @@ use anyhow::{Context, Result}; use filetime::FileTime; +use normpath::{BasePath, BasePathBuf, PathExt}; use std::env; use std::ffi::{OsStr, OsString}; use std::fs::{self, File, OpenOptions}; @@ -78,7 +79,7 @@ pub fn dylib_path() -> Vec { /// [`std::fs::canonicalize`] can be hard to use correctly, since it can often /// fail, or on Windows returns annoying device paths. This is a problem Cargo /// needs to improve on. -pub fn normalize_path(path: &Path) -> PathBuf { +pub fn normalize_path_legacy(path: &Path) -> PathBuf { let mut components = path.components().peekable(); let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { components.next(); @@ -105,6 +106,32 @@ pub fn normalize_path(path: &Path) -> PathBuf { ret } +/// Normalizes a path using "normpath" to return a reliable result. This +/// function should usually be used instead of `normalize_path_legacy`. +/// +/// The returned path will be absolute. +/// +/// Returns an error if normalization fails or the path doesn't exist. +pub fn normalize_path(path: &Path) -> Result { + return path + .normalize() + .with_context(|| format!("failed to normalize `{}`", path.display())) + .map(BasePathBuf::into_path_buf); +} + +/// Returns the normalized result of joining two paths. This function should be +/// used when `base` can be a verbatim path. libstd `Path` doesn't normalize +/// verbatim paths when joining. +/// +/// The returned path will be absolute. +/// +/// Returns an error if normalization fails or reading the current directory +/// fails when needed to normalize the path. +pub fn normalize_joined(base: &Path, path: &Path) -> Result { + let base = BasePath::new(base).with_context(|| "failed to read the current directory")?; + normalize_path(base.join(path).as_path()) +} + /// Returns the absolute path of where the given executable is located based /// on searching the `PATH` environment variable. /// diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 56f1536749c..7f5ccb597f6 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -551,7 +551,7 @@ impl<'cfg> Workspace<'cfg> { .join(root_link) .join("Cargo.toml"); debug!("find_root - pointer {}", path.display()); - paths::normalize_path(&path) + paths::normalize_path_legacy(&path) } { @@ -650,7 +650,7 @@ impl<'cfg> Workspace<'cfg> { if let Some(default) = default_members_paths { for path in default { - let normalized_path = paths::normalize_path(&path); + let normalized_path = paths::normalize_path_legacy(&path); let manifest_path = normalized_path.join("Cargo.toml"); if !self.members.contains(&manifest_path) { // default-members are allowed to be excluded, but they @@ -687,7 +687,7 @@ impl<'cfg> Workspace<'cfg> { root_manifest: &Path, is_path_dep: bool, ) -> CargoResult<()> { - let manifest_path = paths::normalize_path(manifest_path); + let manifest_path = paths::normalize_path_legacy(manifest_path); if self.members.contains(&manifest_path) { return Ok(()); } diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index fd37e2c730e..21de36bde33 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -199,7 +199,7 @@ fn build_ar_list( } if let Some(license_file) = &pkg.manifest().metadata().license_file { let license_path = Path::new(license_file); - let abs_license_path = paths::normalize_path(&pkg.root().join(license_path)); + let abs_license_path = paths::normalize_path_legacy(&pkg.root().join(license_path)); if abs_license_path.exists() { match abs_license_path.strip_prefix(&pkg.root()) { Ok(rel_license_path) => { diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index 732ef39d613..1b89c0c4fee 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -192,7 +192,7 @@ fn read_nested_packages( // TODO: filesystem/symlink implications? if !source_id.is_registry() { for p in nested.iter() { - let path = paths::normalize_path(&path.join(p)); + let path = paths::normalize_path_legacy(&path.join(p)); let result = read_nested_packages(&path, all_packages, source_id, config, visited, errors); // Ignore broken manifests found on git repositories. diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 3b5d9a7e23a..291077dbc2c 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -305,7 +305,7 @@ pub trait ArgMatchesExt { if let Some(path) = self.value_of_path("manifest-path", config) { // In general, we try to avoid normalizing paths in Cargo, // but in this particular case we need it to fix #3586. - let path = paths::normalize_path(&path); + let path = paths::normalize_path_legacy(&path); if !path.ends_with("Cargo.toml") { anyhow::bail!("the manifest-path must be a path to a Cargo.toml file") } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index be8a545ef10..0e06e1fd5b5 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -867,7 +867,7 @@ impl TomlManifest { package.resolver = ws.resolve_behavior().to_manifest(); if let Some(license_file) = &package.license_file { let license_path = Path::new(&license_file); - let abs_license_path = paths::normalize_path(&package_root.join(license_path)); + let abs_license_path = paths::normalize_path_legacy(&package_root.join(license_path)); if abs_license_path.strip_prefix(package_root).is_err() { // This path points outside of the package root. `cargo package` // will copy it into the root, so adjust the path to this location. @@ -1801,8 +1801,9 @@ impl DetailedTomlDependency

{ // always end up hashing to the same value no matter where it's // built from. if cx.source_id.is_path() { - let path = cx.root.join(path); - let path = paths::normalize_path(&path); + let path = paths::normalize_joined(cx.root, &path).chain_err(|| { + format!("dependency ({}) path does not exist", name_in_toml) + })?; SourceId::for_path(&path)? } else { cx.source_id diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 022b5b9ac2f..c6a30898864 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -1174,6 +1174,7 @@ fn ignored_git_revision() { "#, ) .file("src/lib.rs", "") + .file("bar/empty", "") .build(); foo.cargo("build -v") diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 65aa90862b7..cbd000b83a1 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -158,6 +158,50 @@ fn cargo_compile_manifest_path() { assert!(p.bin("foo").is_file()); } +// https://github.com/rust-lang/cargo/issues/6198 +#[cargo_test] +fn cargo_compile_verbatim_manifest_path() { + let p = project() + .no_manifest() + .file( + "main/Cargo.toml", + r#" + [package] + name = "main" + version = "0.0.1" + authors = [] + + [dependencies.dep_a] + path = "../dep_a" + [dependencies.dep_b] + path = "../dep_b" + "#, + ) + .file("main/src/lib.rs", "") + .file( + "dep_a/Cargo.toml", + r#" + [package] + name = "dep_a" + version = "0.0.1" + authors = [] + + [dependencies.dep_b] + path = "../dep_b" + "#, + ) + .file("dep_a/src/lib.rs", "") + .file("dep_b/Cargo.toml", &basic_manifest("dep_b", "0.0.1")) + .file("dep_b/src/lib.rs", "") + .build(); + + p.cargo("build") + .arg("--manifest-path") + // On Windows, canonicalization returns a verbatim path. + .arg(p.root().join("main/Cargo.toml").canonicalize().unwrap()) + .run(); +} + #[cargo_test] fn cargo_compile_with_invalid_manifest() { let p = project().file("Cargo.toml", "").build(); diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index 49a61301bac..2a4c278cda8 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -91,6 +91,7 @@ fn invalid3() { "#, ) .file("src/main.rs", "") + .file("foo/empty", "") .build(); p.cargo("build") @@ -168,6 +169,7 @@ fn invalid5() { "#, ) .file("src/main.rs", "") + .file("bar/empty", "") .build(); p.cargo("build") diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index eac28090a0e..ef5daa32d66 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -1044,18 +1044,23 @@ fn deep_path_error() { .with_status(101) .with_stderr( "\ -[ERROR] failed to get `c` as a dependency of package `b v0.1.0 [..]` - ... which is depended on by `a v0.1.0 [..]` +[ERROR] failed to get `b` as a dependency of package `a v0.1.0 [..]` ... which is depended on by `foo v0.1.0 [..]` Caused by: - failed to load source for dependency `c` + failed to load source for dependency `b` Caused by: - Unable to update [..]/foo/c + Unable to update [..]/foo/b Caused by: - failed to read `[..]/foo/c/Cargo.toml` + failed to parse manifest at `[..]/foo/b/Cargo.toml` + +Caused by: + dependency (c) path does not exist + +Caused by: + failed to normalize `[..]/foo/b/../c` Caused by: [..] diff --git a/tests/testsuite/workspaces.rs b/tests/testsuite/workspaces.rs index ace60d77f83..c30c660df68 100644 --- a/tests/testsuite/workspaces.rs +++ b/tests/testsuite/workspaces.rs @@ -2236,6 +2236,7 @@ fn ws_warn_unused() { ), ) .file("a/src/lib.rs", "") + .file("a/bar/empty", "") .build(); p.cargo("check") .with_stderr_contains(&format!( @@ -2300,16 +2301,13 @@ fn invalid_missing() { .with_status(101) .with_stderr( "\ -[ERROR] failed to get `x` as a dependency of package `foo v0.1.0 [..]` +[ERROR] failed to parse manifest at `[..]Cargo.toml` Caused by: - failed to load source for dependency `x` + dependency (x) path does not exist Caused by: - Unable to update [..]/foo/x - -Caused by: - failed to read `[..]foo/x/Cargo.toml` + failed to normalize `[..]x` Caused by: [..]