From 834328f963b3bc5bf9bcfcc2030e66f02fb5dae8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 12 Apr 2021 09:15:57 -0400 Subject: [PATCH] Move `ref` parsing into core We went through a lot of gyrations on this one. It's only relevant to server side composes, so having the core parse the treefile for it directly just makes sense. --- rust/src/lib.rs | 1 + rust/src/treefile.rs | 4 +++ src/app/rpmostree-compose-builtin-tree.cxx | 3 +- src/app/rpmostree-composeutil.cxx | 10 ------ src/libpriv/rpmostree-core-private.h | 1 + src/libpriv/rpmostree-core.cxx | 40 +++++++++++----------- src/libpriv/rpmostree-core.h | 2 +- 7 files changed, 29 insertions(+), 32 deletions(-) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index c12b82cf..a189530f 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -251,6 +251,7 @@ pub mod ffi { fn get_ostree_override_layers(&self) -> Vec; fn get_all_ostree_layers(&self) -> Vec; fn get_repos(&self) -> Vec; + fn get_ref(&self) -> &str; fn get_rojig_spec_path(&self) -> String; fn get_rojig_name(&self) -> String; fn get_cliwrap(&self) -> bool; diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 17934fab..bae7a88f 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -541,6 +541,10 @@ impl Treefile { self.parsed.repos.clone().unwrap_or_default() } + pub(crate) fn get_ref(&self) -> &str { + self.parsed.treeref.as_deref().unwrap_or_default() + } + pub(crate) fn get_rojig_spec_path(&self) -> String { self.rojig_spec.clone().unwrap_or_default() } diff --git a/src/app/rpmostree-compose-builtin-tree.cxx b/src/app/rpmostree-compose-builtin-tree.cxx index 1f4fe75f..d1bb19d0 100644 --- a/src/app/rpmostree-compose-builtin-tree.cxx +++ b/src/app/rpmostree-compose-builtin-tree.cxx @@ -710,6 +710,8 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (!opt_unified_core) rpmostree_context_disable_selinux (self->corectx); + self->ref = g_strdup (rpmostree_context_get_ref (self->corectx)); + if (opt_lockfiles) { rpmostree_context_set_lockfile (self->corectx, opt_lockfiles, opt_lockfile_strict); @@ -782,7 +784,6 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, error); if (!self->treespec) return FALSE; - self->ref = rpmostree_treespec_get_ref (self->treespec); *out_context = util::move_nullify (self); return TRUE; diff --git a/src/app/rpmostree-composeutil.cxx b/src/app/rpmostree-composeutil.cxx index 774ed1a7..ce652354 100644 --- a/src/app/rpmostree-composeutil.cxx +++ b/src/app/rpmostree-composeutil.cxx @@ -175,7 +175,6 @@ rpmostree_composeutil_get_treespec (RpmOstreeContext *ctx, GError **error) { GLNX_AUTO_PREFIX_ERROR ("Parsing treefile", error); - auto varsubsts = rpmostree_dnfcontext_get_varsubsts (rpmostree_context_get_dnf (ctx)); g_autoptr(GKeyFile) treespec = g_key_file_new (); if (!treespec_bind_array (treedata, treespec, "packages", NULL, TRUE, error)) @@ -195,15 +194,6 @@ rpmostree_composeutil_get_treespec (RpmOstreeContext *ctx, if (!treespec_bind_array (treedata, treespec, "install-langs", "instlangs", FALSE, error)) return NULL; - const char *input_ref = NULL; - if (!_rpmostree_jsonutil_object_get_optional_string_member (treedata, "ref", &input_ref, error)) - return NULL; - if (input_ref) - { - auto ref = rpmostreecxx::varsubstitute (input_ref, *varsubsts); - g_key_file_set_string (treespec, "tree", "ref", ref.c_str()); - } - return rpmostree_treespec_new_from_keyfile (treespec, error); } diff --git a/src/libpriv/rpmostree-core-private.h b/src/libpriv/rpmostree-core-private.h index 3fc5ed52..eee90131 100644 --- a/src/libpriv/rpmostree-core-private.h +++ b/src/libpriv/rpmostree-core-private.h @@ -39,6 +39,7 @@ struct _RpmOstreeContext { rpmostreecxx::Treefile *treefile_rs; /* For composes for now */ gboolean empty; gboolean disable_selinux; + char *ref; /* rojig-mode data */ const char *rojig_spec; /* The rojig spec like: repoid:package */ diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index 97c2dcfa..9e72c8d4 100644 --- a/src/libpriv/rpmostree-core.cxx +++ b/src/libpriv/rpmostree-core.cxx @@ -226,13 +226,6 @@ rpmostree_treespec_new_from_keyfile (GKeyFile *keyfile, g_variant_builder_init (&builder, (GVariantType*)"a{sv}"); - /* We allow the "ref" key to be missing for cases where we don't need one. - * This is abusing the Treespec a bit, but oh well... */ - { g_autofree char *ref = g_key_file_get_string (keyfile, "tree", "ref", NULL); - if (ref) - g_variant_builder_add (&builder, "{sv}", "ref", g_variant_new_string (ref)); - } - #define BIND_STRING(k) \ { g_autofree char *v = g_key_file_get_string (keyfile, "tree", k, NULL); \ if (v) \ @@ -289,14 +282,6 @@ rpmostree_treespec_new (GVariant *variant) return util::move_nullify (ret); } -const char * -rpmostree_treespec_get_ref (RpmOstreeTreespec *spec) -{ - const char *r = NULL; - g_variant_dict_lookup (spec->dict, "ref", "&s", &r); - return r; -} - GVariant * rpmostree_treespec_to_variant (RpmOstreeTreespec *spec) { @@ -317,6 +302,8 @@ rpmostree_context_finalize (GObject *object) g_clear_object (&rctx->spec); g_clear_object (&rctx->dnfctx); + g_clear_pointer (&rctx->ref, g_free); + g_clear_object (&rctx->rojig_pkg); g_free (rctx->rojig_checksum); g_free (rctx->rojig_inputhash); @@ -450,6 +437,15 @@ rpmostree_context_new_tree (int userroot_dfd, dnf_context_set_lock_dir (ret->dnfctx, lockdir); } + // The ref needs special handling as it gets variable-substituted. + auto ref = ret->treefile_rs->get_ref(); + if (ref.length() > 0) + { + auto varsubsts = rpmostree_dnfcontext_get_varsubsts(ret->dnfctx); + auto subst_ref = rpmostreecxx::varsubstitute(ref, *varsubsts); + ret->ref = g_strdup(subst_ref.c_str()); + } + return util::move_nullify (ret); } @@ -520,6 +516,12 @@ rpmostree_context_disable_selinux (RpmOstreeContext *self) self->disable_selinux = TRUE; } +const char * +rpmostree_context_get_ref (RpmOstreeContext *self) +{ + return self->ref; +} + /* XXX: or put this in new_system() instead? */ void rpmostree_context_set_repos (RpmOstreeContext *self, @@ -4829,11 +4831,9 @@ rpmostree_context_commit (RpmOstreeContext *self, return FALSE; } - { const char * ref = rpmostree_treespec_get_ref (self->spec); - if (ref != NULL) - ostree_repo_transaction_set_ref (self->ostreerepo, NULL, ref, - ret_commit_checksum); - } + if (self->ref != NULL) + ostree_repo_transaction_set_ref (self->ostreerepo, NULL, self->ref, + ret_commit_checksum); { OstreeRepoTransactionStats stats; g_autofree char *bytes_written_formatted = NULL; diff --git a/src/libpriv/rpmostree-core.h b/src/libpriv/rpmostree-core.h index 33ed32d1..040e2e37 100644 --- a/src/libpriv/rpmostree-core.h +++ b/src/libpriv/rpmostree-core.h @@ -113,7 +113,6 @@ RpmOstreeTreespec *rpmostree_treespec_new (GVariant *variant); GVariant *rpmostree_context_get_rpmmd_repo_commit_metadata (RpmOstreeContext *self); GVariant *rpmostree_treespec_to_variant (RpmOstreeTreespec *spec); -const char *rpmostree_treespec_get_ref (RpmOstreeTreespec *spec); gboolean rpmostree_context_setup (RpmOstreeContext *self, const char *install_root, @@ -129,6 +128,7 @@ rpmostree_context_configure_from_deployment (RpmOstreeContext *self, void rpmostree_context_set_is_empty (RpmOstreeContext *self); void rpmostree_context_disable_selinux (RpmOstreeContext *self); +const char *rpmostree_context_get_ref (RpmOstreeContext *self); void rpmostree_context_set_repos (RpmOstreeContext *self, OstreeRepo *base_repo,