diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 32e8ee6d93a..33a5320f66a 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -176,11 +176,28 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> { visitor.visit_seq(ConfigSeqAccess::new(self)?) } + fn deserialize_newtype_struct( + self, + name: &'static str, + visitor: V, + ) -> Result + where + V: de::Visitor<'de>, + { + if name == "StringList" { + let vals = self.config.get_list_or_string(&self.key)?; + let vals: Vec = vals.into_iter().map(|vd| vd.0).collect(); + visitor.visit_newtype_struct(vals.into_deserializer()) + } else { + visitor.visit_newtype_struct(self) + } + } + // These aren't really supported, yet. serde::forward_to_deserialize_any! { f32 f64 char str bytes byte_buf unit unit_struct - enum identifier ignored_any newtype_struct + enum identifier ignored_any } } @@ -345,36 +362,7 @@ impl ConfigSeqAccess { res.extend(v.val); } - // Check environment. - if let Some(v) = de.config.env.get(de.key.as_env_key()) { - let def = Definition::Environment(de.key.as_env_key().to_string()); - if de.config.cli_unstable().advanced_env && v.starts_with('[') && v.ends_with(']') { - // Parse an environment string as a TOML array. - let toml_s = format!("value={}", v); - let toml_v: toml::Value = toml::de::from_str(&toml_s).map_err(|e| { - ConfigError::new(format!("could not parse TOML list: {}", e), def.clone()) - })?; - let values = toml_v - .as_table() - .unwrap() - .get("value") - .unwrap() - .as_array() - .expect("env var was not array"); - for value in values { - // TODO: support other types. - let s = value.as_str().ok_or_else(|| { - ConfigError::new( - format!("expected string, found {}", value.type_str()), - def.clone(), - ) - })?; - res.push((s.to_string(), def.clone())); - } - } else { - res.extend(v.split_whitespace().map(|s| (s.to_string(), def.clone()))); - } - } + de.config.get_env_list(&de.key, &mut res)?; Ok(ConfigSeqAccess { list_iter: res.into_iter(), diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index bc3d5434c04..73902f6caff 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -573,6 +573,70 @@ impl Config { } } + /// Helper for StringList type to get something that is a string or list. + fn get_list_or_string(&self, key: &ConfigKey) -> CargoResult> { + let mut res = Vec::new(); + match self.get_cv(&key)? { + Some(CV::List(val, _def)) => res.extend(val), + Some(CV::String(val, def)) => { + let split_vs = val.split_whitespace().map(|s| (s.to_string(), def.clone())); + res.extend(split_vs); + } + Some(val) => { + return self.expected("string or array of strings", key, &val); + } + None => {} + } + + self.get_env_list(key, &mut res)?; + Ok(res) + } + + /// Internal method for getting an environment variable as a list. + fn get_env_list( + &self, + key: &ConfigKey, + output: &mut Vec<(String, Definition)>, + ) -> CargoResult<()> { + let env_val = match self.env.get(key.as_env_key()) { + Some(v) => v, + None => return Ok(()), + }; + + let def = Definition::Environment(key.as_env_key().to_string()); + if self.cli_unstable().advanced_env && env_val.starts_with('[') && env_val.ends_with(']') { + // Parse an environment string as a TOML array. + let toml_s = format!("value={}", env_val); + let toml_v: toml::Value = toml::de::from_str(&toml_s).map_err(|e| { + ConfigError::new(format!("could not parse TOML list: {}", e), def.clone()) + })?; + let values = toml_v + .as_table() + .unwrap() + .get("value") + .unwrap() + .as_array() + .expect("env var was not array"); + for value in values { + // TODO: support other types. + let s = value.as_str().ok_or_else(|| { + ConfigError::new( + format!("expected string, found {}", value.type_str()), + def.clone(), + ) + })?; + output.push((s.to_string(), def.clone())); + } + } else { + output.extend( + env_val + .split_whitespace() + .map(|s| (s.to_string(), def.clone())), + ); + } + Ok(()) + } + /// Low-level method for getting a config value as a `OptValue>`. /// /// NOTE: This does not read from env. The caller is responsible for that. @@ -1650,43 +1714,11 @@ pub struct CargoBuildConfig { /// a = 'a b c' /// b = ['a', 'b', 'c'] /// ``` -#[derive(Debug)] -pub struct StringList { - list: Vec, -} +#[derive(Debug, Deserialize)] +pub struct StringList(Vec); impl StringList { pub fn as_slice(&self) -> &[String] { - &self.list - } -} - -impl<'de> serde::de::Deserialize<'de> for StringList { - fn deserialize>(d: D) -> Result { - #[derive(Deserialize)] - #[serde(untagged)] - enum Target { - String(String), - List(Vec), - } - - // Add some context to the error. Serde gives a vague message for - // untagged enums. See https://github.com/serde-rs/serde/issues/773 - let result = match Target::deserialize(d) { - Ok(r) => r, - Err(e) => { - return Err(serde::de::Error::custom(format!( - "failed to deserialize, expected a string or array of strings: {}", - e - ))) - } - }; - - Ok(match result { - Target::String(s) => StringList { - list: s.split_whitespace().map(str::to_string).collect(), - }, - Target::List(list) => StringList { list }, - }) + &self.0 } } diff --git a/src/cargo/util/config/path.rs b/src/cargo/util/config/path.rs index b1cab15ab0b..030ff085c35 100644 --- a/src/cargo/util/config/path.rs +++ b/src/cargo/util/config/path.rs @@ -56,7 +56,7 @@ impl<'de> serde::Deserialize<'de> for PathAndArgs { D: serde::Deserializer<'de>, { let vsl = Value::::deserialize(deserializer)?; - let mut strings = vsl.val.list; + let mut strings = vsl.val.0; if strings.is_empty() { return Err(D::Error::invalid_length(0, &"at least one element")); } diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 75584089c17..cb54a6c9c89 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -1355,8 +1355,9 @@ Caused by: could not load config key `target.cfg(not(target_os = \"none\")).runner` Caused by: - failed to deserialize, expected a string or array of strings: \ - data did not match any variant of untagged enum Target + invalid configuration for key `target.cfg(not(target_os = \"none\")).runner` +expected a string or array of strings, but found a boolean for \ +`target.cfg(not(target_os = \"none\")).runner` in [..]/foo/.cargo/config ", ) .run(); diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 61d24a881e6..46dbca69d50 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1205,3 +1205,56 @@ fn overlapping_env_config() { assert_eq!(s.debug_assertions, Some(true)); assert_eq!(s.debug, Some(1)); } + +#[cargo_test] +fn string_list_tricky_env() { + // Make sure StringList handles typed env values. + let config = ConfigBuilder::new() + .env("CARGO_KEY1", "123") + .env("CARGO_KEY2", "true") + .env("CARGO_KEY3", "1 2") + .build(); + let x = config.get::("key1").unwrap(); + assert_eq!(x.as_slice(), &["123".to_string()]); + let x = config.get::("key2").unwrap(); + assert_eq!(x.as_slice(), &["true".to_string()]); + let x = config.get::("key3").unwrap(); + assert_eq!(x.as_slice(), &["1".to_string(), "2".to_string()]); +} + +#[cargo_test] +fn string_list_wrong_type() { + // What happens if StringList is given then wrong type. + write_config("some_list = 123"); + let config = ConfigBuilder::new().build(); + assert_error( + config.get::("some_list").unwrap_err(), + "\ +invalid configuration for key `some_list` +expected a string or array of strings, but found a integer for `some_list` in [..]/.cargo/config", + ); + + write_config("some_list = \"1 2\""); + let config = ConfigBuilder::new().build(); + let x = config.get::("some_list").unwrap(); + assert_eq!(x.as_slice(), &["1".to_string(), "2".to_string()]); +} + +#[cargo_test] +fn string_list_advanced_env() { + // StringList with advanced env. + let config = ConfigBuilder::new() + .unstable_flag("advanced-env") + .env("CARGO_KEY1", "[]") + .env("CARGO_KEY2", "['1 2', '3']") + .env("CARGO_KEY3", "[123]") + .build(); + let x = config.get::("key1").unwrap(); + assert_eq!(x.as_slice(), &[] as &[String]); + let x = config.get::("key2").unwrap(); + assert_eq!(x.as_slice(), &["1 2".to_string(), "3".to_string()]); + assert_error( + config.get::("key3").unwrap_err(), + "error in environment variable `CARGO_KEY3`: expected string, found integer", + ); +} diff --git a/tests/testsuite/tool_paths.rs b/tests/testsuite/tool_paths.rs index 3891f252855..ad7f2bc9573 100644 --- a/tests/testsuite/tool_paths.rs +++ b/tests/testsuite/tool_paths.rs @@ -278,6 +278,25 @@ fn custom_runner_env() { .run(); } +#[cargo_test] +#[cfg(unix)] // Assumes `true` is in PATH. +fn custom_runner_env_true() { + // Check for a bug where "true" was interpreted as a boolean instead of + // the executable. + let target = rustc_host(); + let p = project().file("src/main.rs", "fn main() {}").build(); + + let key = format!( + "CARGO_TARGET_{}_RUNNER", + target.to_uppercase().replace('-', "_") + ); + + p.cargo("run") + .env(&key, "true") + .with_stderr_contains("[RUNNING] `true target/debug/foo[EXE]`") + .run(); +} + #[cargo_test] fn custom_linker_env() { let target = rustc_host();