From 00bd491fe21798b488f3e986fa635473f811ca8b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 14 Jul 2019 18:09:41 +0000 Subject: [PATCH] treefile: Support multiple includes I'm working on having Silverblue inherit from Fedora CoreOS. But conceptually it also inherits from (parts of) Workstation. It is just easier if we support multiple inheritance, then I don't need to think too hard about how to make it a single inheritance chain. Closes: #1870 Approved by: jlebon --- docs/manual/treefile.md | 6 ++-- rust/src/treefile.rs | 45 ++++++++++++++++++++----- tests/compose-tests/test-misc-tweaks.sh | 11 ++++-- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/docs/manual/treefile.md b/docs/manual/treefile.md index dc501a41..8629154a 100644 --- a/docs/manual/treefile.md +++ b/docs/manual/treefile.md @@ -216,11 +216,13 @@ It supports the following parameters: are provided, then `postprocess-script` will be executed after all other `postprocess`. - * `include`: string, optional: Path to another treefile which will be + * `include`: string or array of string, optional: Path(s) to treefiles which will be used as an inheritance base. The semantics for inheritance are: Non-array values in child values override parent values. Array values are concatenated. Filenames will be resolved relative to - the including treefile. + the including treefile. Since rpm-ostree 2019.5, this value may + also be an array of strings. Including the same file multiple times + is an error. * `container`: boolean, optional: Defaults to `false`. If `true`, then rpm-ostree will not do any special handling of kernel, initrd or the diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 505ce2af..434bbb65 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -21,7 +21,7 @@ * */ use c_utf8::CUtf8Buf; -use failure::Fallible; +use failure::{Fallible, bail}; use openat; use serde_derive::{Deserialize, Serialize}; use serde_json; @@ -30,6 +30,8 @@ use std::collections::{HashMap, BTreeMap}; use std::io::prelude::*; use std::path::Path; use std::{collections, fs, io}; +use std::os::unix::fs::MetadataExt; +use std::collections::btree_map::Entry; use crate::utils; @@ -213,14 +215,28 @@ fn load_passwd_file>( return Ok(None); } +type IncludeMap = collections::BTreeMap<(u64, u64), String>; + /// Given a treefile filename and an architecture, parse it and also /// open its external files. fn treefile_parse>( filename: P, basearch: Option<&str>, + seen_includes: &mut IncludeMap, ) -> Fallible { let filename = filename.as_ref(); - let mut f = io::BufReader::new(utils::open_file(filename)?); + let f = utils::open_file(filename)?; + let meta = f.metadata()?; + let devino = (meta.dev(), meta.ino()); + match seen_includes.entry(devino) { + Entry::Occupied(_) => { + bail!("Include loop detected; {} was already included", filename.to_str().unwrap()) + }, + Entry::Vacant(e) => { + e.insert(filename.to_str().unwrap().to_string()); + } + }; + let mut f = io::BufReader::new(f); let basename = filename .file_name() .map(|s| s.to_string_lossy()) @@ -379,11 +395,16 @@ fn treefile_parse_recurse>( filename: P, basearch: Option<&str>, depth: u32, + seen_includes: &mut IncludeMap, ) -> Fallible { let filename = filename.as_ref(); - let mut parsed = treefile_parse(filename, basearch)?; - let include_path = parsed.config.include.take(); - if let &Some(ref include_path) = &include_path { + let mut parsed = treefile_parse(filename, basearch, seen_includes)?; + let include = parsed.config.include.take().unwrap_or_else(|| Include::Multiple(Vec::new())); + let includes = match include { + Include::Single(v) => vec![v], + Include::Multiple(v) => v, + }; + for include_path in includes.iter() { if depth == INCLUDE_MAXDEPTH { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -393,7 +414,7 @@ fn treefile_parse_recurse>( } let parent = filename.parent().unwrap(); let include_path = parent.join(include_path); - let mut included = treefile_parse_recurse(include_path, basearch, depth + 1)?; + let mut included = treefile_parse_recurse(include_path, basearch, depth + 1, seen_includes)?; treefile_merge(&mut parsed.config, &mut included.config); treefile_merge_externals(&mut parsed.externals, &mut included.externals); } @@ -419,7 +440,8 @@ impl Treefile { basearch: Option<&str>, workdir: openat::Dir, ) -> Fallible> { - let mut parsed = treefile_parse_recurse(filename, basearch, 0)?; + let mut seen_includes = collections::BTreeMap::new(); + let mut parsed = treefile_parse_recurse(filename, basearch, 0, &mut seen_includes)?; parsed.config = parsed.config.substitute_vars()?; Treefile::validate_config(&parsed.config)?; let dfd = openat::Dir::open(filename.parent().unwrap())?; @@ -612,6 +634,13 @@ struct Rojig { description: Option, } +#[derive(Serialize, Deserialize, Debug)] +#[serde(untagged)] +enum Include { + Single(String), + Multiple(Vec) +} + // Because of how we handle includes, *everything* here has to be // Option. The defaults live in the code (e.g. machineid-compat defaults // to `true`). @@ -634,7 +663,7 @@ struct TreeComposeConfig { #[serde(rename = "gpg-key")] gpg_key: Option, #[serde(skip_serializing_if = "Option::is_none")] - include: Option, + include: Option, // Core content #[serde(skip_serializing_if = "Option::is_none")] diff --git a/tests/compose-tests/test-misc-tweaks.sh b/tests/compose-tests/test-misc-tweaks.sh index 53151dd1..1396d85b 100755 --- a/tests/compose-tests/test-misc-tweaks.sh +++ b/tests/compose-tests/test-misc-tweaks.sh @@ -6,15 +6,20 @@ dn=$(cd $(dirname $0) && pwd) . ${dn}/libcomposetest.sh prepare_compose_test "misc-tweaks" -# No docs -pysetjsonmember "documentation" "False" +# No docs, also test multi includes +cat >composedata/documentation.yaml <<'EOF' +documentation: false +EOF +cat > composedata/recommends.yaml <<'EOF' +recommends: false +EOF +pysetjsonmember "include" '["documentation.yaml", "recommends.yaml"]' # Note this overrides: # $ rpm -q systemd # systemd-238-8.git0e0aa59.fc28.x86_64 # $ rpm -qlv systemd|grep -F 'system/default.target ' # lrwxrwxrwx 1 root root 16 May 11 06:59 /usr/lib/systemd/system/default.target -> graphical.target pysetjsonmember "default_target" '"multi-user.target"' -pysetjsonmember "recommends" 'False' pysetjsonmember "units" '["tuned.service"]' # And test adding/removing files pysetjsonmember "add-files" '[["foo.txt", "/usr/etc/foo.txt"],