core: Strengthen how we enforce lockfiles

One problem with how we use lockfiles right now is that we don't enforce
them for dependencies. That is, if `foo` requires `bar`, but only `foo`
is in the manifest, then while `foo` will be locked, `bar` will never
be checked against the lockfile because it was never explicitly
requested.

Higher-level though, I don't like how indirect the locking here feels.
See some comments about that in:

#1745 (comment)
#1745 (comment)

Essentially, the manifest is an input file of patterns, and all we
really know from the lockfile output is that the set of packages in
there satisfies this input in some way. But:

1. there are multiple ways to satisfy the same input (hence why hints
   like `SOLVER_FAVOR` exist)
2. the solution is dependent on how the solver is implemented (i.e.
   different libsolv versions might yield different solutions)
3. the solution is dependent on flags fed to the solver (i.e. different
   libdnf versions might yield different solutions)

So any attempt at cross-checking between the input file and the lockfile
is going to be very hard. Using a stricter mode as I suggested in #1745
of only allowing pure pkgnames or NEVRAs would help, but it wouldn't
address the dependency issue. (Though I'm still thinking about possibly
doing this anyway.)

The solution I propose here is instead to take the nuclear approach: we
completely exclude from the sack all packages of the same name as
packages in our lockfiles, but which do not match the NEVRA. Therefore,
any possible solution has to also satisfy our lockfile (or error out).

Closes: #1849
Approved by: cgwalters

src/libpriv/rpmostree-core.c

@@ -1830,6 +1830,50 @@ setup_rojig_state (RpmOstreeContext *self,
   return TRUE;
 }
 
+static gboolean
+check_locked_pkgs (RpmOstreeContext *self,
+                   GError          **error)
+{
+  if (!self->vlockmap)
+    return TRUE;
+
+  HyGoal goal = dnf_context_get_goal (self->dnfctx);
+
+  /* sanity check it's all pure installs */
+  { g_autoptr(GPtrArray) packages = dnf_goal_get_packages (goal,
+                                                           DNF_PACKAGE_INFO_REMOVE,
+                                                           DNF_PACKAGE_INFO_OBSOLETE,
+                                                           DNF_PACKAGE_INFO_REINSTALL,
+                                                           DNF_PACKAGE_INFO_UPDATE,
+                                                           DNF_PACKAGE_INFO_DOWNGRADE,
+                                                           -1);
+    if (packages->len > 0)
+      return throw_package_list (error, "Packages not marked for pure install", packages);
+  }
+
+  g_autoptr(GPtrArray) packages =
+    dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_INSTALL, -1);
+
+  for (guint i = 0; i < packages->len; i++)
+    {
+      DnfPackage *pkg = packages->pdata[i];
+      const char *nevra = dnf_package_get_nevra (pkg);
+      const char *chksum = g_hash_table_lookup (self->vlockmap, nevra);
+      if (!chksum)
+        continue;
+
+      g_autofree char *repodata_chksum = NULL;
+      if (!rpmostree_get_repodata_chksum_repr (pkg, &repodata_chksum, error))
+        return FALSE;
+
+      if (chksum && !g_str_equal (chksum, repodata_chksum))
+        return glnx_throw (error, "Locked package %s checksum mismatch: expected %s, got %s",
+                           nevra, chksum, repodata_chksum);
+    }
+
+  return TRUE;
+}
+
 /* Check for/download new rpm-md, then depsolve */
 gboolean
 rpmostree_context_prepare (RpmOstreeContext *self,
@@ -1936,6 +1980,27 @@ rpmostree_context_prepare (RpmOstreeContext *self,
    * uninstall. We don't want to mix those two steps, otherwise we might confuse libdnf,
    * see: https://github.com/rpm-software-management/libdnf/issues/700 */
 
+  /* Before adding any pkgs to the goal; filter out from the sack all pkgs which don't match
+   * our lockfile. (But of course, leave other pkgs untouched.) */
+  if (self->vlockmap != NULL)
+    {
+      g_assert_cmpuint (g_strv_length (cached_replace_pkgs), ==, 0);
+      g_assert_cmpuint (g_strv_length (removed_base_pkgnames), ==, 0);
+
+      GLNX_HASH_TABLE_FOREACH (self->vlockmap, const char*, nevra)
+        {
+          g_autofree char *name = NULL;
+          if (!rpmostree_decompose_nevra (nevra, &name, NULL, NULL, NULL, NULL, error))
+            return FALSE;
+          hy_autoquery HyQuery query = hy_query_create (sack);
+          hy_query_filter (query, HY_PKG_NAME, HY_EQ, name);
+          hy_query_filter (query, HY_PKG_NEVRA, HY_NEQ, nevra);
+          DnfPackageSet *pset = hy_query_run_set (query);
+          dnf_sack_add_excludes (sack, pset);
+          dnf_packageset_free (pset);
+        }
+    }
+
   /* First, handle packages to remove */
   g_autoptr(GPtrArray) removed_pkgnames = g_ptr_array_new ();
   for (char **it = removed_base_pkgnames; it && *it; it++)
@@ -1958,13 +2023,6 @@ rpmostree_context_prepare (RpmOstreeContext *self,
   for (char **it = pkgnames; it && *it; it++)
     {
       const char *pkgname = *it;
-      g_autoptr(DnfPackage) pkg = rpmostree_get_locked_package (sack, self->vlockmap, pkgname);
-      if (pkg)
-        {
-          hy_goal_install (goal, pkg);
-          continue;
-        }
-
       g_assert (!self->rojig_pure);
       g_autoptr(GError) local_error = NULL;
       if (!dnf_context_install (dnfctx, pkgname, &local_error))
@@ -2005,7 +2063,8 @@ rpmostree_context_prepare (RpmOstreeContext *self,
 
       /* XXX: consider a --allow-uninstall switch? */
       if (!dnf_goal_depsolve (goal, actions, error) ||
-          !check_goal_solution (self, removed_pkgnames, replaced_nevras, error))
+          !check_goal_solution (self, removed_pkgnames, replaced_nevras, error) ||
+          !check_locked_pkgs (self, error))
         return FALSE;
       g_clear_pointer (&self->pkgs, (GDestroyNotify)g_ptr_array_unref);
       self->pkgs = dnf_goal_get_packages (dnf_context_get_goal (dnfctx),

src/libpriv/rpmostree-rpm-util.c

@@ -1285,32 +1285,6 @@ rpmostree_create_pkglist_variant (GPtrArray     *pkglist,
   return TRUE;
 }
 
-DnfPackage *
-rpmostree_get_locked_package (DnfSack    *sack,
-                              GHashTable *lockmap,
-                              const char *name)
-{
-  if (!lockmap || !name)
-    return NULL;
-
-  /* The manifest might specify not only package names (foo-1.x) but also
-   * something-that-foo-provides */
-  g_autoptr(GPtrArray) alts = rpmostree_get_matching_packages (sack, name);
-
-  for (gsize i = 0; i < alts->len; i++)
-  {
-    DnfPackage *pkg = alts->pdata[i];
-    g_autofree gchar *repodata_chksum = NULL;
-    if (!rpmostree_get_repodata_chksum_repr (pkg, &repodata_chksum, NULL))
-      continue;
-
-    const char *chksum = g_hash_table_lookup (lockmap, dnf_package_get_nevra (pkg));
-    if (chksum && !g_strcmp0 (chksum, repodata_chksum))
-      return g_object_ref (pkg);
-  }
-  return NULL;
-}
-
 /* Simple wrapper around hy_split_nevra() that adds allow-none and GError convention */
 gboolean
 rpmostree_decompose_nevra (const char  *nevra,

src/libpriv/rpmostree-rpm-util.h

@@ -216,9 +216,6 @@ rpmostree_create_pkglist_variant (GPtrArray     *pkglist,
                                   GCancellable  *cancellable,
                                   GError       **error);
 
-DnfPackage *
-rpmostree_get_locked_package (DnfSack *sack, GHashTable *lockmap, const char *name);
-
 char * rpmostree_get_cache_branch_for_n_evr_a (const char *name, const char *evr, const char *arch);
 char *rpmostree_get_cache_branch_header (Header hdr);
 char *rpmostree_get_rojig_branch_header (Header hdr);

tests/compose-tests/test-lockfile.sh

@@ -7,9 +7,8 @@ dn=$(cd $(dirname $0) && pwd)
 prepare_compose_test "lockfile"
 # Add a local rpm-md repo so we can mutate local test packages
 pyappendjsonmember "repos" '["test-repo"]'
-build_rpm test-pkg \
-          files "/usr/bin/test-pkg" \
-          install "mkdir -p %{buildroot}/usr/bin && echo localpkg data > %{buildroot}/usr/bin/test-pkg"
+build_rpm test-pkg-common
+build_rpm test-pkg requires test-pkg-common
 # The test suite writes to pwd, but we need repos in composedata
 # Also we need to disable gpgcheck
 echo gpgcheck=0 >> yumrepo.repo
@@ -28,15 +27,15 @@ echo "ok compose"
 assert_has_file "versions.lock"
 assert_file_has_content $PWD/versions.lock 'packages'
 assert_file_has_content $PWD/versions.lock 'test-pkg-1.0-1.x86_64'
+assert_file_has_content $PWD/versions.lock 'test-pkg-common-1.0-1.x86_64'
 echo "lockfile created"
 # Read lockfile back
-build_rpm test-pkg \
-          version 2.0 \
-          files "/usr/bin/test-pkg" \
-          install "mkdir -p %{buildroot}/usr/bin && echo localpkg data > %{buildroot}/usr/bin/test-pkg"
+build_rpm test-pkg-common version 2.0
+build_rpm test-pkg version 2.0 requires test-pkg-common
 runcompose --ex-lockfile=$PWD/versions.lock --cachedir $(pwd)/cache
 echo "ok compose with lockfile"
 
-rpm-ostree --repo=${repobuild} db list ${treeref} test-pkg >test-pkg-list.txt
+rpm-ostree --repo=${repobuild} db list ${treeref} > test-pkg-list.txt
 assert_file_has_content test-pkg-list.txt 'test-pkg-1.0-1.x86_64'
+assert_file_has_content test-pkg-list.txt 'test-pkg-common-1.0-1.x86_64'
 echo "lockfile read"