Fix --manifest-path for verbatim paths

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"

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<PathBuf> {
 /// [`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<PathBuf> {
+    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<PathBuf> {
+    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.
 ///

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(());
         }

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) => {

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.

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")
             }

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<P: ResolveToPath> DetailedTomlDependency<P> {
                 // 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

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")

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();

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")

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:
   [..]

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:
   [..]