Refactor configuration return values
authorAlex Crichton <alex@alexcrichton.com>
Fri, 19 Feb 2016 07:51:52 +0000 (23:51 -0800)
committerAlex Crichton <alex@alexcrichton.com>
Fri, 19 Feb 2016 07:51:52 +0000 (23:51 -0800)
Wrap up the value/definition pair in a generic structure so we can extend it
well later.

src/cargo/ops/cargo_compile.rs
src/cargo/ops/cargo_install.rs
src/cargo/ops/cargo_new.rs
src/cargo/ops/registry.rs
src/cargo/util/config.rs

index ba7716f12b54e0fd324a52b80cf71d17b8359588..5226a8aeebbd8e215e31fe4aaf323f05805a02e1 100644 (file)
@@ -421,19 +421,21 @@ fn scrape_build_config(config: &Config,
                        target: Option<String>)
                        -> CargoResult<ops::BuildConfig> {
     let cfg_jobs = match try!(config.get_i64("build.jobs")) {
-        Some((n, p)) => {
-            if n <= 0 {
-                bail!("build.jobs must be positive, but found {} in {:?}", n, p)
-            } else if n >= u32::max_value() as i64 {
-                bail!("build.jobs is too large: found {} in {:?}", n, p)
+        Some(v) => {
+            if v.val <= 0 {
+                bail!("build.jobs must be positive, but found {} in {}",
+                      v.val, v.definition)
+            } else if v.val >= u32::max_value() as i64 {
+                bail!("build.jobs is too large: found {} in {}", v.val,
+                      v.definition)
             } else {
-                Some(n as u32)
+                Some(v.val as u32)
             }
         }
         None => None,
     };
     let jobs = jobs.or(cfg_jobs).unwrap_or(::num_cpus::get() as u32);
-    let cfg_target = try!(config.get_string("build.target")).map(|s| s.0);
+    let cfg_target = try!(config.get_string("build.target")).map(|s| s.val);
     let target = target.or(cfg_target);
     let mut base = ops::BuildConfig {
         jobs: jobs,
@@ -453,16 +455,18 @@ fn scrape_target_config(config: &Config, triple: &str)
 
     let key = format!("target.{}", triple);
     let mut ret = ops::TargetConfig {
-        ar: try!(config.get_path(&format!("{}.ar", key))),
-        linker: try!(config.get_path(&format!("{}.linker", key))),
+        ar: try!(config.get_path(&format!("{}.ar", key))).map(|v| v.val),
+        linker: try!(config.get_path(&format!("{}.linker", key))).map(|v| v.val),
         overrides: HashMap::new(),
     };
     let table = match try!(config.get_table(&key)) {
-        Some((table, _)) => table,
+        Some(table) => table.val,
         None => return Ok(ret),
     };
     for (lib_name, _) in table.into_iter() {
-        if lib_name == "ar" || lib_name == "linker" { continue }
+        if lib_name == "ar" || lib_name == "linker" {
+            continue
+        }
 
         let mut output = BuildOutput {
             library_paths: Vec::new(),
@@ -472,7 +476,7 @@ fn scrape_target_config(config: &Config, triple: &str)
             rerun_if_changed: Vec::new(),
         };
         let key = format!("{}.{}", key, lib_name);
-        let table = try!(config.get_table(&key)).unwrap().0;
+        let table = try!(config.get_table(&key)).unwrap().val;
         for (k, _) in table.into_iter() {
             let key = format!("{}.{}", key, k);
             match try!(config.get(&key)).unwrap() {
index 36fdabaf49615f9731841bc60d532d13c9c12719..95eb4238eea849c31ab6496d3738f1c10b00030d 100644 (file)
@@ -340,11 +340,11 @@ pub fn uninstall(root: Option<&str>,
 }
 
 fn resolve_root(flag: Option<&str>, config: &Config) -> CargoResult<PathBuf> {
-    let config_root = try!(config.get_string("install.root"));
+    let config_root = try!(config.get_path("install.root"));
     Ok(flag.map(PathBuf::from).or_else(|| {
         env::var_os("CARGO_INSTALL_ROOT").map(PathBuf::from)
-    }).or_else(|| {
-        config_root.clone().map(|(v, _)| PathBuf::from(v))
+    }).or_else(move || {
+        config_root.map(|v| v.val)
     }).unwrap_or_else(|| {
         config.home().to_owned()
     }))
index 309c75c61758332af66a193cdb680af34062e855..41fa01fc11fc575db3a17ae5fa04af8f8af473c2 100644 (file)
@@ -63,17 +63,17 @@ fn get_name<'a>(path: &'a Path, opts: &'a NewOptions, config: &Config) -> CargoR
     if let Some(name) = opts.name {
         return Ok(name);
     }
-    
+
     if path.file_name().is_none() {
         bail!("cannot auto-detect project name from path {:?} ; use --name to override",
                               path.as_os_str());
     }
-    
+
     let dir_name = try!(path.file_name().and_then(|s| s.to_str()).chain_error(|| {
         human(&format!("cannot create a project with a non-unicode name: {:?}",
                        path.file_name().unwrap()))
     }));
-    
+
     if opts.bin {
         Ok(dir_name)
     } else {
@@ -99,24 +99,24 @@ fn check_name(name: &str) -> CargoResult<()> {
     Ok(())
 }
 
-fn detect_source_paths_and_types(project_path : &Path, 
-                                 project_name: &str, 
+fn detect_source_paths_and_types(project_path : &Path,
+                                 project_name: &str,
                                  detected_files: &mut Vec<SourceFileInformation>,
                                  ) -> CargoResult<()> {
     let path = project_path;
     let name = project_name;
-    
+
     enum H {
         Bin,
         Lib,
         Detect,
     }
-    
+
     struct Test {
         proposed_path: String,
         handling: H,
     }
-        
+
     let tests = vec![
         Test { proposed_path: format!("src/main.rs"),     handling: H::Bin },
         Test { proposed_path: format!("main.rs"),         handling: H::Bin },
@@ -125,48 +125,48 @@ fn detect_source_paths_and_types(project_path : &Path,
         Test { proposed_path: format!("src/lib.rs"),      handling: H::Lib },
         Test { proposed_path: format!("lib.rs"),          handling: H::Lib },
     ];
-    
+
     for i in tests {
         let pp = i.proposed_path;
-        
+
         // path/pp does not exist or is not a file
         if !fs::metadata(&path.join(&pp)).map(|x| x.is_file()).unwrap_or(false) {
             continue;
         }
-        
+
         let sfi = match i.handling {
             H::Bin => {
-                SourceFileInformation { 
-                    relative_path: pp, 
-                    target_name: project_name.to_string(), 
-                    bin: true 
+                SourceFileInformation {
+                    relative_path: pp,
+                    target_name: project_name.to_string(),
+                    bin: true
                 }
             }
             H::Lib => {
-                SourceFileInformation { 
-                    relative_path: pp, 
-                    target_name: project_name.to_string(), 
+                SourceFileInformation {
+                    relative_path: pp,
+                    target_name: project_name.to_string(),
                     bin: false
                 }
             }
             H::Detect => {
                 let content = try!(paths::read(&path.join(pp.clone())));
                 let isbin = content.contains("fn main");
-                SourceFileInformation { 
-                    relative_path: pp, 
-                    target_name: project_name.to_string(), 
-                    bin: isbin 
+                SourceFileInformation {
+                    relative_path: pp,
+                    target_name: project_name.to_string(),
+                    bin: isbin
                 }
             }
         };
         detected_files.push(sfi);
     }
-    
+
     // Check for duplicate lib attempt
-    
+
     let mut previous_lib_relpath : Option<&str> = None;
     let mut duplicates_checker : BTreeMap<&str, &SourceFileInformation> = BTreeMap::new();
-        
+
     for i in detected_files {
         if i.bin {
             if let Some(x) = BTreeMap::get::<str>(&duplicates_checker, i.target_name.as_ref()) {
@@ -188,13 +188,13 @@ cannot automatically generate Cargo.toml as the main target would be ambiguous",
             previous_lib_relpath = Some(&i.relative_path);
         }
     }
-    
+
     Ok(())
 }
 
 fn plan_new_source_file(bin: bool, project_name: String) -> SourceFileInformation {
     if bin {
-        SourceFileInformation { 
+        SourceFileInformation {
              relative_path: "src/main.rs".to_string(),
              target_name: project_name,
              bin: true,
@@ -214,7 +214,7 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> {
         bail!("destination `{}` already exists",
               path.display())
     }
-    
+
     let name = try!(get_name(&path, &opts, config));
     try!(check_name(name));
 
@@ -225,7 +225,7 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> {
         source_files: vec![plan_new_source_file(opts.bin, name.to_string())],
         bin: opts.bin,
     };
-    
+
     mk(config, &mkopts).chain_error(|| {
         human(format!("Failed to create project `{}` at `{}`",
                       name, path.display()))
@@ -234,19 +234,19 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> {
 
 pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> {
     let path = config.cwd().join(opts.path);
-    
+
     let cargotoml_path = path.join("Cargo.toml");
     if fs::metadata(&cargotoml_path).is_ok() {
         bail!("`cargo init` cannot be run on existing Cargo projects")
     }
-    
+
     let name = try!(get_name(&path, &opts, config));
     try!(check_name(name));
-    
+
     let mut src_paths_types = vec![];
-    
+
     try!(detect_source_paths_and_types(&path, name, &mut src_paths_types));
-    
+
     if src_paths_types.len() == 0 {
         src_paths_types.push(plan_new_source_file(opts.bin, name.to_string()));
     } else {
@@ -254,24 +254,24 @@ pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> {
         // Maybe when doing `cargo init --bin` inside a library project stub,
         // user may mean "initialize for library, but also add binary target"
     }
-    
+
     let mut version_control = opts.version_control;
-    
+
     if version_control == None {
         let mut num_detected_vsces = 0;
-        
+
         if fs::metadata(&path.join(".git")).is_ok() {
             version_control = Some(VersionControl::Git);
             num_detected_vsces += 1;
         }
-        
+
         if fs::metadata(&path.join(".hg")).is_ok() {
             version_control = Some(VersionControl::Hg);
             num_detected_vsces += 1;
         }
-        
+
         // if none exists, maybe create git, like in `cargo new`
-        
+
         if num_detected_vsces > 1 {
             bail!("both .git and .hg directories found \
                               and the ignore file can't be \
@@ -279,7 +279,7 @@ pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> {
                               specify --vcs to override detection");
         }
     }
-    
+
     let mkopts = MkOptions {
         version_control: version_control,
         path: &path,
@@ -287,7 +287,7 @@ pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> {
         bin: src_paths_types.iter().any(|x|x.bin),
         source_files: src_paths_types,
     };
-    
+
     mk(config, &mkopts).chain_error(|| {
         human(format!("Failed to create project `{}` at `{}`",
                       name, path.display()))
@@ -357,11 +357,11 @@ fn mk(config: &Config, opts: &MkOptions) -> CargoResult<()> {
         (Some(name), None, _, None) |
         (None, None, name, None) => name,
     };
-    
+
     let mut cargotoml_path_specifier = String::new();
-    
+
     // Calculare what [lib] and [[bin]]s do we need to append to Cargo.toml
-    
+
     for i in &opts.source_files {
         if i.bin {
             if i.relative_path != "src/main.rs" {
@@ -394,17 +394,17 @@ authors = [{}]
 {}"#, name, toml::Value::String(author), cargotoml_path_specifier).as_bytes()));
 
 
-    // Create all specified source files 
-    // (with respective parent directories) 
+    // Create all specified source files
+    // (with respective parent directories)
     // if they are don't exist
 
     for i in &opts.source_files {
         let path_of_source_file = path.join(i.relative_path.clone());
-        
+
         if let Some(src_dir) = path_of_source_file.parent() {
             try!(fs::create_dir_all(src_dir));
         }
-    
+
         let default_file_content : &[u8] = if i.bin {
             b"\
 fn main() {
@@ -421,7 +421,7 @@ mod tests {
 }
 "
         };
-    
+
         if !fs::metadata(&path_of_source_file).map(|x| x.is_file()).unwrap_or(false) {
             return paths::write(&path_of_source_file, default_file_content)
         }
@@ -454,18 +454,18 @@ fn discover_author() -> CargoResult<(String, Option<String>)> {
 }
 
 fn global_config(config: &Config) -> CargoResult<CargoNewConfig> {
-    let name = try!(config.get_string("cargo-new.name")).map(|s| s.0);
-    let email = try!(config.get_string("cargo-new.email")).map(|s| s.0);
+    let name = try!(config.get_string("cargo-new.name")).map(|s| s.val);
+    let email = try!(config.get_string("cargo-new.email")).map(|s| s.val);
     let vcs = try!(config.get_string("cargo-new.vcs"));
 
-    let vcs = match vcs.as_ref().map(|p| (&p.0[..], &p.1)) {
+    let vcs = match vcs.as_ref().map(|p| (&p.val[..], &p.definition)) {
         Some(("git", _)) => Some(VersionControl::Git),
         Some(("hg", _)) => Some(VersionControl::Hg),
         Some(("none", _)) => Some(VersionControl::NoVcs),
         Some((s, p)) => {
             return Err(internal(format!("invalid configuration for key \
                                          `cargo-new.vcs`, unknown vcs `{}` \
-                                         (found in {:?})", s, p)))
+                                         (found in {})", s, p)))
         }
         None => None
     };
index 9bbdf87127acc0ba9a2fde063f65ee83a5639f0c..0b2f83e18a6886ade73a80d672a6e640c113aef1 100644 (file)
@@ -126,8 +126,8 @@ fn transmit(pkg: &Package, tarball: &Path, registry: &mut Registry)
 }
 
 pub fn registry_configuration(config: &Config) -> CargoResult<RegistryConfig> {
-    let index = try!(config.get_string("registry.index")).map(|p| p.0);
-    let token = try!(config.get_string("registry.token")).map(|p| p.0);
+    let index = try!(config.get_string("registry.index")).map(|p| p.val);
+    let token = try!(config.get_string("registry.token")).map(|p| p.val);
     Ok(RegistryConfig { index: index, token: token })
 }
 
@@ -182,7 +182,7 @@ pub fn http_handle(config: &Config) -> CargoResult<http::Handle> {
 /// via environment variables are picked up by libcurl.
 fn http_proxy(config: &Config) -> CargoResult<Option<String>> {
     match try!(config.get_string("http.proxy")) {
-        Some((s, _)) => return Ok(Some(s)),
+        Some(s) => return Ok(Some(s.val)),
         None => {}
     }
     match git2::Config::open_default() {
@@ -218,7 +218,7 @@ pub fn http_proxy_exists(config: &Config) -> CargoResult<bool> {
 
 pub fn http_timeout(config: &Config) -> CargoResult<Option<i64>> {
     match try!(config.get_i64("http.timeout")) {
-        Some((s, _)) => return Ok(Some(s)),
+        Some(s) => return Ok(Some(s.val)),
         None => {}
     }
     Ok(env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok()))
index 55c6543731f7018b62b07908da6ad7090e22fd9a..5160a91f76bee1e8cc367a9d90e476f979cfbad9 100644 (file)
@@ -148,50 +148,74 @@ impl Config {
         Ok(Some(val.clone()))
     }
 
-    pub fn get_string(&self, key: &str) -> CargoResult<Option<(String, PathBuf)>> {
+    pub fn get_string(&self, key: &str) -> CargoResult<Option<Value<String>>> {
         match try!(self.get(key)) {
-            Some(CV::String(i, path)) => Ok(Some((i, path))),
+            Some(CV::String(i, path)) => {
+                Ok(Some(Value {
+                    val: i,
+                    definition: Definition::Path(path),
+                }))
+            }
             Some(val) => self.expected("string", key, val),
             None => Ok(None),
         }
     }
 
-    pub fn get_path(&self, key: &str) -> CargoResult<Option<PathBuf>> {
-        if let Some((specified_path, path_to_config)) = try!(self.get_string(&key)) {
-            if specified_path.contains("/") || (cfg!(windows) && specified_path.contains("\\")) {
-                // An absolute or a relative path
-                let prefix_path = path_to_config.parent().unwrap().parent().unwrap();
-                // Joining an absolute path to any path results in the given absolute path
-                Ok(Some(prefix_path.join(specified_path)))
+    pub fn get_path(&self, key: &str) -> CargoResult<Option<Value<PathBuf>>> {
+        if let Some(val) = try!(self.get_string(&key)) {
+            let is_path = val.val.contains("/") ||
+                          (cfg!(windows) && val.val.contains("\\"));
+            let path = if is_path {
+                val.definition.root(self).join(val.val)
             } else {
                 // A pathless name
-                Ok(Some(PathBuf::from(specified_path)))
-            }
+                PathBuf::from(val.val)
+            };
+            Ok(Some(Value {
+                val: path,
+                definition: val.definition,
+            }))
         } else {
             Ok(None)
         }
     }
 
-    pub fn get_list(&self, key: &str) -> CargoResult<Option<(Vec<(String, PathBuf)>, PathBuf)>> {
+    pub fn get_list(&self, key: &str)
+                    -> CargoResult<Option<Value<Vec<(String, PathBuf)>>>> {
         match try!(self.get(key)) {
-            Some(CV::List(i, path)) => Ok(Some((i, path))),
+            Some(CV::List(i, path)) => {
+                Ok(Some(Value {
+                    val: i,
+                    definition: Definition::Path(path),
+                }))
+            }
             Some(val) => self.expected("list", key, val),
             None => Ok(None),
         }
     }
 
     pub fn get_table(&self, key: &str)
-                    -> CargoResult<Option<(HashMap<String, CV>, PathBuf)>> {
+                    -> CargoResult<Option<Value<HashMap<String, CV>>>> {
         match try!(self.get(key)) {
-            Some(CV::Table(i, path)) => Ok(Some((i, path))),
+            Some(CV::Table(i, path)) => {
+                Ok(Some(Value {
+                    val: i,
+                    definition: Definition::Path(path),
+                }))
+            }
             Some(val) => self.expected("table", key, val),
             None => Ok(None),
         }
     }
 
-    pub fn get_i64(&self, key: &str) -> CargoResult<Option<(i64, PathBuf)>> {
+    pub fn get_i64(&self, key: &str) -> CargoResult<Option<Value<i64>>> {
         match try!(self.get(key)) {
-            Some(CV::Integer(i, path)) => Ok(Some((i, path))),
+            Some(CV::Integer(i, path)) => {
+                Ok(Some(Value {
+                    val: i,
+                    definition: Definition::Path(path),
+                }))
+            }
             Some(val) => self.expected("integer", key, val),
             None => Ok(None),
         }
@@ -244,12 +268,8 @@ impl Config {
     fn scrape_target_dir_config(&mut self) -> CargoResult<()> {
         if let Some(dir) = env::var_os("CARGO_TARGET_DIR") {
             *self.target_dir.borrow_mut() = Some(self.cwd.join(dir));
-        } else if let Some((dir, dir2)) = try!(self.get_string("build.target-dir")) {
-            let mut path = PathBuf::from(dir2);
-            path.pop();
-            path.pop();
-            path.push(dir);
-            *self.target_dir.borrow_mut() = Some(path);
+        } else if let Some(val) = try!(self.get_path("build.target-dir")) {
+            *self.target_dir.borrow_mut() = Some(val.val);
         }
         Ok(())
     }
@@ -262,7 +282,7 @@ impl Config {
 
         let var = format!("build.{}", tool);
         if let Some(tool_path) = try!(self.get_path(&var)) {
-            return Ok(tool_path);
+            return Ok(tool_path.val);
         }
 
         Ok(PathBuf::from(tool))
@@ -284,6 +304,15 @@ pub enum ConfigValue {
     Boolean(bool, PathBuf),
 }
 
+pub struct Value<T> {
+    pub val: T,
+    pub definition: Definition,
+}
+
+pub enum Definition {
+    Path(PathBuf),
+}
+
 impl fmt::Debug for ConfigValue {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
@@ -466,6 +495,22 @@ impl ConfigValue {
     }
 }
 
+impl Definition {
+    pub fn root<'a>(&'a self, config: &'a Config) -> &'a Path {
+        match *self {
+            Definition::Path(ref p) => p.parent().unwrap().parent().unwrap(),
+        }
+    }
+}
+
+impl fmt::Display for Definition {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            Definition::Path(ref p) => p.display().fmt(f),
+        }
+    }
+}
+
 fn homedir(cwd: &Path) -> Option<PathBuf> {
     let cargo_home = env::var_os("CARGO_HOME").map(|home| {
         cwd.join(home)