From 4f4e3236bb77458146dafd20cb2e0b59ffe372e0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 28 Aug 2014 21:38:35 -0700 Subject: [PATCH] Fix relative override paths Relative paths are now considered relative to the directory containing the `.cargo/config` file specifying the relative path. I also merge ConfigValueValue into ConfigValue by moving the Path directly next to the String that defined it so we can track which strings came from which paths. Closes #458 --- src/cargo/ops/cargo_compile.rs | 28 +++--- src/cargo/util/config.rs | 126 ++++++++++---------------- tests/test_cargo_compile_path_deps.rs | 37 +++++++- 3 files changed, 100 insertions(+), 91 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 8b686af2d..8f16ced77 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -137,18 +137,24 @@ fn source_ids_from_config(configs: &HashMap, cur_path: Path) -> CargoResult> { debug!("loaded config; configs={}", configs); - let config_paths = configs.find_equiv(&"paths").map(|v| v.clone()); - let config_paths = config_paths.unwrap_or_else(|| ConfigValue::new()); - + let config_paths = match configs.find_equiv(&"paths") { + Some(cfg) => cfg, + None => return Ok(Vec::new()) + }; let paths = try!(config_paths.list().chain_error(|| { - internal("invalid configuration for the key `path`") + internal("invalid configuration for the key `paths`") })); - // Make sure we don't override the local package, even if it's in the list - // of override paths - paths.iter().filter(|p| { - cur_path != os::make_absolute(&Path::new(p.as_slice())) - }).map(|p| SourceId::for_path(&Path::new(p.as_slice()))).collect() + paths.iter().map(|&(ref s, ref p)| { + // The path listed next to the string is the config file in which the + // key was located, so we want to pop off the `.cargo/config` component + // to get the directory containing the `.cargo` folder. + p.dir_path().dir_path().join(s.as_slice()) + }).filter(|p| { + // Make sure we don't override the local package, even if it's in the + // list of override paths. + cur_path != *p + }).map(|p| SourceId::for_path(&p)).collect() } fn scrape_target_config(config: &mut Config, @@ -176,7 +182,7 @@ fn scrape_target_config(config: &mut Config, Some(ar) => { config.set_ar(try!(ar.string().chain_error(|| { internal("invalid configuration for key `ar`") - })).to_string()); + })).ref0().to_string()); } } @@ -185,7 +191,7 @@ fn scrape_target_config(config: &mut Config, Some(linker) => { config.set_linker(try!(linker.string().chain_error(|| { internal("invalid configuration for key `ar`") - })).to_string()); + })).ref0().to_string()); } } diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 6e03115ac..2747c8f8a 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -77,80 +77,72 @@ pub enum Location { } #[deriving(Eq,PartialEq,Clone,Decodable)] -pub enum ConfigValueValue { - String(String), - List(Vec), +pub enum ConfigValue { + String(String, Path), + List(Vec<(String, Path)>), Table(HashMap), } -impl fmt::Show for ConfigValueValue { +impl fmt::Show for ConfigValue { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { - String(ref string) => write!(f, "{}", string), - List(ref list) => write!(f, "{}", list), + String(ref string, ref path) => { + write!(f, "{} (from {})", string, path.display()) + } + List(ref list) => { + try!(write!(f, "[")); + for (i, &(ref s, ref path)) in list.iter().enumerate() { + if i > 0 { try!(write!(f, ", ")); } + try!(write!(f, "{} (from {})", s, path.display())); + } + write!(f, "]") + } Table(ref table) => write!(f, "{}", table), } } } -impl> Encodable for ConfigValueValue { +impl> Encodable for ConfigValue { fn encode(&self, s: &mut S) -> Result<(), E> { match *self { - String(ref string) => string.encode(s), - List(ref list) => list.encode(s), + String(ref string, _) => string.encode(s), + List(ref list) => { + let list: Vec<&String> = list.iter().map(|s| s.ref0()).collect(); + list.encode(s) + } Table(ref table) => table.encode(s), } } } -#[deriving(Eq,PartialEq,Clone,Decodable)] -pub struct ConfigValue { - value: ConfigValueValue, - path: Vec -} - impl ConfigValue { - pub fn new() -> ConfigValue { - ConfigValue { value: List(vec!()), path: vec!() } - } - - pub fn get_value(&self) -> &ConfigValueValue { - &self.value - } - fn from_toml(path: &Path, toml: toml::Value) -> CargoResult { - let value = match toml { - toml::String(val) => String(val), + match toml { + toml::String(val) => Ok(String(val, path.clone())), toml::Array(val) => { - List(try!(result::collect(val.move_iter().map(|toml| { + Ok(List(try!(result::collect(val.move_iter().map(|toml| { match toml { - toml::String(val) => Ok(val), + toml::String(val) => Ok((val, path.clone())), _ => Err(internal("")), } - })))) + }))))) } toml::Table(val) => { - Table(try!(result::collect(val.move_iter().map(|(key, value)| { + Ok(Table(try!(result::collect(val.move_iter().map(|(key, value)| { let value = raw_try!(ConfigValue::from_toml(path, value)); Ok((key, value)) - })))) + }))))) } _ => return Err(internal("")) - }; - - Ok(ConfigValue { value: value, path: vec![path.clone()] }) + } } fn merge(&mut self, from: ConfigValue) -> CargoResult<()> { - let ConfigValue { value, path } = from; - match (&mut self.value, value) { - (&String(ref mut old), String(ref mut new)) => { - mem::swap(old, new); - self.path = path; - } + match (self, from) { + (me @ &String(..), from @ String(..)) => *me = from, (&List(ref mut old), List(ref mut new)) => { - old.extend(mem::replace(new, Vec::new()).move_iter()); - self.path.extend(path.move_iter()); + let new = mem::replace(new, Vec::new()); + old.extend(new.move_iter()); } (&Table(ref mut old), Table(ref mut new)) => { let new = mem::replace(new, HashMap::new()); @@ -161,7 +153,6 @@ impl ConfigValue { |_, new| new); try!(err); } - self.path.extend(path.move_iter()); } (expected, found) => { return Err(internal(format!("expected {}, but found {}", @@ -172,33 +163,31 @@ impl ConfigValue { Ok(()) } - pub fn string(&self) -> CargoResult<&str> { - match self.value { - Table(_) => Err(internal("expected a string, but found a table")), - List(_) => Err(internal("expected a string, but found a list")), - String(ref s) => Ok(s.as_slice()), + pub fn string(&self) -> CargoResult<(&str, &Path)> { + match *self { + Table(..) => Err(internal("expected a string, but found a table")), + List(..) => Err(internal("expected a string, but found a list")), + String(ref s, ref p) => Ok((s.as_slice(), p)), } } pub fn table(&self) -> CargoResult<&HashMap> { - match self.value { - String(_) => Err(internal("expected a table, but found a string")), - List(_) => Err(internal("expected a table, but found a list")), + match *self { + String(..) => Err(internal("expected a table, but found a string")), + List(..) => Err(internal("expected a table, but found a list")), Table(ref table) => Ok(table), } } - pub fn list(&self) -> CargoResult<&[String]> { - match self.value { - String(_) => Err(internal("expected a list, but found a string")), - Table(_) => Err(internal("expected a list, but found a table")), + pub fn list(&self) -> CargoResult<&[(String, Path)]> { + match *self { + String(..) => Err(internal("expected a list, but found a string")), + Table(..) => Err(internal("expected a list, but found a table")), List(ref list) => Ok(list.as_slice()), } } -} -impl ConfigValueValue { - fn desc(&self) -> &'static str { + pub fn desc(&self) -> &'static str { match *self { Table(..) => "table", List(..) => "array", @@ -207,32 +196,13 @@ impl ConfigValueValue { } } -impl> Encodable for ConfigValue { - fn encode(&self, s: &mut S) -> Result<(), E> { - s.emit_map(2, |s| { - raw_try!(s.emit_map_elt_key(0, |s| "value".encode(s))); - raw_try!(s.emit_map_elt_val(0, |s| self.value.encode(s))); - Ok(()) - }) - } -} - -impl fmt::Show for ConfigValue { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let paths: Vec = self.path.iter().map(|p| { - p.display().to_string() - }).collect(); - write!(f, "{} (from {})", self.value, paths) - } -} - pub fn get_config(pwd: Path, key: &str) -> CargoResult { find_in_tree(&pwd, |file| extract_config(file, key)).map_err(|_| human(format!("`{}` not found in your configuration", key))) } pub fn all_configs(pwd: Path) -> CargoResult> { - let mut cfg = ConfigValue { value: Table(HashMap::new()), path: Vec::new() }; + let mut cfg = Table(HashMap::new()); try!(walk_tree(&pwd, |mut file| { let path = file.path().clone(); @@ -247,7 +217,7 @@ pub fn all_configs(pwd: Path) -> CargoResult> { }).map_err(|_| human("Couldn't load Cargo configuration"))); - match cfg.value { + match cfg { Table(map) => Ok(map), _ => unreachable!(), } diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index 4385826c8..687eebcb8 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -1,8 +1,8 @@ -use std::io::File; +use std::io::{fs, File, UserRWX}; use support::{ResultTest, project, execs, main_file, cargo_dir, path2url}; use support::{COMPILING, FRESH, RUNNING}; -use support::paths::PathExt; +use support::paths::{mod, PathExt}; use hamcrest::{assert_that, existing_file}; use cargo; use cargo::util::{process}; @@ -534,6 +534,39 @@ test!(error_message_for_missing_manifest { }) +test!(override_relative { + let bar = project("bar") + .file("Cargo.toml", r#" + [package] + + name = "bar" + version = "0.5.0" + authors = ["wycats@example.com"] + "#) + .file("src/lib.rs", ""); + + fs::mkdir(&paths::root().join(".cargo"), UserRWX).assert(); + File::create(&paths::root().join(".cargo/config")).write_str(r#" + paths = ["bar"] + "#).assert(); + + let p = project("foo") + .file("Cargo.toml", format!(r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + path = '{}' + "#, bar.root().display())) + .file("src/lib.rs", ""); + bar.build(); + assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0)); + +}) + test!(override_self { let bar = project("bar") .file("Cargo.toml", r#" -- 2.39.5