itsscb/cargo
1
0
Fork 0
mirror of https://github.com/rust-lang/cargo.git synced 2025-09-28 11:20:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improvements to StringList config handling.

Browse Source
This commit is contained in:
Eric Huss 2020-02-16 15:29:59 -08:00
parent 5f07972103
commit fd258634c9
6 changed files with 162 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/cargo/util/config/de.rs
View File

@ -176,11 +176,28 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
visitor.visit_seq(ConfigSeqAccess::new(self)?)
}
fn deserialize_newtype_struct<V>(
self,
name: &'static str,
visitor: V,
) -> Result<V::Value, Self::Error>
where
V: de::Visitor<'de>,
{
if name == "StringList" {
let vals = self.config.get_list_or_string(&self.key)?;
let vals: Vec<String> = 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(),

102
src/cargo/util/config/mod.rs
View File

@ -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<Vec<(String, Definition)>> {
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<HashMap<String, CV>>`.
///
/// 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<String>,
}
#[derive(Debug, Deserialize)]
pub struct StringList(Vec<String>);
impl StringList {
pub fn as_slice(&self) -> &[String] {
&self.list
}
}
impl<'de> serde::de::Deserialize<'de> for StringList {
fn deserialize<D: serde::de::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
#[derive(Deserialize)]
#[serde(untagged)]
enum Target {
String(String),
List(Vec<String>),
}
// 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
}
}

2
src/cargo/util/config/path.rs
View File

@ -56,7 +56,7 @@ impl<'de> serde::Deserialize<'de> for PathAndArgs {
D: serde::Deserializer<'de>,
{
let vsl = Value::<StringList>::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"));
}

5
tests/testsuite/bad_config.rs
View File

@ -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();

53
tests/testsuite/config.rs
View File

@ -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::<StringList>("key1").unwrap();
assert_eq!(x.as_slice(), &["123".to_string()]);
let x = config.get::<StringList>("key2").unwrap();
assert_eq!(x.as_slice(), &["true".to_string()]);
let x = config.get::<StringList>("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::<StringList>("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::<StringList>("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::<StringList>("key1").unwrap();
assert_eq!(x.as_slice(), &[] as &[String]);
let x = config.get::<StringList>("key2").unwrap();
assert_eq!(x.as_slice(), &["1 2".to_string(), "3".to_string()]);
assert_error(
config.get::<StringList>("key3").unwrap_err(),
"error in environment variable `CARGO_KEY3`: expected string, found integer",
);
}

19
tests/testsuite/tool_paths.rs
View File

@ -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();