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 27bc32f042..fc85f26252 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -1565,147 +1565,58 @@ 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. - */ +/* For every package which was replaced (i.e. upgraded or downgraded), collects into + * self->pkgs_to_replace the (old pkg, new pkg) tuples so we can inject it later on in the + * commit metadata (this then gets displaying in `rpm-ostree status` for example). We do + * this right after depsolving. */ static gboolean -check_goal_solution (RpmOstreeContext *self, - GPtrArray *removed_pkgnames, - GPtrArray *replaced_nevras, - GError **error) +collect_replaced_packages (RpmOstreeContext *self, + GPtrArray *replaced_pkgnames, + GError **error) { - HyGoal goal = dnf_context_get_goal (self->dnfctx); - - /* check that we're not removing anything we didn't expect */ - { g_autoptr(GPtrArray) forbidden = g_ptr_array_new_with_free_func (g_free); - - /* also collect info about those we're removing */ - g_assert (!self->pkgs_to_remove); - self->pkgs_to_remove = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, - (GDestroyNotify)g_variant_unref); - g_autoptr(GPtrArray) packages = dnf_goal_get_packages (goal, - DNF_PACKAGE_INFO_REMOVE, - DNF_PACKAGE_INFO_OBSOLETE, -1); - for (guint i = 0; i < packages->len; i++) - { - DnfPackage *pkg = packages->pdata[i]; - const char *name = dnf_package_get_name (pkg); - const char *nevra = dnf_package_get_nevra (pkg); - - /* did we expect this package to be removed? */ - if (rpmostree_str_ptrarray_contains (removed_pkgnames, name)) - g_hash_table_insert (self->pkgs_to_remove, g_strdup (name), - gv_nevra_from_pkg (pkg)); - else - g_ptr_array_add (forbidden, g_strdup (nevra)); - } - - if (forbidden->len > 0) - return throw_package_list (error, "Base packages would be removed", forbidden); - } - - /* check that all the pkgs we expect to remove are marked for removal */ - { g_autoptr(GPtrArray) forbidden = g_ptr_array_new (); - - for (guint i = 0; i < removed_pkgnames->len; i++) - { - const char *pkgname = removed_pkgnames->pdata[i]; - if (!g_hash_table_contains (self->pkgs_to_remove, pkgname)) - g_ptr_array_add (forbidden, (gpointer)pkgname); - } - - if (forbidden->len > 0) - return throw_package_list (error, "Base packages not marked to be removed", forbidden); - } + g_assert (self->dnfctx); + /* it's programmer error to call this function twice */ + g_assert (!self->pkgs_to_replace); - /* REINSTALLs should never happen since it doesn't make sense in the rpm-ostree flow, and - * we check very early whether a package is already in the rootfs or not, but let's check - * for it anyway so that we get a bug report in case it somehow happens. */ - { g_autoptr(GPtrArray) packages = - dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_REINSTALL, -1); - g_assert_cmpint (packages->len, ==, 0); - } + HyGoal goal = dnf_context_get_goal (self->dnfctx); - /* Look at UPDATE and DOWNGRADE, and see whether they're doing what we expect */ - { g_autoptr(GHashTable) forbidden_replacements = - g_hash_table_new_full (NULL, NULL, (GDestroyNotify)g_object_unref, - (GDestroyNotify)g_object_unref); - - /* also collect info about those we're replacing */ - g_assert (!self->pkgs_to_replace); - self->pkgs_to_replace = g_hash_table_new_full (gv_nevra_hash, g_variant_equal, - (GDestroyNotify)g_variant_unref, - (GDestroyNotify)g_variant_unref); - g_autoptr(GPtrArray) packages = dnf_goal_get_packages (goal, - DNF_PACKAGE_INFO_UPDATE, - DNF_PACKAGE_INFO_DOWNGRADE, -1); - for (guint i = 0; i < packages->len; i++) - { - DnfPackage *pkg = packages->pdata[i]; - const char *nevra = dnf_package_get_nevra (pkg); - - /* just pick the first pkg */ - g_autoptr(GPtrArray) old = hy_goal_list_obsoleted_by_package (goal, pkg); - g_assert_cmpint (old->len, >, 0); - DnfPackage *old_pkg = old->pdata[0]; - - /* did we expect this nevra to replace a base pkg? */ - if (rpmostree_str_ptrarray_contains (replaced_nevras, nevra)) - g_hash_table_insert (self->pkgs_to_replace, gv_nevra_from_pkg (pkg), - gv_nevra_from_pkg (old_pkg)); - else - g_hash_table_insert (forbidden_replacements, g_object_ref (old_pkg), - g_object_ref (pkg)); - } + g_autoptr(GHashTable) replaced_pkgs = + g_hash_table_new_full (gv_nevra_hash, g_variant_equal, (GDestroyNotify)g_variant_unref, + (GDestroyNotify)g_variant_unref); + g_autoptr(GPtrArray) packages = dnf_goal_get_packages (goal, + DNF_PACKAGE_INFO_UPDATE, + DNF_PACKAGE_INFO_DOWNGRADE, -1); + g_autoptr(GPtrArray) forbidden = NULL; + for (guint i = 0; i < packages->len; i++) + { + DnfPackage *pkg = packages->pdata[i]; + const char *pkgname = dnf_package_get_name (pkg); + g_assert (pkgname); - if (g_hash_table_size (forbidden_replacements) > 0) - { - rpmostree_output_message ("Forbidden base package replacements:"); - GLNX_HASH_TABLE_FOREACH_KV (forbidden_replacements, DnfPackage*, old_pkg, - DnfPackage*, new_pkg) - { - const char *old_name = dnf_package_get_name (old_pkg); - const char *new_name = dnf_package_get_name (new_pkg); - const char *new_repo = dnf_package_get_reponame (new_pkg); - if (g_str_equal (old_name, new_name)) - rpmostree_output_message (" %s %s -> %s (%s)", old_name, - dnf_package_get_evr (old_pkg), - dnf_package_get_evr (new_pkg), new_repo); - else - rpmostree_output_message (" %s -> %s (%s)", - dnf_package_get_nevra (old_pkg), - dnf_package_get_nevra (new_pkg), new_repo); - } - rpmostree_output_message ("This likely means that some of your layered packages " - "have requirements on newer or older versions of some " - "base packages. Doing `rpm-ostree cleanup -m` and " - "`rpm-ostree upgrade` first may help. For " - "more details, see: " - "https://github.com/projectatomic/rpm-ostree/issues/415"); - - return glnx_throw (error, "Some base packages would be replaced"); - } - } + /* just pick the first pkg */ + g_autoptr(GPtrArray) old = hy_goal_list_obsoleted_by_package (goal, pkg); + g_assert_cmpint (old->len, >, 0); + DnfPackage *old_pkg = old->pdata[0]; - /* check that all the pkgs we expect to replace are marked for replacement */ - { g_autoptr(GPtrArray) forbidden = g_ptr_array_new_with_free_func (g_free); + /* just a final sanity-check here that it's a package we expected to be replaced */ + if (!rpmostree_str_ptrarray_contains (replaced_pkgnames, pkgname)) + { + if (!forbidden) + forbidden = g_ptr_array_new_with_free_func (g_free); + g_ptr_array_add (forbidden, g_strdup_printf ("%s -> %s", + dnf_package_get_nevra (old_pkg), + dnf_package_get_nevra (pkg))); + continue; + } - GLNX_HASH_TABLE_FOREACH_KV (self->pkgs_to_replace, GVariant*, new, GVariant*, old) - { - g_autoptr(GVariant) nevra_v = g_variant_get_child_value (new, 0); - const char *nevra = g_variant_get_string (nevra_v, NULL); - if (!rpmostree_str_ptrarray_contains (replaced_nevras, nevra)) - g_ptr_array_add (forbidden, g_strdup (nevra)); - } + g_hash_table_insert (replaced_pkgs, gv_nevra_from_pkg (pkg), + gv_nevra_from_pkg (old_pkg)); + } - if (forbidden->len > 0) - return throw_package_list (error, "Base packages not marked to be installed", forbidden); - } + if (forbidden && forbidden->len > 0) + return throw_package_list (error, "Unexpected upgrade/downgrade", forbidden); + self->pkgs_to_replace = g_steal_pointer (&replaced_pkgs); return TRUE; } @@ -2003,7 +1914,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); /* 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; @@ -2017,7 +1928,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, if (!add_pkg_from_cache (self, nevra, sha256, &pkg, cancellable, error)) return FALSE; - g_ptr_array_add (replaced_nevras, (gpointer)nevra); + g_ptr_array_add (replaced_pkgnames, g_strdup (dnf_package_get_name (pkg))); g_hash_table_insert (local_pkgs_to_install, (gpointer)nevra, g_steal_pointer (&pkg)); } @@ -2175,6 +2086,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)) @@ -2194,11 +2123,12 @@ rpmostree_context_prepare (RpmOstreeContext *self, rpmostree_output_task_begin (&task, "Resolving dependencies"); /* XXX: consider a --allow-uninstall switch? */ - if (!dnf_goal_depsolve (goal, actions, error) || - !check_goal_solution (self, removed_pkgnames, replaced_nevras, error)) + if (!dnf_goal_depsolve (goal, actions, error)) + return FALSE; + if (!collect_replaced_packages (self, replaced_pkgnames, 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