From dfbaa529df307588992824fafb1f2175152d0efa Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 26 Feb 2019 11:04:48 -0500 Subject: [PATCH 1/4] tests/libtest.sh: Lift assert_jq from libvm.sh We already had this logic, but it was in `libvm.sh`. Prep for using it elsewhere. --- tests/common/libtest.sh | 13 +++++++++++++ tests/common/libvm.sh | 10 +--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/common/libtest.sh b/tests/common/libtest.sh index 2404334a2a..a06a34159c 100644 --- a/tests/common/libtest.sh +++ b/tests/common/libtest.sh @@ -562,3 +562,16 @@ assert_files_hardlinked() { fatal "Files '$1' and '$2' are not hardlinked" fi } + +# $1 - json file +# $2+ - assertions +assert_jq() { + f=$1; shift + for expression in "$@"; do + if ! jq -e "${expression}" >/dev/null < $f; then + jq . < $f | sed -e 's/^/# /' >&2 + echo 1>&2 "${expression} failed to match $f" + exit 1 + fi + done +} diff --git a/tests/common/libvm.sh b/tests/common/libvm.sh index 06c9c023e2..3716ee2978 100644 --- a/tests/common/libvm.sh +++ b/tests/common/libvm.sh @@ -354,15 +354,7 @@ vm_assert_layered_pkg() { # and asserts that they are true. vm_assert_status_jq() { vm_rpmostree status --json > status.json - vm_rpmostree status > status.txt - for expression in "$@"; do - if ! jq -e "${expression}" >/dev/null < status.json; then - jq . < status.json | sed -e 's/^/# /' >&2 - echo 1>&2 "${expression} failed to match status.json" - cat status.txt - exit 1 - fi - done + assert_jq status.json "$@" } vm_pending_is_staged() { From 7f02b92dcf551db6a01e1199c1e70713ba457b79 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 26 Feb 2019 09:37:04 -0500 Subject: [PATCH 2/4] rust/treefile: Rename arch -> basearch We're really using this variable to substitute `${basearch}` and find basearch-specific packages. Let's rename the variable to make that more obvious. --- rust/src/treefile.rs | 46 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index f2139f616d..870bd72fcf 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -68,12 +68,12 @@ enum InputFormat { JSON, } -/// Parse a YAML treefile definition using architecture `arch`. +/// Parse a YAML treefile definition using base architecture `basearch`. /// This does not open the externals. fn treefile_parse_stream( fmt: InputFormat, input: &mut R, - arch: Option<&str>, + basearch: Option<&str>, ) -> Fallible { let mut treefile: TreeComposeConfig = match fmt { InputFormat::YAML => { @@ -97,10 +97,10 @@ fn treefile_parse_stream( }; // Substitute ${basearch} - treefile.treeref = match (arch, treefile.treeref.take()) { - (Some(arch), Some(treeref)) => { + treefile.treeref = match (basearch, treefile.treeref.take()) { + (Some(basearch), Some(treeref)) => { let mut varsubsts = HashMap::new(); - varsubsts.insert("basearch".to_string(), arch.to_string()); + varsubsts.insert("basearch".to_string(), basearch.to_string()); Some( utils::varsubst(&treeref, &varsubsts) .map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e.to_string()))?, @@ -111,7 +111,7 @@ fn treefile_parse_stream( // Special handling for packages, since we allow whitespace within items. // We also canonicalize bootstrap_packages to packages here so it's - // easier to append the arch packages after. + // easier to append the basearch packages after. let mut pkgs: Vec = vec![]; { if let Some(base_pkgs) = treefile.packages.take() { @@ -122,7 +122,7 @@ fn treefile_parse_stream( } } - let arch_pkgs = match arch { + let arch_pkgs = match basearch { Some("aarch64") => treefile.packages_aarch64.take(), Some("armhfp") => treefile.packages_armhfp.take(), Some("ppc64") => treefile.packages_ppc64.take(), @@ -164,7 +164,7 @@ fn load_passwd_file>( /// open its external files. fn treefile_parse>( filename: P, - arch: Option<&str>, + basearch: Option<&str>, ) -> Fallible { let filename = filename.as_ref(); let mut f = io::BufReader::new(open_file(filename)?); @@ -177,7 +177,7 @@ fn treefile_parse>( } else { InputFormat::JSON }; - let tf = treefile_parse_stream(fmt, &mut f, arch).map_err(|e| { + let tf = treefile_parse_stream(fmt, &mut f, basearch).map_err(|e| { io::Error::new( io::ErrorKind::InvalidInput, format!("Parsing {}: {}", filename.to_string_lossy(), e.to_string()), @@ -302,11 +302,11 @@ fn treefile_merge_externals(dest: &mut TreefileExternals, src: &mut TreefileExte /// Recursively parse a treefile, merging along the way. fn treefile_parse_recurse>( filename: P, - arch: Option<&str>, + basearch: Option<&str>, depth: u32, ) -> Fallible { let filename = filename.as_ref(); - let mut parsed = treefile_parse(filename, arch)?; + let mut parsed = treefile_parse(filename, basearch)?; let include_path = parsed.config.include.take(); if let &Some(ref include_path) = &include_path { if depth == INCLUDE_MAXDEPTH { @@ -317,7 +317,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, arch, depth + 1)?; + let mut included = treefile_parse_recurse(include_path, basearch, depth + 1)?; treefile_merge(&mut parsed.config, &mut included.config); treefile_merge_externals(&mut parsed.externals, &mut included.externals); } @@ -340,10 +340,10 @@ impl Treefile { /// The main treefile creation entrypoint. fn new_boxed( filename: &Path, - arch: Option<&str>, + basearch: Option<&str>, workdir: openat::Dir, ) -> Fallible> { - let parsed = treefile_parse_recurse(filename, arch, 0)?; + let parsed = treefile_parse_recurse(filename, basearch, 0)?; Treefile::validate_config(&parsed.config)?; let dfd = openat::Dir::open(filename.parent().unwrap())?; let (rojig_name, rojig_spec) = if let Some(rojig) = parsed.config.rojig.as_ref() { @@ -752,7 +752,7 @@ remove-files: } impl TreefileTest { - fn new<'a, 'b>(contents: &'a str, arch: Option<&'b str>) -> Fallible { + fn new<'a, 'b>(contents: &'a str, basearch: Option<&'b str>) -> Fallible { let workdir = tempfile::tempdir()?; let tf_path = workdir.path().join("treefile.yaml"); { @@ -760,7 +760,7 @@ remove-files: tf_stream.write_all(contents.as_bytes())?; } let tf = - Treefile::new_boxed(tf_path.as_path(), arch, openat::Dir::open(workdir.path())?)?; + Treefile::new_boxed(tf_path.as_path(), basearch, openat::Dir::open(workdir.path())?)?; Ok(TreefileTest { tf, workdir }) } } @@ -796,18 +796,18 @@ rojig: #[test] fn test_treefile_merge() { - let arch = Some(ARCH_X86_64); + let basearch = Some(ARCH_X86_64); let mut base_input = io::BufReader::new(VALID_PRELUDE.as_bytes()); - let mut base = treefile_parse_stream(InputFormat::YAML, &mut base_input, arch).unwrap(); + let mut base = treefile_parse_stream(InputFormat::YAML, &mut base_input, basearch).unwrap(); let mut mid_input = io::BufReader::new( r###" packages: - some layered packages "###.as_bytes(), ); - let mut mid = treefile_parse_stream(InputFormat::YAML, &mut mid_input, arch).unwrap(); + let mut mid = treefile_parse_stream(InputFormat::YAML, &mut mid_input, basearch).unwrap(); let mut top_input = io::BufReader::new(ROJIG_YAML.as_bytes()); - let mut top = treefile_parse_stream(InputFormat::YAML, &mut top_input, arch).unwrap(); + let mut top = treefile_parse_stream(InputFormat::YAML, &mut top_input, basearch).unwrap(); treefile_merge(&mut mid, &mut base); treefile_merge(&mut top, &mut mid); let tf = ⊤ @@ -849,18 +849,18 @@ mod ffi { #[no_mangle] pub extern "C" fn ror_treefile_new( filename: *const libc::c_char, - arch: *const libc::c_char, + basearch: *const libc::c_char, workdir_dfd: libc::c_int, gerror: *mut *mut glib_sys::GError, ) -> *mut Treefile { // Convert arguments let filename = ffi_view_os_str(filename); - let arch = ffi_view_nullable_str(arch); + let basearch = ffi_view_nullable_str(basearch); let workdir = ffi_view_openat_dir(workdir_dfd); // Run code, map error if any, otherwise extract raw pointer, passing // ownership back to C. ptr_glib_error( - Treefile::new_boxed(filename.as_ref(), arch, workdir), + Treefile::new_boxed(filename.as_ref(), basearch, workdir), gerror, ) } From 02dbdcc2ef7d6c5b7528d2ab75c803f22639ae4c Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 26 Feb 2019 10:51:14 -0500 Subject: [PATCH 3/4] rust/treefile: Add basearch key Add a `basearch` key to the manifest. This can be used at compose time to assert the architecture the compose is running on. Though my motivation is for the common case where it gets omitted from the input manifest and gets automatically added by rpm-ostree into `/usr/share/rpm-ostree/treefile.json` for introspection on the client. (The crucial part here is that the treefile created by rpm-ostree remains deserializable into a `TreeComposeConfig`). Closes: https://github.com/coreos/fedora-coreos-tracker/issues/154 --- rust/src/treefile.rs | 18 ++++++++++++++++++ tests/compose-tests/libbasic-test.sh | 4 ++++ 2 files changed, 22 insertions(+) diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 870bd72fcf..1012b1a6f9 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -96,6 +96,22 @@ fn treefile_parse_stream( } }; + treefile.basearch = match (treefile.basearch, basearch) { + (Some(treearch), Some(arch)) => { + if treearch != arch { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Invalid basearch: cross-composes are not supported"), + ).into()) + } else { + Some(treearch) + } + } + (None, Some(arch)) => Some(arch.into()), + // really, only for tests do we not specify a basearch. let's just canonicalize to None + (_, None) => None, + }; + // Substitute ${basearch} treefile.treeref = match (basearch, treefile.treeref.take()) { (Some(basearch), Some(treeref)) => { @@ -501,6 +517,8 @@ struct TreeComposeConfig { #[serde(rename = "ref")] #[serde(skip_serializing_if = "Option::is_none")] treeref: Option, + #[serde(skip_serializing_if = "Option::is_none")] + basearch: Option, // Optional rojig data #[serde(skip_serializing_if = "Option::is_none")] rojig: Option, diff --git a/tests/compose-tests/libbasic-test.sh b/tests/compose-tests/libbasic-test.sh index d6fefd684b..401ca6b150 100644 --- a/tests/compose-tests/libbasic-test.sh +++ b/tests/compose-tests/libbasic-test.sh @@ -101,3 +101,7 @@ assert_file_has_content pkglist.txt 'systemd' assert_file_has_content pkglist.txt 'systemd-bootchart' echo "ok compose pkglist" } + +ostree --repo=${repobuild} cat ${treeref} /usr/share/rpm-ostree/treefile.json > treefile.json +assert_jq treefile.json '.basearch == "x86_64"' +echo "ok basearch" From 92d8f86e57b01bd893f63a906f62e27b0efb37f2 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 26 Feb 2019 11:37:03 -0500 Subject: [PATCH 4/4] fixup! rust/treefile: Add basearch key --- rust/src/treefile.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 1012b1a6f9..ebc162892f 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -101,7 +101,8 @@ fn treefile_parse_stream( if treearch != arch { return Err(io::Error::new( io::ErrorKind::InvalidInput, - format!("Invalid basearch: cross-composes are not supported"), + format!("Invalid basearch {} on {}: cross-composes are not supported", + treearch, arch), ).into()) } else { Some(treearch)