Auto merge of #2398 - alexcrichton:config-env-var, r=brson

This commit adds a more principled system to rationalize what ends up being a
configuration value versus an environment variable. This problem is solved by
just saying that they're one and the same! Similar to Bundler, this commit
supports overriding the `foo.bar` configuration value with the `CARGO_FOO_BAR`
environment variable.

Currently this is used as part of the `get_string` and `get_i64` methods on
`Config`. This means, for example, that the following environment variables can
now be used to configure Cargo:

* CARGO_BUILD_JOBS
* CARGO_HTTP_TIMEOUT
* CARGO_HTTP_PROXY

Currently it's not supported to encode a list in an environment variable, so for
example `CARGO_PATHS` would not be read when reading the global `paths`
configuration value.

cc #2362
cc #2395 -- intended to close this in tandem with #2397

Author: bors

src/cargo/ops/cargo_compile.rs

@@ -32,7 +32,7 @@ use core::{Source, SourceId, SourceMap, PackageSet, Package, Target};
 use core::{Profile, TargetKind, Profiles};
 use core::resolver::{Method, Resolve};
 use ops::{self, BuildOutput, ExecEngine};
-use util::config::{ConfigValue, Config};
+use util::config::Config;
 use util::{CargoResult, internal, ChainError, profile};
 
 /// Contains information about how a package should be compiled.
@@ -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,40 +476,39 @@ 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() {
-                ConfigValue::String(v, path) => {
-                    if k == "rustc-flags" {
-                        let whence = format!("in `{}` (in {})", key,
-                                             path.display());
-                        let (paths, links) = try!(
-                            BuildOutput::parse_rustc_flags(&v, &whence)
-                        );
-                        output.library_paths.extend(paths.into_iter());
-                        output.library_links.extend(links.into_iter());
-                    } else {
-                        output.metadata.push((k, v));
-                    }
-                },
-                ConfigValue::List(a, p) => {
-                    if k == "rustc-link-lib" {
-                        output.library_links.extend(a.into_iter().map(|v| v.0));
-                    } else if k == "rustc-link-search" {
-                        output.library_paths.extend(a.into_iter().map(|v| {
-                            PathBuf::from(&v.0)
-                        }));
-                    } else if k == "rustc-cfg" {
-                        output.cfgs.extend(a.into_iter().map(|v| v.0));
-                    } else {
-                        try!(config.expected("string", &k,
-                                             ConfigValue::List(a, p)));
-                    }
-                },
-                // technically could be a list too, but that's the exception to
-                // the rule...
-                cv => { try!(config.expected("string", &k, cv)); }
+            match &k[..] {
+                "rustc-flags" => {
+                    let flags = try!(config.get_string(&key)).unwrap();
+                    let whence = format!("in `{}` (in {})", key,
+                                         flags.definition);
+                    let (paths, links) = try!(
+                        BuildOutput::parse_rustc_flags(&flags.val, &whence)
+                    );
+                    output.library_paths.extend(paths.into_iter());
+                    output.library_links.extend(links.into_iter());
+                }
+                "rustc-link-lib" => {
+                    let list = try!(config.get_list(&key)).unwrap();
+                    output.library_links.extend(list.val.into_iter()
+                                                        .map(|v| v.0));
+                }
+                "rustc-link-search" => {
+                    let list = try!(config.get_list(&key)).unwrap();
+                    output.library_paths.extend(list.val.into_iter().map(|v| {
+                        PathBuf::from(&v.0)
+                    }));
+                }
+                "rustc-cfg" => {
+                    let list = try!(config.get_list(&key)).unwrap();
+                    output.cfgs.extend(list.val.into_iter().map(|v| v.0));
+                }
+                _ => {
+                    let val = try!(config.get_string(&key)).unwrap();
+                    output.metadata.push((k, val.val));
+                }
             }
         }
         ret.overrides.insert(lib_name, output);

src/cargo/ops/cargo_install.rs

@@ -347,11 +347,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()
     }))