diff --git a/libdnf b/libdnf index 6043874b95..240fdd4f14 160000 --- a/libdnf +++ b/libdnf @@ -1 +1 @@ -Subproject commit 6043874b9542f8dcedd9f0be25e1044b1b4d6a66 +Subproject commit 240fdd4f14d9b552067ee713f858b5d19b624b99 diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c index cd50d0b1a0..7591a862d5 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -1573,12 +1573,14 @@ gv_nevra_hash (gconstpointer v) return g_str_hash (g_variant_get_string (nevra, NULL)); } -/* We need to make sure that only the pkgs in the base allowed to be - * removed are removed. The issue is that marking a package for DNF_INSTALL - * could uninstall a base pkg if it's an update or obsoletes it. There doesn't - * seem to be a way to tell libsolv to not touch some pkgs in the base layer, - * so we just inspect its solution in retrospect. libdnf has the concept of - * protected packages, but it still allows updating protected packages. +/* Before hy_goal_lock(), this function was the only way for us to make sure that libsolv + * wasn't trying to modify a base package it wasn't supposed to. Its secondary purpose was + * to collect the packages being replaced and removed into `self->pkgs_to_replace` and + * `self->pkgs_to_remove` respectively. + * + * Nowadays, that secondary purpose is now its primary purpose because we're already + * guaranteed the first part by hy_goal_lock(). Though we still leave all those original + * checks in place as a fail-safe in case libsolv messes up. */ static gboolean check_goal_solution (RpmOstreeContext *self, @@ -2012,6 +2014,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, /* Handle packages to replace; only add them to the sack for now */ g_autoptr(GPtrArray) replaced_nevras = g_ptr_array_new (); + g_autoptr(GPtrArray) replaced_pkgnames = g_ptr_array_new_with_free_func (g_free); for (char **it = cached_replace_pkgs; it && *it; it++) { const char *nevra = *it; @@ -2025,7 +2028,13 @@ rpmostree_context_prepare (RpmOstreeContext *self, if (!add_pkg_from_cache (self, nevra, sha256, &pkg, cancellable, error)) return FALSE; + const char *name = dnf_package_get_name (pkg); + g_assert (name); + g_ptr_array_add (replaced_nevras, (gpointer)nevra); + // this is a bit wasteful, but for locking purposes, we need the pkgname to match + // against the base, not the nevra which naturally will be different + g_ptr_array_add (replaced_pkgnames, g_strdup (name)); g_hash_table_insert (local_pkgs_to_install, (gpointer)nevra, g_steal_pointer (&pkg)); } @@ -2183,6 +2192,24 @@ rpmostree_context_prepare (RpmOstreeContext *self, if (missing_pkgs && missing_pkgs->len > 0) return throw_package_list (error, "Packages not found", missing_pkgs); + /* And lock all the base packages we don't expect to be replaced. */ + { hy_autoquery HyQuery query = hy_query_create (sack); + hy_query_filter (query, HY_PKG_REPONAME, HY_EQ, HY_SYSTEM_REPO_NAME); + g_autoptr(GPtrArray) pkgs = hy_query_run (query); + for (guint i = 0; i < pkgs->len; i++) + { + DnfPackage *pkg = pkgs->pdata[i]; + const char *pkgname = dnf_package_get_name (pkg); + g_assert (pkgname); + + if (rpmostree_str_ptrarray_contains (removed_pkgnames, pkgname) || + rpmostree_str_ptrarray_contains (replaced_pkgnames, pkgname)) + continue; + if (hy_goal_lock (goal, pkg, error) != 0) + return glnx_prefix_error (error, "while locking pkg '%s'", pkgname); + } + } + if (self->rojig_spec) { if (!setup_rojig_state (self, error)) @@ -2206,7 +2233,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, !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), + self->pkgs = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_INSTALL, DNF_PACKAGE_INFO_UPDATE, DNF_PACKAGE_INFO_DOWNGRADE, -1); diff --git a/tests/vmcheck/test-layering-basic-1.sh b/tests/vmcheck/test-layering-basic-1.sh index 9820b44fbd..1e10c96363 100755 --- a/tests/vmcheck/test-layering-basic-1.sh +++ b/tests/vmcheck/test-layering-basic-1.sh @@ -206,15 +206,36 @@ echo "ok no leftover rpmdb files" # upgrade to a layer with foo already builtin vm_ostree_commit_layered_as_base $booted_csum vmcheck vm_rpmostree upgrade + +# check that we can't layer a pkg which wants to change a base pkg vm_build_rpm bar conflicts foo if vm_rpmostree install bar &> err.txt; then assert_not_reached "successfully layered conflicting pkg bar?" fi -assert_file_has_content err.txt "Base packages would be removed" +assert_file_has_content err.txt "conflicting requests" +assert_file_has_content err.txt "foo-1.0-1.x86_64" +echo "ok can't layer pkg that would remove base pkg" + +vm_build_rpm foo version 2.0 +vm_build_rpm foo-ext version 2.0 requires "foo = 2.0-1" +if vm_rpmostree install foo-ext &> err.txt; then + assert_not_reached "successfully layered updated split pkg foo-ext?" +fi +assert_file_has_content err.txt "conflicting requests" assert_file_has_content err.txt "foo-1.0-1.x86_64" +assert_file_has_content err.txt "foo-2.0-1.x86_64" +echo "ok can't layer pkg that would upgrade base pkg" + +# check that we can select a repo split pkg which matches the base version +vm_build_rpm foo version 1.0 +vm_build_rpm foo-ext version 1.0 requires "foo = 1.0-1" +vm_rpmostree install foo-ext +vm_assert_status_jq \ + '.deployments[0]["packages"]|length == 1' \ + '.deployments[0]["packages"]|index("foo-ext") >= 0' +echo "ok can layer split pkg matching base version" vm_rpmostree cleanup -p vm_cmd ostree reset vmcheck $(vm_cmd ostree rev-parse "vmcheck^") -echo "ok can't layer pkg that would remove base pkg" # check that root is a shared mount # https://bugzilla.redhat.com/show_bug.cgi?id=1318547