Skip to content

Commit

Permalink
core: Use SOLVER_LOCK for locking base packages
Browse files Browse the repository at this point in the history 
For the Fedora CoreOS extensions work, when layering packages, we need
to be able to tell libsolv to pick the packages which will go with the
base packages. IOW, it needs to know that the base packages shouldn't be
uninstalled.

While investigating
coreos/fedora-coreos-tracker#525, I realized
that libsolv does have a flag which allows us to express this:
`SOLVER_LOCK`.

This then allows libsolv to choose the right package for us (if found).
And in the case where it can't find a matching package, libsolv itself
will print exactly what the conflict is, allowing us to avoid
sanity-checking the solution post-solve for "forbidden replacements".

Update submodule: libdnf
  • Loading branch information
@jlebon
jlebon committed Aug 26, 2020
1 parent f0b2bef commit 5812912
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 140 deletions.
2 changes: 1 addition & 1 deletion libdnf
Submodule libdnf updated 11 files
+1 −0 data/tests/hawkey/@System.repo
+6 −0 data/tests/hawkey/updates.repo
+6 −0 libdnf/goal/Goal.cpp
+1 −0 libdnf/goal/Goal.hpp
+7 −0 libdnf/hy-goal.cpp
+10 −0 libdnf/hy-goal.h
+6 −6 python/hawkey/tests/tests/test_query.py
+4 −0 tests/hawkey/README
+99 −11 tests/hawkey/test_goal.cpp
+4 −4 tests/hawkey/test_query.cpp
+2 −2 tests/hawkey/testshared.h
204 changes: 67 additions & 137 deletions src/libpriv/rpmostree-core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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));
}

Expand Down Expand Up @@ -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))
Expand All @@ -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);
Expand Down
25 changes: 23 additions & 2 deletions tests/vmcheck/test-layering-basic-1.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5812912

Please sign in to comment.