Skip to content

Commit e1a37a3

Browse files
authored
feat(config-include): support inline and array of tables (#16174)
### What does this PR try to resolve? Part of <#7723 (comment)> Both are now supported: ``` include = ['simple.toml', { path = 'other.toml' }] ``` and ``` [[include]] path = 'first.toml' [[include]] path = 'second.toml' ``` So in the future we can extend field like `optional` or `if` in the include table ### How to test and review this PR? On the implementation side, To avoid misuse of `GlobalContext::get<ConfigInclude>`, I chose not using serde deserialization, and instead did manual table extraction
2 parents c3fc265 + 25d7318 commit e1a37a3

File tree

2 files changed

+237
-68
lines changed

2 files changed

+237
-68
lines changed

src/cargo/util/context/mod.rs

Lines changed: 85 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,11 +1257,15 @@ impl GlobalContext {
12571257
output: &mut Vec<CV>,
12581258
) -> CargoResult<()> {
12591259
let includes = self.include_paths(cv, false)?;
1260-
for (path, abs_path, def) in includes {
1260+
for include in includes {
12611261
let mut cv = self
1262-
._load_file(&abs_path, seen, false, WhyLoad::FileDiscovery)
1262+
._load_file(&include.abs_path(self), seen, false, WhyLoad::FileDiscovery)
12631263
.with_context(|| {
1264-
format!("failed to load config include `{}` from `{}`", path, def)
1264+
format!(
1265+
"failed to load config include `{}` from `{}`",
1266+
include.path.display(),
1267+
include.def
1268+
)
12651269
})?;
12661270
self.load_unmerged_include(&mut cv, seen, output)?;
12671271
output.push(cv);
@@ -1364,58 +1368,59 @@ impl GlobalContext {
13641368
}
13651369
// Accumulate all values here.
13661370
let mut root = CV::Table(HashMap::new(), value.definition().clone());
1367-
for (path, abs_path, def) in includes {
1368-
self._load_file(&abs_path, seen, true, why_load)
1371+
for include in includes {
1372+
self._load_file(&include.abs_path(self), seen, true, why_load)
13691373
.and_then(|include| root.merge(include, true))
13701374
.with_context(|| {
1371-
format!("failed to load config include `{}` from `{}`", path, def)
1375+
format!(
1376+
"failed to load config include `{}` from `{}`",
1377+
include.path.display(),
1378+
include.def
1379+
)
13721380
})?;
13731381
}
13741382
root.merge(value, true)?;
13751383
Ok(root)
13761384
}
13771385

13781386
/// Converts the `include` config value to a list of absolute paths.
1379-
fn include_paths(
1380-
&self,
1381-
cv: &mut CV,
1382-
remove: bool,
1383-
) -> CargoResult<Vec<(String, PathBuf, Definition)>> {
1384-
let abs = |path: &str, def: &Definition| -> (String, PathBuf, Definition) {
1385-
let abs_path = match def {
1386-
Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap().join(&path),
1387-
Definition::Environment(_) | Definition::Cli(None) | Definition::BuiltIn => {
1388-
self.cwd().join(&path)
1389-
}
1390-
};
1391-
(path.to_string(), abs_path, def.clone())
1392-
};
1387+
fn include_paths(&self, cv: &mut CV, remove: bool) -> CargoResult<Vec<ConfigInclude>> {
13931388
let CV::Table(table, _def) = cv else {
13941389
unreachable!()
13951390
};
1396-
let owned;
13971391
let include = if remove {
1398-
owned = table.remove("include");
1399-
owned.as_ref()
1392+
table.remove("include").map(Cow::Owned)
14001393
} else {
1401-
table.get("include")
1394+
table.get("include").map(Cow::Borrowed)
14021395
};
1403-
let includes = match include {
1404-
Some(CV::String(s, def)) => {
1405-
vec![abs(s, def)]
1406-
}
1396+
let includes = match include.map(|c| c.into_owned()) {
1397+
Some(CV::String(s, def)) => vec![ConfigInclude::new(s, def)],
14071398
Some(CV::List(list, _def)) => list
1408-
.iter()
1409-
.map(|cv| match cv {
1410-
CV::String(s, def) => Ok(abs(s, def)),
1399+
.into_iter()
1400+
.enumerate()
1401+
.map(|(idx, cv)| match cv {
1402+
CV::String(s, def) => Ok(ConfigInclude::new(s, def)),
1403+
CV::Table(mut table, def) => {
1404+
// Extract `include.path`
1405+
let s = match table.remove("path") {
1406+
Some(CV::String(s, _)) => s,
1407+
Some(other) => bail!(
1408+
"expected a string, but found {} at `include[{idx}].path` in `{def}`",
1409+
other.desc()
1410+
),
1411+
None => bail!("missing field `path` at `include[{idx}]` in `{def}`"),
1412+
};
1413+
Ok(ConfigInclude::new(s, def))
1414+
}
14111415
other => bail!(
1412-
"`include` expected a string or list of strings, but found {} in list",
1413-
other.desc()
1416+
"expected a string or table, but found {} at `include[{idx}]` in {}",
1417+
other.desc(),
1418+
other.definition(),
14141419
),
14151420
})
14161421
.collect::<CargoResult<Vec<_>>>()?,
14171422
Some(other) => bail!(
1418-
"`include` expected a string or list, but found {} in `{}`",
1423+
"expected a string or list of strings, but found {} at `include` in `{}",
14191424
other.desc(),
14201425
other.definition()
14211426
),
@@ -1424,11 +1429,13 @@ impl GlobalContext {
14241429
}
14251430
};
14261431

1427-
for (path, abs_path, def) in &includes {
1428-
if abs_path.extension() != Some(OsStr::new("toml")) {
1432+
for include in &includes {
1433+
if include.path.extension() != Some(OsStr::new("toml")) {
14291434
bail!(
14301435
"expected a config include path ending with `.toml`, \
1431-
but found `{path}` from `{def}`",
1436+
but found `{}` from `{}`",
1437+
include.path.display(),
1438+
include.def,
14321439
)
14331440
}
14341441
}
@@ -1447,14 +1454,10 @@ impl GlobalContext {
14471454
let arg_as_path = self.cwd.join(arg);
14481455
let tmp_table = if !arg.is_empty() && arg_as_path.exists() {
14491456
// --config path_to_file
1450-
let str_path = arg_as_path
1451-
.to_str()
1452-
.ok_or_else(|| {
1453-
anyhow::format_err!("config path {:?} is not utf-8", arg_as_path)
1457+
self._load_file(&arg_as_path, &mut seen, true, WhyLoad::Cli)
1458+
.with_context(|| {
1459+
format!("failed to load config from `{}`", arg_as_path.display())
14541460
})?
1455-
.to_string();
1456-
self._load_file(&self.cwd().join(&str_path), &mut seen, true, WhyLoad::Cli)
1457-
.with_context(|| format!("failed to load config from `{}`", str_path))?
14581461
} else {
14591462
let doc = toml_dotted_keys(arg)?;
14601463
let doc: toml::Value = toml::Value::deserialize(doc.into_deserializer())
@@ -2482,6 +2485,44 @@ pub fn save_credentials(
24822485
}
24832486
}
24842487

2488+
/// Represents a config-include value in the configuration.
2489+
///
2490+
/// This intentionally doesn't derive serde deserialization
2491+
/// to avoid any misuse of `GlobalContext::get::<ConfigInclude>()`,
2492+
/// which might lead to wrong config loading order.
2493+
struct ConfigInclude {
2494+
/// Path to a config-include configuration file.
2495+
/// Could be either relative or absolute.
2496+
path: PathBuf,
2497+
def: Definition,
2498+
}
2499+
2500+
impl ConfigInclude {
2501+
fn new(p: impl Into<PathBuf>, def: Definition) -> Self {
2502+
Self {
2503+
path: p.into(),
2504+
def,
2505+
}
2506+
}
2507+
2508+
/// Gets the absolute path of the config-include config file.
2509+
///
2510+
/// For file based include,
2511+
/// it is relative to parent directory of the config file includes it.
2512+
/// For example, if `.cargo/config.toml has a `include = "foo.toml"`,
2513+
/// Cargo will load `.cargo/foo.toml`.
2514+
///
2515+
/// For CLI based include (e.g., `--config 'include = "foo.toml"'`),
2516+
/// it is relative to the current working directory.
2517+
fn abs_path(&self, gctx: &GlobalContext) -> PathBuf {
2518+
match &self.def {
2519+
Definition::Path(p) | Definition::Cli(Some(p)) => p.parent().unwrap(),
2520+
Definition::Environment(_) | Definition::Cli(None) | Definition::BuiltIn => gctx.cwd(),
2521+
}
2522+
.join(&self.path)
2523+
}
2524+
}
2525+
24852526
#[derive(Debug, Default, Deserialize, PartialEq)]
24862527
#[serde(rename_all = "kebab-case")]
24872528
pub struct CargoHttpConfig {

0 commit comments

Comments
 (0)