diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index b7124992c4d..8c989c2b83a 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -1257,11 +1257,15 @@ impl GlobalContext { output: &mut Vec, ) -> CargoResult<()> { let includes = self.include_paths(cv, false)?; - for (path, abs_path, def) in includes { + for include in includes { let mut cv = self - ._load_file(&abs_path, seen, false, WhyLoad::FileDiscovery) + ._load_file(&include.abs_path(self), seen, false, WhyLoad::FileDiscovery) .with_context(|| { - format!("failed to load config include `{}` from `{}`", path, def) + format!( + "failed to load config include `{}` from `{}`", + include.path.display(), + include.def + ) })?; self.load_unmerged_include(&mut cv, seen, output)?; output.push(cv); @@ -1364,11 +1368,15 @@ impl GlobalContext { } // 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, why_load) + for include in includes { + self._load_file(&include.abs_path(self), seen, true, why_load) .and_then(|include| root.merge(include, true)) .with_context(|| { - format!("failed to load config include `{}` from `{}`", path, def) + format!( + "failed to load config include `{}` from `{}`", + include.path.display(), + include.def + ) })?; } root.merge(value, true)?; @@ -1376,46 +1384,43 @@ impl GlobalContext { } /// Converts the `include` config value to a list of absolute paths. - fn include_paths( - &self, - cv: &mut CV, - remove: bool, - ) -> CargoResult> { - let abs = |path: &str, def: &Definition| -> (String, PathBuf, Definition) { - let abs_path = match def { - Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap().join(&path), - Definition::Environment(_) | Definition::Cli(None) | Definition::BuiltIn => { - self.cwd().join(&path) - } - }; - (path.to_string(), abs_path, def.clone()) - }; + fn include_paths(&self, cv: &mut CV, remove: bool) -> CargoResult> { let CV::Table(table, _def) = cv else { unreachable!() }; - let owned; let include = if remove { - owned = table.remove("include"); - owned.as_ref() + table.remove("include").map(Cow::Owned) } else { - table.get("include") + table.get("include").map(Cow::Borrowed) }; - let includes = match include { - Some(CV::String(s, def)) => { - vec![abs(s, def)] - } + let includes = match include.map(|c| c.into_owned()) { + Some(CV::String(s, def)) => vec![ConfigInclude::new(s, def)], Some(CV::List(list, _def)) => list - .iter() - .map(|cv| match cv { - CV::String(s, def) => Ok(abs(s, def)), + .into_iter() + .enumerate() + .map(|(idx, cv)| match cv { + CV::String(s, def) => Ok(ConfigInclude::new(s, def)), + CV::Table(mut table, def) => { + // Extract `include.path` + let s = match table.remove("path") { + Some(CV::String(s, _)) => s, + Some(other) => bail!( + "expected a string, but found {} at `include[{idx}].path` in `{def}`", + other.desc() + ), + None => bail!("missing field `path` at `include[{idx}]` in `{def}`"), + }; + Ok(ConfigInclude::new(s, def)) + } other => bail!( - "`include` expected a string or list of strings, but found {} in list", - other.desc() + "expected a string or table, but found {} at `include[{idx}]` in {}", + other.desc(), + other.definition(), ), }) .collect::>>()?, Some(other) => bail!( - "`include` expected a string or list, but found {} in `{}`", + "expected a string or list of strings, but found {} at `include` in `{}", other.desc(), other.definition() ), @@ -1424,11 +1429,13 @@ impl GlobalContext { } }; - for (path, abs_path, def) in &includes { - if abs_path.extension() != Some(OsStr::new("toml")) { + for include in &includes { + if include.path.extension() != Some(OsStr::new("toml")) { bail!( "expected a config include path ending with `.toml`, \ - but found `{path}` from `{def}`", + but found `{}` from `{}`", + include.path.display(), + include.def, ) } } @@ -1447,14 +1454,10 @@ impl GlobalContext { let arg_as_path = self.cwd.join(arg); let tmp_table = if !arg.is_empty() && arg_as_path.exists() { // --config path_to_file - let str_path = arg_as_path - .to_str() - .ok_or_else(|| { - anyhow::format_err!("config path {:?} is not utf-8", arg_as_path) + self._load_file(&arg_as_path, &mut seen, true, WhyLoad::Cli) + .with_context(|| { + format!("failed to load config from `{}`", arg_as_path.display()) })? - .to_string(); - self._load_file(&self.cwd().join(&str_path), &mut seen, true, WhyLoad::Cli) - .with_context(|| format!("failed to load config from `{}`", str_path))? } else { let doc = toml_dotted_keys(arg)?; let doc: toml::Value = toml::Value::deserialize(doc.into_deserializer()) @@ -2482,6 +2485,44 @@ pub fn save_credentials( } } +/// Represents a config-include value in the configuration. +/// +/// This intentionally doesn't derive serde deserialization +/// to avoid any misuse of `GlobalContext::get::()`, +/// which might lead to wrong config loading order. +struct ConfigInclude { + /// Path to a config-include configuration file. + /// Could be either relative or absolute. + path: PathBuf, + def: Definition, +} + +impl ConfigInclude { + fn new(p: impl Into, def: Definition) -> Self { + Self { + path: p.into(), + def, + } + } + + /// Gets the absolute path of the config-include config file. + /// + /// For file based include, + /// it is relative to parent directory of the config file includes it. + /// For example, if `.cargo/config.toml has a `include = "foo.toml"`, + /// Cargo will load `.cargo/foo.toml`. + /// + /// For CLI based include (e.g., `--config 'include = "foo.toml"'`), + /// it is relative to the current working directory. + fn abs_path(&self, gctx: &GlobalContext) -> PathBuf { + match &self.def { + Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap(), + Definition::Environment(_) | Definition::Cli(None) | Definition::BuiltIn => gctx.cwd(), + } + .join(&self.path) + } +} + #[derive(Debug, Default, Deserialize, PartialEq)] #[serde(rename_all = "kebab-case")] pub struct CargoHttpConfig { diff --git a/tests/testsuite/config_include.rs b/tests/testsuite/config_include.rs index 03e0057c601..21b3b54fd87 100644 --- a/tests/testsuite/config_include.rs +++ b/tests/testsuite/config_include.rs @@ -317,19 +317,18 @@ fn missing_file() { .build_err(); assert_error( gctx.unwrap_err(), - &format!( - "\ + str![[r#" could not load Cargo configuration Caused by: - failed to load config include `missing.toml` from `[..]/.cargo/config.toml` + failed to load config include `missing.toml` from `[ROOT]/.cargo/config.toml` Caused by: - failed to read configuration file `[..]/.cargo/missing.toml` + failed to read configuration file `[ROOT]/.cargo/missing.toml` Caused by: - [NOT_FOUND]", - ), + [NOT_FOUND] +"#]], ); } @@ -342,11 +341,12 @@ fn wrong_file_extension() { .build_err(); assert_error( gctx.unwrap_err(), - "\ + str![[r#" could not load Cargo configuration Caused by: - expected a config include path ending with `.toml`, but found `config.png` from `[ROOT]/.cargo/config.toml`", + expected a config include path ending with `.toml`, but found `config.png` from `[ROOT]/.cargo/config.toml` +"#]], ); } @@ -361,20 +361,21 @@ fn cycle() { .build_err(); assert_error( gctx.unwrap_err(), - "\ + str![[r#" could not load Cargo configuration Caused by: - failed to load config include `one.toml` from `[..]/.cargo/config.toml` + failed to load config include `one.toml` from `[ROOT]/.cargo/config.toml` Caused by: - failed to load config include `two.toml` from `[..]/.cargo/one.toml` + failed to load config include `two.toml` from `[ROOT]/.cargo/one.toml` Caused by: - failed to load config include `config.toml` from `[..]/.cargo/two.toml` + failed to load config include `config.toml` from `[ROOT]/.cargo/two.toml` Caused by: - config `include` cycle detected with path `[..]/.cargo/config.toml`", + config `include` cycle detected with path `[ROOT]/.cargo/config.toml` +"#]], ); } @@ -407,11 +408,12 @@ fn bad_format() { .build_err(); assert_error( gctx.unwrap_err(), - "\ + str![[r#" could not load Cargo configuration Caused by: - `include` expected a string or list, but found integer in `[..]/.cargo/config.toml`", + expected a string or list of strings, but found integer at `include` in `[ROOT]/.cargo/config.toml +"#]], ); } @@ -424,19 +426,18 @@ fn cli_include_failed() { .build_err(); assert_error( gctx.unwrap_err(), - &format!( - "\ + str![[r#" failed to load --config include Caused by: failed to load config include `foobar.toml` from `--config cli option` Caused by: - failed to read configuration file `[..]/foobar.toml` + failed to read configuration file `[ROOT]/foobar.toml` Caused by: - [NOT_FOUND]" - ), + [NOT_FOUND] +"#]], ); } @@ -457,12 +458,12 @@ fn cli_merge_failed() { // Maybe this error message should mention it was from an include file? assert_error( gctx.unwrap_err(), - "\ -failed to merge --config key `foo` into `[..]/.cargo/config.toml` + str![[r#" +failed to merge --config key `foo` into `[ROOT]/.cargo/config.toml` Caused by: - failed to merge config value from `[..]/.cargo/other.toml` into `[..]/.cargo/config.toml`: \ - expected array, but found string", + failed to merge config value from `[ROOT]/.cargo/other.toml` into `[ROOT]/.cargo/config.toml`: expected array, but found string +"#]], ); } @@ -493,3 +494,130 @@ fn cli_include_take_priority_over_env() { .build(); assert_eq!(gctx.get::("k").unwrap(), "include"); } + +#[cargo_test] +fn inline_table_style() { + write_config_at( + ".cargo/config.toml", + " + include = ['simple.toml', { path = 'other.toml' }] + key1 = 1 + key2 = 2 + ", + ); + write_config_at( + ".cargo/simple.toml", + " + key2 = 3 + key3 = 4 + ", + ); + write_config_at( + ".cargo/other.toml", + " + key3 = 5 + key4 = 6 + ", + ); + + let gctx = GlobalContextBuilder::new() + .unstable_flag("config-include") + .build(); + assert_eq!(gctx.get::("key1").unwrap(), 1); + assert_eq!(gctx.get::("key2").unwrap(), 2); + assert_eq!(gctx.get::("key3").unwrap(), 5); + assert_eq!(gctx.get::("key4").unwrap(), 6); +} + +#[cargo_test] +fn array_of_tables_style() { + write_config_at( + ".cargo/config.toml", + " + key1 = 1 + key2 = 2 + + [[include]] + path = 'other1.toml' + + [[include]] + path = 'other2.toml' + ", + ); + write_config_at( + ".cargo/other1.toml", + " + key2 = 3 + key3 = 4 + ", + ); + write_config_at( + ".cargo/other2.toml", + " + key3 = 5 + key4 = 6 + ", + ); + + let gctx = GlobalContextBuilder::new() + .unstable_flag("config-include") + .build(); + assert_eq!(gctx.get::("key1").unwrap(), 1); + assert_eq!(gctx.get::("key2").unwrap(), 2); + assert_eq!(gctx.get::("key3").unwrap(), 5); + assert_eq!(gctx.get::("key4").unwrap(), 6); +} + +#[cargo_test] +fn table_with_unknown_fields() { + // Unknown fields should be ignored for forward compatibility + write_config_at( + ".cargo/config.toml", + " + key1 = 1 + + [[include]] + path = 'other.toml' + unknown_foo = true + unknown_bar = 123 + ", + ); + write_config_at( + ".cargo/other.toml", + " + key2 = 2 + ", + ); + + let gctx = GlobalContextBuilder::new() + .unstable_flag("config-include") + .build(); + assert_eq!(gctx.get::("key1").unwrap(), 1); + assert_eq!(gctx.get::("key2").unwrap(), 2); +} + +#[cargo_test] +fn table_missing_required_field() { + // Missing required field should fail + write_config_at( + ".cargo/config.toml", + " + key1 = 1 + [[include]] + random_field = true + ", + ); + + let gctx = GlobalContextBuilder::new() + .unstable_flag("config-include") + .build_err(); + assert_error( + gctx.unwrap_err(), + str![[r#" +could not load Cargo configuration + +Caused by: + missing field `path` at `include[0]` in `[ROOT]/.cargo/config.toml` +"#]], + ); +}