diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 23d0b1b3ccd..6fddc7e71f0 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -473,7 +473,13 @@ impl<'de, 'config> de::MapAccess<'de> for ValueDeserializer<'config> { Definition::Environment(env) => { seed.deserialize(Tuple2Deserializer(1i32, env.as_str())) } - Definition::Cli => seed.deserialize(Tuple2Deserializer(2i32, "")), + Definition::Cli(path) => { + let str = path + .as_ref() + .map(|p| p.to_string_lossy()) + .unwrap_or_default(); + seed.deserialize(Tuple2Deserializer(2i32, str)) + } } } } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index df5e0373af5..765c93db704 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -121,6 +121,20 @@ macro_rules! get_value_typed { }; } +/// Indicates why a config value is being loaded. +#[derive(Clone, Copy, Debug)] +enum WhyLoad { + /// Loaded due to a request from the global cli arg `--config` + /// + /// Indirect configs loaded via [`config-include`] are also seen as from cli args, + /// if the initial config is being loaded from cli. + /// + /// [`config-include`]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#config-include + Cli, + /// Loaded due to config file discovery. + FileDiscovery, +} + /// Configuration information for cargo. This is not specific to a build, it is information /// relating to cargo itself. #[derive(Debug)] @@ -1005,12 +1019,15 @@ impl Config { self.load_values_from(&self.cwd) } + /// Like [`load_values`](Config::load_values) but without merging config values. + /// + /// This is primarily crafted for `cargo config` command. pub(crate) fn load_values_unmerged(&self) -> CargoResult> { let mut result = Vec::new(); let mut seen = HashSet::new(); let home = self.home_path.clone().into_path_unlocked(); self.walk_tree(&self.cwd, &home, |path| { - let mut cv = self._load_file(path, &mut seen, false)?; + let mut cv = self._load_file(path, &mut seen, false, WhyLoad::FileDiscovery)?; if self.cli_unstable().config_include { self.load_unmerged_include(&mut cv, &mut seen, &mut result)?; } @@ -1021,6 +1038,9 @@ impl Config { Ok(result) } + /// Like [`load_includes`](Config::load_includes) but without merging config values. + /// + /// This is primarily crafted for `cargo config` command. fn load_unmerged_include( &self, cv: &mut CV, @@ -1029,15 +1049,18 @@ impl Config { ) -> CargoResult<()> { let includes = self.include_paths(cv, false)?; for (path, abs_path, def) in includes { - let mut cv = self._load_file(&abs_path, seen, false).with_context(|| { - format!("failed to load config include `{}` from `{}`", path, def) - })?; + let mut cv = self + ._load_file(&abs_path, seen, false, WhyLoad::FileDiscovery) + .with_context(|| { + format!("failed to load config include `{}` from `{}`", path, def) + })?; self.load_unmerged_include(&mut cv, seen, output)?; output.push(cv); } Ok(()) } + /// Start a config file discovery from a path and merges all config values found. fn load_values_from(&self, path: &Path) -> CargoResult> { // This definition path is ignored, this is just a temporary container // representing the entire file. @@ -1045,7 +1068,7 @@ impl Config { let home = self.home_path.clone().into_path_unlocked(); self.walk_tree(path, &home, |path| { - let value = self.load_file(path, true)?; + let value = self.load_file(path)?; cfg.merge(value, false).with_context(|| { format!("failed to merge configuration at `{}`", path.display()) })?; @@ -1059,15 +1082,28 @@ impl Config { } } - fn load_file(&self, path: &Path, includes: bool) -> CargoResult { - self._load_file(path, &mut HashSet::new(), includes) + /// Loads a config value from a path. + /// + /// This is used during config file discovery. + fn load_file(&self, path: &Path) -> CargoResult { + self._load_file(path, &mut HashSet::new(), true, WhyLoad::FileDiscovery) } + /// Loads a config value from a path with options. + /// + /// This is actual implementation of loading a config value from a path. + /// + /// * `includes` determines whether to load configs from [`config-include`]. + /// * `seen` is used to check for cyclic includes. + /// * `why_load` tells why a config is being loaded. + /// + /// [`config-include`]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#config-include fn _load_file( &self, path: &Path, seen: &mut HashSet, includes: bool, + why_load: WhyLoad, ) -> CargoResult { if !seen.insert(path.to_path_buf()) { bail!( @@ -1080,15 +1116,18 @@ impl Config { let toml = cargo_toml::parse(&contents, path, self).with_context(|| { format!("could not parse TOML configuration in `{}`", path.display()) })?; - let value = - CV::from_toml(Definition::Path(path.to_path_buf()), toml).with_context(|| { - format!( - "failed to load TOML configuration from `{}`", - path.display() - ) - })?; + let def = match why_load { + WhyLoad::Cli => Definition::Cli(Some(path.into())), + WhyLoad::FileDiscovery => Definition::Path(path.into()), + }; + let value = CV::from_toml(def, toml).with_context(|| { + format!( + "failed to load TOML configuration from `{}`", + path.display() + ) + })?; if includes { - self.load_includes(value, seen) + self.load_includes(value, seen, why_load) } else { Ok(value) } @@ -1098,8 +1137,14 @@ impl Config { /// /// Returns `value` with the given include files merged into it. /// - /// `seen` is used to check for cyclic includes. - fn load_includes(&self, mut value: CV, seen: &mut HashSet) -> CargoResult { + /// * `seen` is used to check for cyclic includes. + /// * `why_load` tells why a config is being loaded. + fn load_includes( + &self, + mut value: CV, + seen: &mut HashSet, + why_load: WhyLoad, + ) -> CargoResult { // Get the list of files to load. let includes = self.include_paths(&mut value, true)?; // Check unstable. @@ -1109,7 +1154,7 @@ impl Config { // Accumulate all values here. let mut root = CV::Table(HashMap::new(), value.definition().clone()); for (path, abs_path, def) in includes { - self._load_file(&abs_path, seen, true) + self._load_file(&abs_path, seen, true, why_load) .and_then(|include| root.merge(include, true)) .with_context(|| { format!("failed to load config include `{}` from `{}`", path, def) @@ -1127,8 +1172,8 @@ impl Config { ) -> CargoResult> { let abs = |path: &str, def: &Definition| -> (String, PathBuf, Definition) { let abs_path = match def { - Definition::Path(p) => p.parent().unwrap().join(&path), - Definition::Environment(_) | Definition::Cli => self.cwd().join(&path), + Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap().join(&path), + Definition::Environment(_) | Definition::Cli(None) => self.cwd().join(&path), }; (path.to_string(), abs_path, def.clone()) }; @@ -1162,7 +1207,7 @@ impl Config { /// Parses the CLI config args and returns them as a table. pub(crate) fn cli_args_as_table(&self) -> CargoResult { - let mut loaded_args = CV::Table(HashMap::new(), Definition::Cli); + let mut loaded_args = CV::Table(HashMap::new(), Definition::Cli(None)); let cli_args = match &self.cli_config { Some(cli_args) => cli_args, None => return Ok(loaded_args), @@ -1178,7 +1223,7 @@ impl Config { anyhow::format_err!("config path {:?} is not utf-8", arg_as_path) })? .to_string(); - self._load_file(&self.cwd().join(&str_path), &mut seen, true) + self._load_file(&self.cwd().join(&str_path), &mut seen, true, WhyLoad::Cli) .with_context(|| format!("failed to load config from `{}`", str_path))? } else { // We only want to allow "dotted key" (see https://toml.io/en/v1.0.0#keys) @@ -1273,11 +1318,11 @@ impl Config { ); } - CV::from_toml(Definition::Cli, toml_v) + CV::from_toml(Definition::Cli(None), toml_v) .with_context(|| format!("failed to convert --config argument `{arg}`"))? }; let tmp_table = self - .load_includes(tmp_table, &mut HashSet::new()) + .load_includes(tmp_table, &mut HashSet::new(), WhyLoad::Cli) .with_context(|| "failed to load --config include".to_string())?; loaded_args .merge(tmp_table, true) @@ -1431,7 +1476,7 @@ impl Config { None => return Ok(()), }; - let mut value = self.load_file(&credentials, true)?; + let mut value = self.load_file(&credentials)?; // Backwards compatibility for old `.cargo/credentials` layout. { let (value_map, def) = match value { diff --git a/src/cargo/util/config/value.rs b/src/cargo/util/config/value.rs index 65b0bffe46b..a70d75a07c5 100644 --- a/src/cargo/util/config/value.rs +++ b/src/cargo/util/config/value.rs @@ -59,7 +59,8 @@ pub enum Definition { /// Defined in an environment variable, includes the environment key. Environment(String), /// Passed in on the command line. - Cli, + /// A path is attached when the config value is a path to a config file. + Cli(Option), } impl Definition { @@ -69,8 +70,8 @@ impl Definition { /// CLI and env are the current working directory. pub fn root<'a>(&'a self, config: &'a Config) -> &'a Path { match self { - Definition::Path(p) => p.parent().unwrap().parent().unwrap(), - Definition::Environment(_) | Definition::Cli => config.cwd(), + Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap().parent().unwrap(), + Definition::Environment(_) | Definition::Cli(None) => config.cwd(), } } @@ -80,8 +81,8 @@ impl Definition { pub fn is_higher_priority(&self, other: &Definition) -> bool { matches!( (self, other), - (Definition::Cli, Definition::Environment(_)) - | (Definition::Cli, Definition::Path(_)) + (Definition::Cli(_), Definition::Environment(_)) + | (Definition::Cli(_), Definition::Path(_)) | (Definition::Environment(_), Definition::Path(_)) ) } @@ -100,9 +101,9 @@ impl PartialEq for Definition { impl fmt::Display for Definition { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Definition::Path(p) => p.display().fmt(f), + Definition::Path(p) | Definition::Cli(Some(p)) => p.display().fmt(f), Definition::Environment(key) => write!(f, "environment variable `{}`", key), - Definition::Cli => write!(f, "--config cli option"), + Definition::Cli(None) => write!(f, "--config cli option"), } } } @@ -218,8 +219,11 @@ impl<'de> de::Deserialize<'de> for Definition { match discr { 0 => Ok(Definition::Path(value.into())), 1 => Ok(Definition::Environment(value)), - 2 => Ok(Definition::Cli), - _ => panic!("unexpected discriminant {} value {}", discr, value), + 2 => { + let path = (value.len() > 0).then_some(value.into()); + Ok(Definition::Cli(path)) + } + _ => panic!("unexpected discriminant {discr} value {value}"), } } } diff --git a/tests/testsuite/config_cli.rs b/tests/testsuite/config_cli.rs index e4c593443e7..27b5cf9afba 100644 --- a/tests/testsuite/config_cli.rs +++ b/tests/testsuite/config_cli.rs @@ -1,6 +1,8 @@ //! Tests for the --config CLI option. -use super::config::{assert_error, assert_match, read_output, write_config, ConfigBuilder}; +use super::config::{ + assert_error, assert_match, read_output, write_config, write_config_at, ConfigBuilder, +}; use cargo::util::config::Definition; use cargo_test_support::paths; use std::{collections::HashMap, fs}; @@ -53,6 +55,72 @@ fn cli_priority() { assert_eq!(config.get::("term.quiet").unwrap(), true); } +#[cargo_test] +fn merge_primitives_for_multiple_cli_occurences() { + let config_path0 = ".cargo/file0.toml"; + write_config_at(config_path0, "k = 'file0'"); + let config_path1 = ".cargo/file1.toml"; + write_config_at(config_path1, "k = 'file1'"); + + // k=env0 + let config = ConfigBuilder::new().env("CARGO_K", "env0").build(); + assert_eq!(config.get::("k").unwrap(), "env0"); + + // k=env0 + // --config k='cli0' + // --config k='cli1' + let config = ConfigBuilder::new() + .env("CARGO_K", "env0") + .config_arg("k='cli0'") + .config_arg("k='cli1'") + .build(); + assert_eq!(config.get::("k").unwrap(), "cli1"); + + // Env has a lower priority when comparing with file from CLI arg. + // + // k=env0 + // --config k='cli0' + // --config k='cli1' + // --config .cargo/file0.toml + let config = ConfigBuilder::new() + .env("CARGO_K", "env0") + .config_arg("k='cli0'") + .config_arg("k='cli1'") + .config_arg(config_path0) + .build(); + assert_eq!(config.get::("k").unwrap(), "file0"); + + // k=env0 + // --config k='cli0' + // --config k='cli1' + // --config .cargo/file0.toml + // --config k='cli2' + let config = ConfigBuilder::new() + .env("CARGO_K", "env0") + .config_arg("k='cli0'") + .config_arg("k='cli1'") + .config_arg(config_path0) + .config_arg("k='cli2'") + .build(); + assert_eq!(config.get::("k").unwrap(), "cli2"); + + // k=env0 + // --config k='cli0' + // --config k='cli1' + // --config .cargo/file0.toml + // --config k='cli2' + // --config .cargo/file1.toml + let config = ConfigBuilder::new() + .env("CARGO_K", "env0") + .config_arg("k='cli0'") + .config_arg("k='cli1'") + .config_arg(config_path0) + .config_arg("k='cli2'") + .config_arg(config_path1) + .build(); + assert_eq!(config.get::("k").unwrap(), "file1"); +} + #[cargo_test] fn merges_array() { // Array entries are appended. diff --git a/tests/testsuite/config_include.rs b/tests/testsuite/config_include.rs index f605d087e88..b2d0ddf66c6 100644 --- a/tests/testsuite/config_include.rs +++ b/tests/testsuite/config_include.rs @@ -254,3 +254,31 @@ Caused by: expected array, but found string", ); } + +#[cargo_test] +fn cli_include_take_priority_over_env() { + write_config_at(".cargo/include.toml", "k='include'"); + + // k=env + let config = ConfigBuilder::new().env("CARGO_K", "env").build(); + assert_eq!(config.get::("k").unwrap(), "env"); + + // k=env + // --config 'include=".cargo/include.toml"' + let config = ConfigBuilder::new() + .env("CARGO_K", "env") + .unstable_flag("config-include") + .config_arg("include='.cargo/include.toml'") + .build(); + assert_eq!(config.get::("k").unwrap(), "include"); + + // k=env + // --config '.cargo/foo.toml' + write_config_at(".cargo/foo.toml", "include='include.toml'"); + let config = ConfigBuilder::new() + .env("CARGO_K", "env") + .unstable_flag("config-include") + .config_arg(".cargo/foo.toml") + .build(); + assert_eq!(config.get::("k").unwrap(), "include"); +}