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, which is more informative than
the "forbidden replacements" error we currently print out.

Update submodule: libdnf
  • Loading branch information
jlebon authored and openshift-merge-robot committed Aug 28, 2020
1 parent 5ff769f commit 71992e3
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 10 deletions.
41 changes: 34 additions & 7 deletions src/libpriv/rpmostree-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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));
}

Expand Down Expand Up @@ -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))
Expand All @@ -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);
Expand Down
25 changes: 23 additions & 2 deletions tests/vmcheck/test-layering-basic-1.sh
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 71992e3

Please sign in to comment.