diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index 458b2a50cc..8652d82b71 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -79,6 +79,7 @@ static char *opt_write_composejson_to; static gboolean opt_no_parent; static char *opt_write_lockfile_to; static char **opt_lockfiles; +static gboolean opt_lockfile_strict; static char *opt_parent; /* shared by both install & commit */ @@ -104,6 +105,7 @@ static GOptionEntry install_option_entries[] = { { "workdir-tmpfs", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_workdir_tmpfs, "Use tmpfs for working state", NULL }, { "ex-write-lockfile-to", 0, 0, G_OPTION_ARG_STRING, &opt_write_lockfile_to, "Write lockfile to FILE", "FILE" }, { "ex-lockfile", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_lockfiles, "Read lockfile from FILE", "FILE" }, + { "ex-lockfile-strict", 0, 0, G_OPTION_ARG_NONE, &opt_lockfile_strict, "With --ex-lockfile, require full match", NULL }, { NULL } }; @@ -702,7 +704,7 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, g_autoptr(GHashTable) map = ror_lockfile_read (opt_lockfiles, error); if (!map) return FALSE; - rpmostree_context_set_vlockmap (self->corectx, map); + rpmostree_context_set_vlockmap (self->corectx, map, opt_lockfile_strict); g_print ("Loaded lockfiles:\n %s\n", g_strjoinv ("\n ", opt_lockfiles)); } diff --git a/src/libpriv/rpmostree-core-private.h b/src/libpriv/rpmostree-core-private.h index c0ce31bca2..03cd7f19c7 100644 --- a/src/libpriv/rpmostree-core-private.h +++ b/src/libpriv/rpmostree-core-private.h @@ -73,6 +73,7 @@ struct _RpmOstreeContext { GHashTable *pkgs_to_replace; /* new gv_nevra --> old gv_nevra */ GHashTable *vlockmap; /* nevra --> repochecksum */ + gboolean vlockmap_strict; GLnxTmpDir tmpdir; diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c index 1614bd0f1b..a44e01bcba 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -1846,48 +1846,60 @@ setup_rojig_state (RpmOstreeContext *self, return TRUE; } -static gboolean -check_locked_pkgs (RpmOstreeContext *self, - GError **error) +/* Return all the packages that match lockfile constraints. Multiple packages may be + * returned per NEVRA so that libsolv can respect e.g. repo costs. */ +static GPtrArray* +find_locked_packages (RpmOstreeContext *self, + GError **error) { - if (!self->vlockmap) - return TRUE; - - HyGoal goal = dnf_context_get_goal (self->dnfctx); - - /* sanity check it's all pure installs */ - { g_autoptr(GPtrArray) packages = dnf_goal_get_packages (goal, - DNF_PACKAGE_INFO_REMOVE, - DNF_PACKAGE_INFO_OBSOLETE, - DNF_PACKAGE_INFO_REINSTALL, - DNF_PACKAGE_INFO_UPDATE, - DNF_PACKAGE_INFO_DOWNGRADE, - -1); - if (packages->len > 0) - return throw_package_list (error, "Packages not marked for pure install", packages); - } - - g_autoptr(GPtrArray) packages = - dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_INSTALL, -1); + g_assert (self->vlockmap); + DnfContext *dnfctx = self->dnfctx; + DnfSack *sack = dnf_context_get_sack (dnfctx); - for (guint i = 0; i < packages->len; i++) + g_autoptr(GPtrArray) pkgs = + g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref); + GLNX_HASH_TABLE_FOREACH_KV (self->vlockmap, const char*, nevra, const char*, chksum) { - DnfPackage *pkg = packages->pdata[i]; - const char *nevra = dnf_package_get_nevra (pkg); - const char *chksum = g_hash_table_lookup (self->vlockmap, nevra); - if (!chksum || !*chksum) - continue; + g_assert (chksum); + hy_autoquery HyQuery query = hy_query_create (sack); + hy_query_filter (query, HY_PKG_NEVRA_STRICT, HY_EQ, nevra); + g_autoptr(GPtrArray) matches = hy_query_run (query); - g_autofree char *repodata_chksum = NULL; - if (!rpmostree_get_repodata_chksum_repr (pkg, &repodata_chksum, error)) - return FALSE; + gboolean at_least_one = FALSE; + guint n_checksum_mismatches = 0; + for (guint i = 0; i < matches->len; i++) + { + DnfPackage *match = matches->pdata[i]; + if (!*chksum) + { + /* we could optimize this path outside the loop in the future using the + * g_ptr_array_extend_and_steal API, though that's still too new for e.g. el8 */ + g_ptr_array_add (pkgs, g_object_ref (match)); + at_least_one = TRUE; + } + else + { + g_autofree char *repodata_chksum = NULL; + if (!rpmostree_get_repodata_chksum_repr (match, &repodata_chksum, error)) + return NULL; - if (chksum && !g_str_equal (chksum, repodata_chksum)) - return glnx_throw (error, "Locked package %s checksum mismatch: expected %s, got %s", - nevra, chksum, repodata_chksum); + if (!g_str_equal (chksum, repodata_chksum)) + n_checksum_mismatches++; + else + { + g_ptr_array_add (pkgs, g_object_ref (match)); + at_least_one = TRUE; + } + } + } + if (!at_least_one) + return glnx_null_throw (error, "Couldn't find locked package '%s'%s%s " + "(pkgs matching NEVRA: %d; mismatched checksums: %d)", + nevra, *chksum ? " with checksum " : "", + *chksum ? chksum : "", matches->len, n_checksum_mismatches); } - return TRUE; + return g_steal_pointer (&pkgs); } /* Check for/download new rpm-md, then depsolve */ @@ -1930,6 +1942,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, journal_rpmmd_info (self); } DnfSack *sack = dnf_context_get_sack (dnfctx); + HyGoal goal = dnf_context_get_goal (dnfctx); /* Don't try to keep multiple kernels per root; that's a traditional thing, * ostree binds kernel + userspace. @@ -1945,6 +1958,15 @@ rpmostree_context_prepare (RpmOstreeContext *self, g_assert_cmpint (g_strv_length (removed_base_pkgnames), ==, 0); } + if (self->vlockmap) + { + /* we only support pure installs for now (compose case) */ + g_assert (!self->rojig_pure); + g_assert_cmpint (g_strv_length (cached_pkgnames), ==, 0); + g_assert_cmpint (g_strv_length (cached_replace_pkgs), ==, 0); + g_assert_cmpint (g_strv_length (removed_base_pkgnames), ==, 0); + } + /* track cached pkgs already added to the sack so far */ g_autoptr(GHashTable) local_pkgs_to_install = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); @@ -1999,40 +2021,60 @@ rpmostree_context_prepare (RpmOstreeContext *self, * uninstall. We don't want to mix those two steps, otherwise we might confuse libdnf, * see: https://github.com/rpm-software-management/libdnf/issues/700 */ - /* Before adding any pkgs to the goal; filter out from the sack all pkgs which don't match - * our lockfile. (But of course, leave other pkgs untouched.) */ if (self->vlockmap != NULL) { - g_assert_cmpuint (g_strv_length (cached_replace_pkgs), ==, 0); - g_assert_cmpuint (g_strv_length (removed_base_pkgnames), ==, 0); + /* first, find our locked pkgs in the rpmmd */ + g_autoptr(GPtrArray) locked_pkgs = find_locked_packages (self, error); + if (!locked_pkgs) + return FALSE; - GLNX_HASH_TABLE_FOREACH_KV (self->vlockmap, const char*, nevra, const char*, chksum) + /* build a packageset from it */ + DnfPackageSet *locked_pset = dnf_packageset_new (sack); + for (guint i = 0; i < locked_pkgs->len; i++) + dnf_packageset_add (locked_pset, locked_pkgs->pdata[i]); + + if (self->vlockmap_strict) { - g_autofree char *name = NULL; - if (!rpmostree_decompose_nevra (nevra, &name, NULL, NULL, NULL, NULL, error)) - return FALSE; - hy_autoquery HyQuery query = hy_query_create (sack); - hy_query_filter (query, HY_PKG_NAME, HY_EQ, name); - g_autoptr(GPtrArray) pkglist = hy_query_run (query); + /* In strict mode, we basically *only* want locked packages to be considered, so + * exclude everything else. Note we still don't directly do `hy_goal_install` + * here; we want the treefile to still be canonical, but we just make sure that + * the end result matches what we expect. */ DnfPackageSet *pset = dnf_packageset_new (sack); - for (guint i = 0; i < pkglist->len; i++) - { - DnfPackage *pkg = pkglist->pdata[i]; - const char *pkg_nevra = dnf_package_get_nevra (pkg); - if (!g_str_equal (pkg_nevra, nevra)) - dnf_packageset_add (pset, pkg); - else if (chksum && *chksum) - { - g_autofree char *pkg_chksum = NULL; - if (!rpmostree_get_repodata_chksum_repr (pkg, &pkg_chksum, error)) - return FALSE; - if (!g_str_equal (chksum, pkg_chksum)) - dnf_packageset_add (pset, pkg); - } - } + Map *map = dnf_packageset_get_map (pset); + map_setall (map); + map_subtract (map, dnf_packageset_get_map (locked_pset)); dnf_sack_add_excludes (sack, pset); dnf_packageset_free (pset); } + else + { + /* In relaxed mode, we allow packages to be added or removed without having to + * edit lockfiles. However, we still want to make sure that if a package does get + * installed which is in the lockfile, it can only pick that NEVRA. To do this, we + * filter out from the sack all pkgs which don't match. */ + + /* map of all packages with names found in the lockfile */ + DnfPackageSet *named_pkgs = dnf_packageset_new (sack); + Map *named_pkgs_map = dnf_packageset_get_map (named_pkgs); + GLNX_HASH_TABLE_FOREACH (self->vlockmap, const char*, nevra) + { + g_autofree char *name = NULL; + if (!rpmostree_decompose_nevra (nevra, &name, NULL, NULL, NULL, NULL, error)) + return FALSE; + hy_autoquery HyQuery query = hy_query_create (sack); + hy_query_filter (query, HY_PKG_NAME, HY_EQ, name); + DnfPackageSet *pset = hy_query_run_set (query); + map_or (named_pkgs_map, dnf_packageset_get_map (pset)); + dnf_packageset_free (pset); + } + + /* remove our locked packages from the exclusion set */ + map_subtract (named_pkgs_map, dnf_packageset_get_map (locked_pset)); + dnf_sack_add_excludes (sack, named_pkgs); + dnf_packageset_free (named_pkgs); + } + + dnf_packageset_free (locked_pset); } /* Process excludes */ @@ -2059,7 +2101,6 @@ rpmostree_context_prepare (RpmOstreeContext *self, } /* Then, handle local packages to install */ - HyGoal goal = dnf_context_get_goal (dnfctx); GLNX_HASH_TABLE_FOREACH_V (local_pkgs_to_install, DnfPackage*, pkg) hy_goal_install (goal, pkg); @@ -2108,8 +2149,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, /* XXX: consider a --allow-uninstall switch? */ if (!dnf_goal_depsolve (goal, actions, error) || - !check_goal_solution (self, removed_pkgnames, replaced_nevras, error) || - !check_locked_pkgs (self, error)) + !check_goal_solution (self, removed_pkgnames, replaced_nevras, error)) return FALSE; g_clear_pointer (&self->pkgs, (GDestroyNotify)g_ptr_array_unref); self->pkgs = dnf_goal_get_packages (dnf_context_get_goal (dnfctx), @@ -2177,11 +2217,15 @@ rpmostree_context_get_packages_to_import (RpmOstreeContext *self) return g_ptr_array_ref (self->pkgs_to_import); } +/* Note this must be called *before* rpmostree_context_setup(). */ void -rpmostree_context_set_vlockmap (RpmOstreeContext *self, GHashTable *map) +rpmostree_context_set_vlockmap (RpmOstreeContext *self, + GHashTable *map, + gboolean strict) { g_clear_pointer (&self->vlockmap, (GDestroyNotify)g_hash_table_unref); self->vlockmap = g_hash_table_ref (map); + self->vlockmap_strict = strict; } /* XXX: push this into libdnf */ diff --git a/src/libpriv/rpmostree-core.h b/src/libpriv/rpmostree-core.h index 80ba69ce5a..089953a86e 100644 --- a/src/libpriv/rpmostree-core.h +++ b/src/libpriv/rpmostree-core.h @@ -173,7 +173,10 @@ gboolean rpmostree_context_set_packages (RpmOstreeContext *self, GPtrArray *rpmostree_context_get_packages_to_import (RpmOstreeContext *self); -void rpmostree_context_set_vlockmap (RpmOstreeContext *self, GHashTable *map); +void +rpmostree_context_set_vlockmap (RpmOstreeContext *self, + GHashTable *map, + gboolean strict); gboolean rpmostree_context_download (RpmOstreeContext *self, GCancellable *cancellable, diff --git a/tests/compose/libcomposetest.sh b/tests/compose/libcomposetest.sh index 555570f631..ed36946be1 100644 --- a/tests/compose/libcomposetest.sh +++ b/tests/compose/libcomposetest.sh @@ -56,6 +56,10 @@ else: tf['$1'] += $2" } +treefile_remove() { + treefile_pyedit "tf['$1'].remove($2)" +} + # for tests that need direct control on rpm-ostree export compose_base_argv="\ --unified-core \ diff --git a/tests/compose/test-lockfile.sh b/tests/compose/test-lockfile.sh index dba59a899e..11aa45f366 100755 --- a/tests/compose/test-lockfile.sh +++ b/tests/compose/test-lockfile.sh @@ -58,15 +58,96 @@ EOF runcompose \ --ex-lockfile="$PWD/versions.lock" \ --ex-lockfile="$PWD/override.lock" \ - --ex-write-lockfile-to="$PWD/versions.lock" \ + --ex-write-lockfile-to="$PWD/versions.lock.new" \ --dry-run "${treefile}" |& tee out.txt -echo "ok compose with lockfile" - assert_file_has_content out.txt 'test-pkg-1.0-1.x86_64' assert_file_has_content out.txt 'test-pkg-common-1.0-1.x86_64' assert_file_has_content out.txt 'another-test-pkg-2.0-1.x86_64' -assert_jq versions.lock \ +assert_jq versions.lock.new \ '.packages["test-pkg"].evra = "1.0-1.x86_64"' \ '.packages["test-pkg-common"].evra = "1.0-1.x86_64"' \ '.packages["another-test-pkg"].evra = "2.0-1.x86_64"' echo "ok override" + +# sanity-check that we can remove packages in relaxed mode +treefile_remove "packages" '"another-test-pkg"' +runcompose \ + --ex-lockfile="$PWD/versions.lock" \ + --ex-lockfile="$PWD/override.lock" \ + --ex-write-lockfile-to="$PWD/versions.lock.new" \ + --dry-run "${treefile}" |& tee out.txt +assert_file_has_content out.txt 'test-pkg-1.0-1.x86_64' +assert_file_has_content out.txt 'test-pkg-common-1.0-1.x86_64' +assert_not_file_has_content out.txt 'another-test-pkg' +echo "ok relaxed mode can remove pkg" + +# test strict mode + +# sanity-check that refeeding the output lockfile as input satisfies strict mode +mv versions.lock{.new,} +runcompose \ + --ex-lockfile-strict \ + --ex-lockfile="$PWD/versions.lock" \ + --ex-write-lockfile-to="$PWD/versions.lock.new" \ + --dry-run "${treefile}" |& tee out.txt +assert_streq \ + "$(jq .packages versions.lock | sha256sum)" \ + "$(jq .packages versions.lock.new | sha256sum)" +echo "ok strict mode sanity check" + +# check that trying to install a pkg that's not in the lockfiles fails +build_rpm unlocked-pkg +treefile_append "packages" '["unlocked-pkg"]' +if runcompose \ + --ex-lockfile-strict \ + --ex-lockfile="$PWD/versions.lock" \ + --dry-run "${treefile}" &>err.txt; then + fatal "compose unexpectedly succeeded" +fi +assert_file_has_content err.txt 'Packages not found: unlocked-pkg' +echo "ok strict mode no unlocked pkgs" + +# check that a locked pkg with unlocked deps causes an error +build_rpm unlocked-pkg version 2.0 requires unlocked-pkg-dep +build_rpm unlocked-pkg-dep version 2.0 +# notice we add unlocked-pkg, but not unlocked-pkg-dep +cat > override.lock <err.txt; then + fatal "compose unexpectedly succeeded" +fi +assert_file_has_content err.txt 'Could not depsolve transaction' +assert_file_has_content err.txt 'unlocked-pkg-dep-2.0-1.x86_64 is filtered out by exclude filtering' +treefile_remove "packages" '"unlocked-pkg"' +echo "ok strict mode no unlocked pkg deps" + +# check that a locked pkg which isn't actually in the repos causes an error +cat > override.lock <err.txt; then + fatal "compose unexpectedly succeeded" +fi +assert_file_has_content err.txt "Couldn't find locked package 'unmatched-pkg-1.0-1.x86_64'" +echo "ok strict mode locked pkg missing from rpmmd"