From 1fe66e91d07aeaf3355cca7aa229a1e1e092bc5e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 1 Mar 2019 12:49:47 -0500 Subject: [PATCH] rust/treefile: Support dash convention for all options Now that we support YAML, it's a gotcha/eyesore that some of our options use underscores rather than dashes. Let's be nice and switch those few options over, while of course still supporting the previous name. Co-authored-by: Colin Walters Closes: #1749 Approved by: cgwalters --- .../fedora-rawhide-base.json | 14 +-- .../fedora-rawhide-docker.json | 3 +- docs/manual/treefile.md | 35 +++--- rust/src/treefile.rs | 106 +++++++++++++++++- src/app/rpmostree-compose-builtin-rojig.c | 4 +- src/app/rpmostree-compose-builtin-tree.c | 8 +- src/libpriv/rpmostree-postprocess.c | 6 +- 7 files changed, 134 insertions(+), 42 deletions(-) diff --git a/api-doc/treefile-examples/fedora-rawhide-base.json b/api-doc/treefile-examples/fedora-rawhide-base.json index d594fdb9..19e0277a 100644 --- a/api-doc/treefile-examples/fedora-rawhide-base.json +++ b/api-doc/treefile-examples/fedora-rawhide-base.json @@ -1,19 +1,15 @@ { "ref": "fedora-atomic/rawhide/x86_64/base/core", - "gpg_key": "", + "gpg-key": "", "repos": ["fedora-rawhide"], "selinux": true, - "bootstrap_packages": ["filesystem", "glibc", "nss-altfiles", "shadow-utils", - "generic-release"], - "packages": ["kernel", "ostree", "lvm2", - "btrfs-progs", "e2fsprogs", "xfsprogs", - "gnupg2", "selinux-policy-targeted", - "openssh-server", "openssh-clients", - "NetworkManager", "vim-minimal", "nano", "sudo"] + "btrfs-progs", "e2fsprogs", "xfsprogs", + "gnupg2", "selinux-policy-targeted", + "openssh-server", "openssh-clients", + "NetworkManager", "vim-minimal", "nano", "sudo"] } - diff --git a/api-doc/treefile-examples/fedora-rawhide-docker.json b/api-doc/treefile-examples/fedora-rawhide-docker.json index d2a77173..a47b87d6 100644 --- a/api-doc/treefile-examples/fedora-rawhide-docker.json +++ b/api-doc/treefile-examples/fedora-rawhide-docker.json @@ -4,7 +4,6 @@ "include": "fedora-rawhide-base.json", "packages": ["docker-io"], - + "units": ["docker.service", "docker.socket"] } - diff --git a/docs/manual/treefile.md b/docs/manual/treefile.md index e9435fd8..bd48f321 100644 --- a/docs/manual/treefile.md +++ b/docs/manual/treefile.md @@ -13,8 +13,8 @@ It supports the following parameters: * `ref`: string, mandatory: Holds a string which will be the name of the branch for the content. - * `gpg_key` string, optional: Key ID for GPG signing; the secret key - must be in the home directory of the building user. Defaults to + * `gpg-key` (or `gpg_key`): string, optional: Key ID for GPG signing; the + secret key must be in the home directory of the building user. Defaults to none. * `repos` array of strings, mandatory: Names of yum repositories to @@ -26,10 +26,10 @@ It supports the following parameters: * `selinux`: boolean, optional: Defaults to `true`. If `false`, then no SELinux labeling will be performed on the server side. - * `boot_location`: string, optional: Historically, ostree put bootloader data - in /boot. However, this has a few flaws; it gets shadowed at boot time, - and also makes dealing with Anaconda installation harder. There are 3 - possible values: + * `boot-location` (or `boot_location`): string, optional: Historically, ostree + put bootloader data in /boot. However, this has a few flaws; it gets + shadowed at boot time, and also makes dealing with Anaconda installation + harder. There are 3 possible values: * "both": the default, kernel data goes in /boot and /usr/lib/ostree-boot * "legacy": Now an alias for "both"; historically meant just "boot" * "new": kernel data goes in /usr/lib/ostree-boot and /usr/lib/modules @@ -66,7 +66,8 @@ It supports the following parameters: * `units`: Array of strings, optional: Systemd units to enable by default - * `default_target`: String, optional: Set the default systemd target + * `default-target` (or `default_target`): String, optional: Set the default + systemd target. * `initramfs-args`: Array of strings, optional. Passed to the initramfs generation program (presently `dracut`). An example use @@ -143,14 +144,14 @@ It supports the following parameters: Example: `releasever: "26"` - * `automatic_version_prefix`: String, optional: Set the prefix for versions - on the commits. The idea is that if the previous commit on the branch to the - doesn't match the prefix, or doesn't have a version, then the new commit will - have the version as specified. If the prefix matches exactly, then we append - ".1". Otherwise we parse the number after the prefix and increment it by one - and then append that to the prefix. + * `automatic-version-prefix` (or `automatic_version_prefix`): String, optional: + Set the prefix for versions on the commits. The idea is that if the previous + commit on the branch to the doesn't match the prefix, or doesn't have a + version, then the new commit will have the version as specified. If the + prefix matches exactly, then we append ".1". Otherwise we parse the number + after the prefix and increment it by one and then append that to the prefix. - A current date/time may also be passed through `automatic_version_prefix`, + A current date/time may also be passed through `automatic-version-prefix`, by including a date tag in the prefix as such: ``, where `format` is a string with date formats such as `%Y` (year), `%m` (month), etc. The full list of supported formats is [found in the GLib API](https://developer.gnome.org/glib/stable/glib-GDateTime.html#g-date-time-format). @@ -158,18 +159,18 @@ It supports the following parameters: the version, if not present in the prefix, which resets to `.0` if the date (or prefix) changes. - This means that on an empty branch with an `automatic_version_prefix` + This means that on an empty branch with an `automatic-version-prefix` of `"22"` the first three commits would get the versions: "22", "22.1", "22.2". Some example progressions are shown: - | `automatic_version_prefix` | version progression | + | `automatic-version-prefix` | version progression | | -------------------------- | ------------------------------------------ | | `22` | 22, 22.1, 22.2, ... | | `22.1` | 22.1.1, 22.1.2, 22.1.3, ... | | `22.` | 22.2019.0, 22.2019.1, 22.2020.0, ... | | `22..1` | 22.2019.1.0, 22.2019.1.1, 22.2020.1.0, ... | - Example: `automatic_version_prefix: "22.0"` + Example: `automatic-version-prefix: "22.0"` * `postprocess-script`: String, optional: Full filesystem path to a script that will be executed in the context of the target tree. The script diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 0d539394..e529a79c 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -154,6 +154,7 @@ fn treefile_parse_stream( } treefile.packages = Some(pkgs); + treefile = treefile.migrate_legacy_fields()?; Ok(treefile) } @@ -496,7 +497,7 @@ fn whitespace_split_packages(pkgs: &[String]) -> Vec { .collect() } -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, PartialEq)] enum BootLocation { #[serde(rename = "both")] Both, @@ -561,6 +562,7 @@ struct TreeComposeConfig { #[serde(skip_serializing_if = "Option::is_none")] selinux: Option, #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "gpg-key")] gpg_key: Option, #[serde(skip_serializing_if = "Option::is_none")] include: Option, @@ -588,6 +590,7 @@ struct TreeComposeConfig { // Tree layout options #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "boot-location")] boot_location: Option, #[serde(skip_serializing_if = "Option::is_none")] #[serde(rename = "tmp-is-dir")] @@ -597,6 +600,7 @@ struct TreeComposeConfig { #[serde(skip_serializing_if = "Option::is_none")] units: Option>, #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "default-target")] default_target: Option, #[serde(skip_serializing_if = "Option::is_none")] #[serde(rename = "machineid-compat")] @@ -607,6 +611,7 @@ struct TreeComposeConfig { #[serde(skip_serializing_if = "Option::is_none")] releasever: Option, #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "automatic-version-prefix")] automatic_version_prefix: Option, #[serde(skip_serializing_if = "Option::is_none")] #[serde(rename = "mutate-os-release")] @@ -650,10 +655,49 @@ struct TreeComposeConfig { #[serde(rename = "remove-from-packages")] remove_from_packages: Option>>, + #[serde(flatten)] + legacy_fields: LegacyTreeComposeConfigFields, + #[serde(flatten)] extra: HashMap, } +#[derive(Serialize, Deserialize, Debug)] +struct LegacyTreeComposeConfigFields { + #[serde(skip_serializing)] + gpg_key: Option, + #[serde(skip_serializing)] + boot_location: Option, + #[serde(skip_serializing)] + default_target: Option, + #[serde(skip_serializing)] + automatic_version_prefix: Option, +} + +impl TreeComposeConfig { + /// Look for use of legacy/renamed fields and migrate them to the new field. + fn migrate_legacy_fields(mut self) -> Fallible { + macro_rules! migrate_field { + ( $field:ident ) => {{ + if self.legacy_fields.$field.is_some() && self.$field.is_some() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Cannot use new and legacy forms of {}", stringify!($field)), + ).into()); + } + self.$field = self.$field.or(self.legacy_fields.$field.take()); + }} + }; + + migrate_field!(gpg_key); + migrate_field!(boot_location); + migrate_field!(default_target); + migrate_field!(automatic_version_prefix); + + Ok(self) + } +} + #[cfg(test)] mod tests { use super::*; @@ -733,11 +777,15 @@ remove-files: assert!(treefile.packages.unwrap().len() == 3); } + fn append_and_parse(append: &'static str) -> TreeComposeConfig { + let buf = VALID_PRELUDE.to_string() + append; + let mut input = io::BufReader::new(buf.as_bytes()); + treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)).unwrap() + } + fn test_invalid(data: &'static str) { - let mut buf = VALID_PRELUDE.to_string(); - buf.push_str(data); - let buf = buf.as_bytes(); - let mut input = io::BufReader::new(buf); + let buf = VALID_PRELUDE.to_string() + data; + let mut input = io::BufReader::new(buf.as_bytes()); match treefile_parse_stream(InputFormat::YAML, &mut input, Some(ARCH_X86_64)) { Err(ref e) => { match e.downcast_ref::() { @@ -749,6 +797,54 @@ remove-files: } } + #[test] + fn basic_valid_legacy() { + let treefile = append_and_parse(" +gpg_key: foo +boot_location: both +default_target: bar +automatic_version_prefix: baz + "); + assert!(treefile.gpg_key.unwrap() == "foo"); + assert!(treefile.boot_location.unwrap() == BootLocation::Both); + assert!(treefile.default_target.unwrap() == "bar"); + assert!(treefile.automatic_version_prefix.unwrap() == "baz"); + } + + #[test] + fn basic_valid_legacy_new() { + let treefile = append_and_parse(" +gpg-key: foo +boot-location: both +default-target: bar +automatic-version-prefix: baz + "); + assert!(treefile.gpg_key.unwrap() == "foo"); + assert!(treefile.boot_location.unwrap() == BootLocation::Both); + assert!(treefile.default_target.unwrap() == "bar"); + assert!(treefile.automatic_version_prefix.unwrap() == "baz"); + } + + #[test] + fn basic_invalid_legacy_both() { + test_invalid(" +gpg-key: foo +gpg_key: bar + "); + test_invalid(" +boot-location: new +boot_location: both + "); + test_invalid(" +default-target: foo +default_target: bar + "); + test_invalid(" +automatic-version-prefix: foo +automatic_version_prefix: bar + "); + } + #[test] fn test_invalid_install_langs() { test_invalid( diff --git a/src/app/rpmostree-compose-builtin-rojig.c b/src/app/rpmostree-compose-builtin-rojig.c index 682abfa7..e26a4644 100644 --- a/src/app/rpmostree-compose-builtin-rojig.c +++ b/src/app/rpmostree-compose-builtin-rojig.c @@ -389,12 +389,12 @@ impl_rojig_build (RpmOstreeRojigCompose *self, return FALSE; g_autofree char *next_version = NULL; - if (json_object_has_member (self->treefile, "automatic_version_prefix") && + if (json_object_has_member (self->treefile, "automatic-version-prefix") && /* let --add-metadata-string=version=... take precedence */ !g_hash_table_contains (self->metadata, OSTREE_COMMIT_META_KEY_VERSION)) { const char *ver_prefix = - _rpmostree_jsonutil_object_require_string_member (self->treefile, "automatic_version_prefix", error); + _rpmostree_jsonutil_object_require_string_member (self->treefile, "automatic-version-prefix", error); if (!ver_prefix) return FALSE; diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index 54923c7d..731139c2 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -773,12 +773,12 @@ impl_install_tree (RpmOstreeTreeComposeContext *self, return FALSE; g_autofree char *next_version = NULL; - if (json_object_has_member (self->treefile, "automatic_version_prefix") && + if (json_object_has_member (self->treefile, "automatic-version-prefix") && /* let --add-metadata-string=version=... take precedence */ !g_hash_table_contains (self->metadata, OSTREE_COMMIT_META_KEY_VERSION)) { const char *ver_prefix = - _rpmostree_jsonutil_object_require_string_member (self->treefile, "automatic_version_prefix", error); + _rpmostree_jsonutil_object_require_string_member (self->treefile, "automatic-version-prefix", error); if (!ver_prefix) return FALSE; @@ -953,7 +953,7 @@ impl_commit_tree (RpmOstreeTreeComposeContext *self, g_variant_builder_init (&composemeta_builder, G_VARIANT_TYPE ("a{sv}")); const char *gpgkey = NULL; - if (!_rpmostree_jsonutil_object_get_optional_string_member (self->treefile, "gpg_key", &gpgkey, error)) + if (!_rpmostree_jsonutil_object_get_optional_string_member (self->treefile, "gpg-key", &gpgkey, error)) return FALSE; gboolean selinux = TRUE; @@ -1156,7 +1156,7 @@ rpmostree_compose_builtin_postprocess (int argc, const char *rootfs_path = argv[1]; /* Here we *optionally* process a treefile; some things like `tmp-is-dir` and - * `boot_location` are configurable and relevant here, but a lot of users + * `boot-location` are configurable and relevant here, but a lot of users * will also probably be OK with the defaults, and part of the idea here is * to avoid at least some of the use cases requiring a treefile. */ diff --git a/src/libpriv/rpmostree-postprocess.c b/src/libpriv/rpmostree-postprocess.c index aeb7ad77..2d018b4f 100644 --- a/src/libpriv/rpmostree-postprocess.c +++ b/src/libpriv/rpmostree-postprocess.c @@ -270,7 +270,7 @@ rpmostree_postprocess_run_depmod (int rootfs_dfd, * - /boot (CentOS, Fedora treecompose before we suppressed kernel.spec's %posttrans) * - /usr/lib/modules (Fedora treecompose without kernel.spec's %posttrans) * - * We then need to handle the boot_location option, which can put that data in + * We then need to handle the boot-location option, which can put that data in * either both (/boot and /usr/lib/ostree-boot), or just the latter. */ static gboolean @@ -359,7 +359,7 @@ process_kernel_and_initramfs (int rootfs_dfd, RPMOSTREE_POSTPROCESS_BOOT_LOCATION_BOTH; const char *boot_location_str = NULL; if (!_rpmostree_jsonutil_object_get_optional_string_member (treefile, - "boot_location", + "boot-location", &boot_location_str, error)) return FALSE; if (boot_location_str != NULL) @@ -1571,7 +1571,7 @@ rpmostree_treefile_postprocessing (int rootfs_fd, return FALSE; const char *default_target = NULL; - if (!_rpmostree_jsonutil_object_get_optional_string_member (treefile, "default_target", + if (!_rpmostree_jsonutil_object_get_optional_string_member (treefile, "default-target", &default_target, error)) return FALSE;