Extract common code for dealing with path-valued arguments

diff --git a/src/bin/command_prelude.rs b/src/bin/command_prelude.rs
index b93d16dc26ce34dcc76e8e60aa357e797cef248c..bc738a052b808df783520a2f320a5bae880525d5 100644 (file)
--- a/src/bin/command_prelude.rs
+++ b/src/bin/command_prelude.rs
@@ -1,10 +1,12 @@
 use std::path::PathBuf;
+use std::fs;
 
 use clap::{self, SubCommand};
 use cargo::CargoResult;
 use cargo::core::Workspace;
 use cargo::ops::{CompileFilter, CompileMode, CompileOptions, MessageFormat, NewOptions, Packages,
                  VersionControl};
+use cargo::util::paths;
 use cargo::util::important_paths::find_root_manifest_for_wd;
 
 pub use clap::{AppSettings, Arg, ArgMatches};
@@ -196,9 +198,28 @@ pub trait ArgMatchesExt {
         Ok(arg)
     }
 
+    /// Returns value of the `name` command-line argument as an absolute path
+    fn value_of_path(&self, name: &str, config: &Config) -> Option<PathBuf> {
+        self._value_of(name).map(|path| config.cwd().join(path))
+    }
+
     fn root_manifest(&self, config: &Config) -> CargoResult<PathBuf> {
-        let manifest_path = self._value_of("manifest-path");
-        find_root_manifest_for_wd(manifest_path, config.cwd())
+        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);
+            if !path.ends_with("Cargo.toml") {
+                bail!("the manifest-path must be a path to a Cargo.toml file")
+            }
+            if !fs::metadata(&path).is_ok() {
+                bail!(
+                    "manifest path `{}` does not exist",
+                    self._value_of("manifest-path").unwrap()
+                )
+            }
+            return Ok(path);
+        }
+        find_root_manifest_for_wd(config.cwd())
     }
 
     fn workspace<'a>(&self, config: &'a Config) -> CargoResult<Workspace<'a>> {
@@ -281,7 +302,7 @@ pub trait ArgMatchesExt {
         Ok(compile_opts)
     }
 
-    fn new_options(&self) -> CargoResult<NewOptions> {
+    fn new_options(&self, config: &Config) -> CargoResult<NewOptions> {
         let vcs = self._value_of("vcs").map(|vcs| match vcs {
             "git" => VersionControl::Git,
             "hg" => VersionControl::Hg,
@@ -294,7 +315,7 @@ pub trait ArgMatchesExt {
             vcs,
             self._is_present("bin"),
             self._is_present("lib"),
-            self._value_of("path").unwrap().to_string(),
+            self.value_of_path("path", config).unwrap(),
             self._value_of("name").map(|s| s.to_string()),
         )
     }

diff --git a/src/bin/commands/init.rs b/src/bin/commands/init.rs
index 9155813ade543fd5e98423f41338984b0ef3c01a..c32dead4d522776173e97462b2d29146c20ded1e 100644 (file)
--- a/src/bin/commands/init.rs
+++ b/src/bin/commands/init.rs
@@ -10,7 +10,7 @@ pub fn cli() -> App {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let opts = args.new_options()?;
+    let opts = args.new_options(config)?;
     ops::init(&opts, config)?;
     config
         .shell()

diff --git a/src/bin/commands/install.rs b/src/bin/commands/install.rs
index 369b52898a82cabcc949ed8535fd85cc0aa6ae85..4c259e4d39eb429a00b302f613efe0410316b025 100644 (file)
--- a/src/bin/commands/install.rs
+++ b/src/bin/commands/install.rs
@@ -91,8 +91,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
             GitReference::Branch("master".to_string())
         };
         SourceId::for_git(&url, gitref)?
-    } else if let Some(path) = args.value_of("path") {
-        SourceId::for_path(&config.cwd().join(path))?
+    } else if let Some(path) = args.value_of_path("path", config) {
+        SourceId::for_path(&path)?
     } else if krates.is_empty() {
         SourceId::for_path(config.cwd())?
     } else {

diff --git a/src/bin/commands/new.rs b/src/bin/commands/new.rs
index 68d199222f7997fa0a754992a53f3c2e26fc3f01..8b8e607400e219d869f4b1c0293d73a2e0d51d17 100644 (file)
--- a/src/bin/commands/new.rs
+++ b/src/bin/commands/new.rs
@@ -10,7 +10,7 @@ pub fn cli() -> App {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let opts = args.new_options()?;
+    let opts = args.new_options(config)?;
     ops::new(&opts, config)?;
     let path = args.value_of("path").unwrap();
     config

diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs
index c433b739c9400f130d03f67d40a0a9e30dcb798c..4d53ede2fb429d70a1a5176b16021647e89d0018 100644 (file)
--- a/src/cargo/ops/cargo_new.rs
+++ b/src/cargo/ops/cargo_new.rs
@@ -2,7 +2,7 @@ use std::collections::BTreeMap;
 use std::env;
 use std::fs;
 use std::fmt;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use git2::Config as GitConfig;
 use git2::Repository as GitRepository;
@@ -28,7 +28,8 @@ pub enum VersionControl {
 pub struct NewOptions {
     pub version_control: Option<VersionControl>,
     pub kind: NewProjectKind,
-    pub path: String,
+    /// Absolute path to the directory for the new project
+    pub path: PathBuf,
     pub name: Option<String>,
 }
 
@@ -72,7 +73,7 @@ impl NewOptions {
         version_control: Option<VersionControl>,
         bin: bool,
         lib: bool,
-        path: String,
+        path: PathBuf,
         name: Option<String>,
     ) -> CargoResult<NewOptions> {
         let kind = match (bin, lib) {
@@ -303,17 +304,16 @@ fn plan_new_source_file(bin: bool, project_name: String) -> SourceFileInformatio
 }
 
 pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
-    let path = config.cwd().join(&opts.path);
-    if fs::metadata(&path).is_ok() {
+    let path = &opts.path;
+    if fs::metadata(path).is_ok() {
         bail!(
             "destination `{}` already exists\n\n\
-             Use `cargo init` to initialize the directory\
-             ",
+             Use `cargo init` to initialize the directory",
             path.display()
         )
     }
 
-    let name = get_name(&path, opts)?;
+    let name = get_name(path, opts)?;
     check_name(name, opts)?;
 
     let mkopts = MkOptions {
@@ -335,10 +335,9 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
 }
 
 pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
-    let path = config.cwd().join(&opts.path);
+    let path = &opts.path;
 
-    let cargotoml_path = path.join("Cargo.toml");
-    if fs::metadata(&cargotoml_path).is_ok() {
+    if fs::metadata(&path.join("Cargo.toml")).is_ok() {
         bail!("`cargo init` cannot be run on existing Cargo projects")
     }
 
@@ -395,7 +394,7 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
 
     let mkopts = MkOptions {
         version_control,
-        path: &path,
+        path,
         name,
         bin: src_paths_types.iter().any(|x| x.bin),
         source_files: src_paths_types,

diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs
index 60e4641c1aaf0d219b50b717df889173f5dd29cc..ed997ccb266df8cc696db432aec431a790fe9ff1 100644 (file)
--- a/src/cargo/ops/registry.rs
+++ b/src/cargo/ops/registry.rs
@@ -446,7 +446,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
     let name = match opts.krate {
         Some(ref name) => name.clone(),
         None => {
-            let manifest_path = find_root_manifest_for_wd(None, config.cwd())?;
+            let manifest_path = find_root_manifest_for_wd(config.cwd())?;
             let ws = Workspace::new(&manifest_path, config)?;
             ws.current()?.package_id().name().to_string()
         }
@@ -507,7 +507,7 @@ pub fn yank(
     let name = match krate {
         Some(name) => name,
         None => {
-            let manifest_path = find_root_manifest_for_wd(None, config.cwd())?;
+            let manifest_path = find_root_manifest_for_wd(config.cwd())?;
             let ws = Workspace::new(&manifest_path, config)?;
             ws.current()?.package_id().name().to_string()
         }

diff --git a/src/cargo/util/important_paths.rs b/src/cargo/util/important_paths.rs
index f6d207ada86ce0b63c77092a4d5233c147de24dd..2fb4dea59f6d3c06890bac5d14bddf236a933864 100644 (file)
--- a/src/cargo/util/important_paths.rs
+++ b/src/cargo/util/important_paths.rs
@@ -3,57 +3,23 @@ use std::path::{Path, PathBuf};
 use util::errors::CargoResult;
 use util::paths;
 
-/// Iteratively search for `file` in `pwd` and its parents, returning
-/// the path of the directory.
-pub fn find_project(pwd: &Path, file: &str) -> CargoResult<PathBuf> {
-    find_project_manifest(pwd, file).map(|mut p| {
-        // remove the file, leaving just the directory
-        p.pop();
-        p
-    })
-}
-
-/// Iteratively search for `file` in `pwd` and its parents, returning
-/// the path to the file.
-pub fn find_project_manifest(pwd: &Path, file: &str) -> CargoResult<PathBuf> {
-    let mut current = pwd;
-
-    loop {
+/// Find the root Cargo.toml
+pub fn find_root_manifest_for_wd(cwd: &Path) -> CargoResult<PathBuf> {
+    let file = "Cargo.toml";
+    for current in paths::ancestors(cwd) {
         let manifest = current.join(file);
         if fs::metadata(&manifest).is_ok() {
             return Ok(manifest);
         }
-
-        match current.parent() {
-            Some(p) => current = p,
-            None => break,
-        }
     }
 
     bail!(
         "could not find `{}` in `{}` or any parent directory",
         file,
-        pwd.display()
+        cwd.display()
     )
 }
 
-/// Find the root Cargo.toml
-pub fn find_root_manifest_for_wd(manifest_path: Option<&str>, cwd: &Path) -> CargoResult<PathBuf> {
-    match manifest_path {
-        Some(path) => {
-            let absolute_path = paths::normalize_path(&cwd.join(&path));
-            if !absolute_path.ends_with("Cargo.toml") {
-                bail!("the manifest-path must be a path to a Cargo.toml file")
-            }
-            if !fs::metadata(&absolute_path).is_ok() {
-                bail!("manifest path `{}` does not exist", path)
-            }
-            Ok(absolute_path)
-        }
-        None => find_project_manifest(cwd, "Cargo.toml"),
-    }
-}
-
 /// Return the path to the `file` in `pwd`, if it exists.
 pub fn find_project_manifest_exact(pwd: &Path, file: &str) -> CargoResult<PathBuf> {
     let manifest = pwd.join(file);