From 4249dc8c6313ca5ff97a4123218f08a86ce0699c Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 8 Jun 2018 15:48:05 -0400 Subject: [PATCH] libpriv: Directly parse NEVRAs, don't use branches When I initially added support for local RPM layering (#657), I was operating under the assumption that there is no reliable way to parse a NEVRA back to its constituent elements. So we worked around this by doing lookups in the pkgcache and comparing against branches converted back to their NEVRAs, which was pretty hacky. Later on, I learned that you *can* reliably parse a NEVRA back to its elements because RPM forbids all fields from having a colon `:` and all fields other than the package name to have a dash `-`. In fact, there is a helper from `libdnf` to do exactly this. This patch swaps out the old way of decomposing NEVRAs and converting NEVRAs to their cache branch notation with a simpler more direct approach using the helper function from `libdnf`. Closes: #1395 Approved by: cgwalters --- src/daemon/rpmostree-sysroot-core.c | 3 +- src/daemon/rpmostree-sysroot-upgrader.c | 4 +- src/libpriv/rpmostree-core.c | 71 +------------------------ src/libpriv/rpmostree-core.h | 18 ------- src/libpriv/rpmostree-rpm-util.c | 59 +++++++++++++++++++- src/libpriv/rpmostree-rpm-util.h | 13 +++++ tests/check/test-utils.c | 10 ++-- 7 files changed, 81 insertions(+), 97 deletions(-) diff --git a/src/daemon/rpmostree-sysroot-core.c b/src/daemon/rpmostree-sysroot-core.c index b9926161..ca0328fd 100644 --- a/src/daemon/rpmostree-sysroot-core.c +++ b/src/daemon/rpmostree-sysroot-core.c @@ -234,8 +234,7 @@ generate_pkgcache_refs (OstreeSysroot *sysroot, GLNX_HASH_TABLE_FOREACH (local_replace, const char*, nevra) { g_autofree char *cachebranch = NULL; - if (!rpmostree_find_cache_branch_by_nevra (repo, nevra, &cachebranch, - cancellable, error)) + if (!rpmostree_nevra_to_cache_branch (nevra, &cachebranch, error)) return FALSE; g_hash_table_add (referenced_pkgs, g_steal_pointer (&cachebranch)); diff --git a/src/daemon/rpmostree-sysroot-upgrader.c b/src/daemon/rpmostree-sysroot-upgrader.c index fc404067..cf4a671f 100644 --- a/src/daemon/rpmostree-sysroot-upgrader.c +++ b/src/daemon/rpmostree-sysroot-upgrader.c @@ -677,10 +677,8 @@ finalize_replacement_overrides (RpmOstreeSysrootUpgrader *self, GLNX_HASH_TABLE_FOREACH_KV (local_replacements, const char*, nevra, const char*, sha256) { - /* use the pkgcache because there's no safe way to go from nevra --> pkgname */ g_autofree char *pkgname = NULL; - if (!rpmostree_get_nevra_from_pkgcache (self->repo, nevra, &pkgname, NULL, NULL, - NULL, NULL, cancellable, error)) + if (!rpmostree_decompose_nevra (nevra, &pkgname, NULL, NULL, NULL, NULL, error)) return FALSE; g_autoptr(DnfPackage) pkg = NULL; diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c index 59ea30b6..fb715d66 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -919,37 +919,6 @@ checkout_pkg_metadata_by_dnfpkg (RpmOstreeContext *self, return checkout_pkg_metadata (self, nevra, header, cancellable, error); } -gboolean -rpmostree_find_cache_branch_by_nevra (OstreeRepo *pkgcache, - const char *nevra, - char **out_cache_branch, - GCancellable *cancellable, - GError **error) - -{ - /* there's no safe way to convert a nevra string to its cache branch, so let's - * just do a dumb lookup */ - /* TODO: parse the refs once at core init, this function is itself - * called in loops */ - - g_autoptr(GHashTable) refs = NULL; - if (!ostree_repo_list_refs_ext (pkgcache, "rpmostree/pkg", &refs, - OSTREE_REPO_LIST_REFS_EXT_NONE, cancellable, error)) - return FALSE; - - GLNX_HASH_TABLE_FOREACH (refs, const char*, ref) - { - g_autofree char *cache_nevra = rpmostree_cache_branch_to_nevra (ref); - if (g_str_equal (nevra, cache_nevra)) - { - *out_cache_branch = g_strdup (ref); - return TRUE; - } - } - - return glnx_throw (error, "Failed to find cached pkg for %s", nevra); -} - gboolean rpmostree_pkgcache_find_pkg_header (OstreeRepo *pkgcache, const char *nevra, @@ -959,8 +928,7 @@ rpmostree_pkgcache_find_pkg_header (OstreeRepo *pkgcache, GError **error) { g_autofree char *cache_branch = NULL; - if (!rpmostree_find_cache_branch_by_nevra (pkgcache, nevra, &cache_branch, - cancellable, error)) + if (!rpmostree_nevra_to_cache_branch (nevra, &cache_branch, error)) return FALSE; if (expected_sha256 != NULL) @@ -1001,43 +969,6 @@ checkout_pkg_metadata_by_nevra (RpmOstreeContext *self, return checkout_pkg_metadata (self, nevra, header, cancellable, error); } -/* Fetches decomposed NEVRA information from pkgcache for a given nevra string. Requires the - * package to have been unpacked with unpack_version 1.4+ */ -gboolean -rpmostree_get_nevra_from_pkgcache (OstreeRepo *repo, - const char *nevra, - char **out_name, - guint64 *out_epoch, - char **out_version, - char **out_release, - char **out_arch, - GCancellable *cancellable, - GError **error) -{ - g_autofree char *ref = NULL; - if (!rpmostree_find_cache_branch_by_nevra (repo, nevra, &ref, cancellable, error)) - return FALSE; - - g_autofree char *rev = NULL; - if (!ostree_repo_resolve_rev (repo, ref, FALSE, &rev, error)) - return FALSE; - - g_autoptr(GVariant) commit = NULL; - if (!ostree_repo_load_commit (repo, rev, &commit, NULL, error)) - return FALSE; - - g_autoptr(GVariant) meta = g_variant_get_child_value (commit, 0); - g_autoptr(GVariantDict) dict = g_variant_dict_new (meta); - - g_autofree char *actual_nevra = NULL; - if (!g_variant_dict_lookup (dict, "rpmostree.nevra", "(sstsss)", &actual_nevra, - out_name, out_epoch, out_version, out_release, out_arch)) - return glnx_throw (error, "Cannot get nevra variant from commit metadata"); - - g_assert (g_str_equal (nevra, actual_nevra)); - return TRUE; -} - /* Initiate download of rpm-md */ gboolean rpmostree_context_download_metadata (RpmOstreeContext *self, diff --git a/src/libpriv/rpmostree-core.h b/src/libpriv/rpmostree-core.h index 90c241d5..83e880d5 100644 --- a/src/libpriv/rpmostree-core.h +++ b/src/libpriv/rpmostree-core.h @@ -123,13 +123,6 @@ gboolean rpmostree_context_get_state_sha512 (RpmOstreeContext *self, char **out_checksum, GError **error); -gboolean -rpmostree_find_cache_branch_by_nevra (OstreeRepo *pkgcache, - const char *nevra, - char **out_cache_branch, - GCancellable *cancellable, - GError **error); - gboolean rpmostree_pkgcache_find_pkg_header (OstreeRepo *pkgcache, const char *nevra, @@ -138,17 +131,6 @@ rpmostree_pkgcache_find_pkg_header (OstreeRepo *pkgcache, GCancellable *cancellable, GError **error); -gboolean -rpmostree_get_nevra_from_pkgcache (OstreeRepo *repo, - const char *nevra, - char **out_name, - guint64 *out_epoch, - char **out_version, - char **out_release, - char **out_arch, - GCancellable *cancellable, - GError **error); - gboolean rpmostree_context_download_metadata (RpmOstreeContext *context, DnfContextSetupSackFlags flags, GCancellable *cancellable, diff --git a/src/libpriv/rpmostree-rpm-util.c b/src/libpriv/rpmostree-rpm-util.c index 64bdebed..70238759 100644 --- a/src/libpriv/rpmostree-rpm-util.c +++ b/src/libpriv/rpmostree-rpm-util.c @@ -1199,7 +1199,64 @@ rpmostree_create_rpmdb_pkglist_variant (int dfd, return TRUE; } -/* translates cachebranch back to nevra */ +/* Simple wrapper around hy_split_nevra() that adds allow-none and GError convention */ +gboolean +rpmostree_decompose_nevra (const char *nevra, + char **out_name, /* allow-none */ + guint64 *out_epoch, /* allow-none */ + char **out_version, /* allow-none */ + char **out_release, /* allow-none */ + char **out_arch, /* allow-none */ + GError **error) +{ + g_autofree char *name = NULL; + int epoch; + g_autofree char *version = NULL; + g_autofree char *release = NULL; + g_autofree char *arch = NULL; + + if (hy_split_nevra (nevra, &name, &epoch, &version, &release, &arch) != 0) + return glnx_throw (error, "Failed to decompose NEVRA string '%s'", nevra); + + if (out_name) + *out_name = g_steal_pointer (&name); + if (out_epoch) + *out_epoch = epoch; /* note widening */ + if (out_version) + *out_version = g_steal_pointer (&version); + if (out_release) + *out_release = g_steal_pointer (&release); + if (out_arch) + *out_arch = g_steal_pointer (&arch); + + return TRUE; +} + +/* translates NEVRA to its cache branch */ +gboolean +rpmostree_nevra_to_cache_branch (const char *nevra, + char **cache_branch, + GError **error) +{ + /* It's cumbersome, but we have to decompose the NEVRA and the rebuild it into a cache + * branch. Something something Rust slices... */ + + g_autofree char *name = NULL; + guint64 epoch; + g_autofree char *version = NULL; + g_autofree char *release = NULL; + g_autofree char *arch = NULL; + + if (!rpmostree_decompose_nevra (nevra, &name, &epoch, &version, &release, &arch, error)) + return FALSE; + + g_autofree char *evr = rpmostree_custom_nevra_strdup (name, epoch, version, release, arch, + PKG_NEVRA_FLAGS_EVR); + *cache_branch = rpmostree_get_cache_branch_for_n_evr_a (name, evr, arch); + return TRUE; +} + +/* translates cachebranch back to nevra (inverse of rpmostree_cache_branch_to_nevra) */ char * rpmostree_cache_branch_to_nevra (const char *cachebranch) { diff --git a/src/libpriv/rpmostree-rpm-util.h b/src/libpriv/rpmostree-rpm-util.h index 306fb995..c73d3946 100644 --- a/src/libpriv/rpmostree-rpm-util.h +++ b/src/libpriv/rpmostree-rpm-util.h @@ -211,5 +211,18 @@ char *rpmostree_get_rojig_branch_header (Header hdr); char *rpmostree_get_cache_branch_pkg (DnfPackage *pkg); char *rpmostree_get_rojig_branch_pkg (DnfPackage *pkg); +gboolean +rpmostree_decompose_nevra (const char *nevra, + char **out_name, /* allow-none */ + guint64 *out_epoch, /* allow-none */ + char **out_version, /* allow-none */ + char **out_release, /* allow-none */ + char **out_arch, /* allow-none */ + GError **error); + +gboolean +rpmostree_nevra_to_cache_branch (const char *nevra, + char **cache_branch, + GError **error); char * rpmostree_cache_branch_to_nevra (const char *cachebranch); diff --git a/tests/check/test-utils.c b/tests/check/test-utils.c index a195ebaa..2d4bce93 100644 --- a/tests/check/test-utils.c +++ b/tests/check/test-utils.c @@ -6,7 +6,7 @@ #include #include "libglnx.h" -#include "rpmostree-util.h" +#include "rpmostree-rpm-util.h" #include "rpmostree-core.h" #include "rpmostree-importer.h" #include "libtest.h" @@ -66,6 +66,10 @@ test_one_cache_branch_to_nevra (const char *cache_branch, rpmostree_cache_branch_to_nevra (cache_branch); g_print ("comparing %s to %s\n", expected_nevra, actual_nevra); g_assert (g_str_equal (expected_nevra, actual_nevra)); + + g_autofree char *actual_branch = NULL; + g_assert (rpmostree_nevra_to_cache_branch (expected_nevra, &actual_branch, NULL)); + g_assert (g_str_equal (cache_branch, actual_branch)); } static void @@ -204,8 +208,8 @@ test_variant_to_nevra(void) g_autofree char *trelease; g_autofree char *tarch; - ret = rpmostree_get_nevra_from_pkgcache (repo, nevra, &tname, &tepoch, &tversion, - &trelease, &tarch, NULL, &error); + ret = rpmostree_decompose_nevra (nevra, &tname, &tepoch, &tversion, + &trelease, &tarch, &error); g_assert_no_error (error); g_assert (ret);