core: Use SOLVER_LOCK for locking base packages

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

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);

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