Skip to content

Commit

Permalink
WIP: Use SOLVER_LOCK for locking base packages
Browse files Browse the repository at this point in the history 
While investigating
coreos/fedora-coreos-tracker#525, I realized
that libsolv does have a flag which allows us to express that our base
packages shouldn't be upgraded: `SOLVER_LOCK`. This then lets us not
have to sanity-check the solution post-solve for "forbidden
replacements", and lets libsolv/libdnf print exactly what the conflict
is.
  • Loading branch information
@jlebon
jlebon committed Jun 9, 2020
1 parent 65156cb commit 8d916d3
Showing 1 changed file with 45 additions and 138 deletions.
183 changes: 45 additions & 138 deletions src/libpriv/rpmostree-core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1564,146 +1564,37 @@ 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.
*/
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);
}

/* 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);
}

/* 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));
}

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");
}
}
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 *pkgname = dnf_package_get_name (pkg);

/* 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 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];

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));
}
/* just a final sanity-check here that it's a package we expected to be replaced */
if (!rpmostree_str_ptrarray_contains (replaced_pkgnames, pkgname))
return glnx_throw (error, "Found unexpected upgrade/downgrade for '%s'", pkgname);

if (forbidden->len > 0)
return throw_package_list (error, "Base packages not marked to be installed", forbidden);
}
g_hash_table_insert (self->pkgs_to_replace, gv_nevra_from_pkg (pkg),
gv_nevra_from_pkg (old_pkg));
}

return TRUE;
}
Expand Down Expand Up @@ -2002,7 +1893,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 @@ -2016,7 +1907,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 @@ -2171,6 +2062,21 @@ rpmostree_context_prepare (RpmOstreeContext *self,
}
}

/* 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);
if (rpmostree_str_ptrarray_contains (removed_pkgnames, pkgname) ||
rpmostree_str_ptrarray_contains (replaced_pkgnames, pkgname))
continue;
hy_goal_lock (goal, pkg);
}
}

if (missing_pkgs && missing_pkgs->len > 0)
return throw_package_list (error, "Packages not found", missing_pkgs);

Expand All @@ -2193,11 +2099,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

0 comments on commit 8d916d3

Please sign in to comment.