app/db-diff: make use of new db API

This is the first step towards unifying how we introspect packages from
a specific commit. We currently do this in three ways: libdnf, librpm,
and now `rpmostree.rpmdb.pkglist`. I'd like to get to a point where we
only have `rpmostree.rpmdb.pkglist` and libdnf, the latter only when
more complex queries are required.

This patch teaches the `db diff` command to make use of the new db diff
API so that it can work even on metadata-only commits. This is relevant
for use cases mentioned in #558.

I didn't get rid of the `rpmhdrs_diff` functions right now because of
the `--changelogs` option: libdnf currently does not expose this, so we
fall back to the previous API in that case. OTOH, I wonder how much it's
actually used in the wild; maybe we could just nix it?

Closes: #1162
Approved by: cgwalters

src/app/rpmostree-db-builtin-diff.c

@@ -20,11 +20,12 @@
 
 #include "config.h"
 
+#include "rpmostree.h"
 #include "rpmostree-db-builtins.h"
 #include "rpmostree-libbuiltin.h"
 #include "rpmostree-rpm-util.h"
 
-static char *opt_format;
+static char *opt_format = "block";
 static gboolean opt_changelogs;
 
 static GOptionEntry option_entries[] = {
@@ -38,60 +39,74 @@ rpmostree_db_builtin_diff (int argc, char **argv,
                            RpmOstreeCommandInvocation *invocation,
                            GCancellable *cancellable, GError **error)
 {
-  g_autoptr(GOptionContext) context =
-    g_option_context_new ("COMMIT COMMIT");
+  g_autoptr(GOptionContext) context = g_option_context_new ("COMMIT COMMIT");
 
   g_autoptr(OstreeRepo) repo = NULL;
-  if (!rpmostree_db_option_context_parse (context, option_entries, &argc, &argv, invocation, &repo,
-                                          cancellable, error))
+  if (!rpmostree_db_option_context_parse (context, option_entries, &argc, &argv, invocation,
+                                          &repo, cancellable, error))
     return EXIT_FAILURE;
 
   if (argc != 3)
     {
-      g_autofree char *message = NULL;
-
-      message = g_strdup_printf ("\"%s\" takes exactly 2 arguments",
-                                 g_get_prgname ());
+      g_autofree char *message =
+        g_strdup_printf ("\"%s\" takes exactly 2 arguments", g_get_prgname ());
       rpmostree_usage_error (context, message, error);
       return EXIT_FAILURE;
     }
 
-  g_autoptr(RpmRevisionData) rpmrev1 = NULL;
-  if (!(rpmrev1 = rpmrev_new (repo, argv[1], NULL, cancellable, error)))
+  const char *old_ref = argv[1];
+  g_autofree char *old_checksum = NULL;
+  if (!ostree_repo_resolve_rev (repo, old_ref, FALSE, &old_checksum, error))
     return EXIT_FAILURE;
 
-  g_autoptr(RpmRevisionData) rpmrev2 = NULL;
-  if (!(rpmrev2 = rpmrev_new (repo, argv[2], NULL, cancellable, error)))
+  const char *new_ref = argv[2];
+  g_autofree char *new_checksum = NULL;
+  if (!ostree_repo_resolve_rev (repo, new_ref, FALSE, &new_checksum, error))
     return EXIT_FAILURE;
 
-  if (!g_str_equal (argv[1], rpmrev_get_commit (rpmrev1)))
-    printf ("ostree diff commit old: %s (%s)\n", argv[1], rpmrev_get_commit (rpmrev1));
+  if (!g_str_equal (old_ref, old_checksum))
+    printf ("ostree diff commit old: %s (%s)\n", old_ref, old_checksum);
   else
-    printf ("ostree diff commit old: %s\n", argv[1]);
+    printf ("ostree diff commit old: %s\n", old_ref);
 
-  if (!g_str_equal (argv[2], rpmrev_get_commit (rpmrev2)))
-    printf ("ostree diff commit new: %s (%s)\n", argv[2], rpmrev_get_commit (rpmrev2));
+  if (!g_str_equal (new_ref, new_checksum))
+    printf ("ostree diff commit new: %s (%s)\n", new_ref, new_checksum);
   else
-    printf ("ostree diff commit new: %s\n", argv[2]);
+    printf ("ostree diff commit new: %s\n", new_ref);
 
-  if (opt_format == NULL)
-    opt_format = "block";
+  g_autoptr(GPtrArray) removed = NULL;
+  g_autoptr(GPtrArray) added = NULL;
+  g_autoptr(GPtrArray) modified_old = NULL;
+  g_autoptr(GPtrArray) modified_new = NULL;
 
-  if (g_str_equal (opt_format, "diff"))
-    {
-      rpmhdrs_diff_prnt_diff (rpmhdrs_diff (rpmrev_get_headers (rpmrev1),
-                                            rpmrev_get_headers (rpmrev2)));
-    }
-  else if (g_str_equal (opt_format, "block"))
+  /* we still use the old API for changelogs; should enhance libdnf for this */
+  if (g_str_equal (opt_format, "block") && opt_changelogs)
     {
-      rpmhdrs_diff_prnt_block (opt_changelogs,
-                               rpmhdrs_diff (rpmrev_get_headers (rpmrev1),
-                                             rpmrev_get_headers (rpmrev2)));
+      g_autoptr(RpmRevisionData) rpmrev1 = rpmrev_new (repo, old_ref, NULL, cancellable, error);
+      if (!rpmrev1)
+        return EXIT_FAILURE;
+      g_autoptr(RpmRevisionData) rpmrev2 = rpmrev_new (repo, new_ref, NULL, cancellable, error);
+      if (!rpmrev2)
+        return EXIT_FAILURE;
+
+      rpmhdrs_diff_prnt_block (TRUE, rpmhdrs_diff (rpmrev_get_headers (rpmrev1),
+                                                   rpmrev_get_headers (rpmrev2)));
     }
   else
     {
-      glnx_throw (error, "Format argument is invalid, pick one of: diff, block");
-      return EXIT_FAILURE;
+      if (!rpm_ostree_db_diff (repo, old_ref, new_ref, &removed, &added, &modified_old,
+                               &modified_new, cancellable, error))
+        return EXIT_FAILURE;
+
+      if (g_str_equal (opt_format, "diff"))
+        rpmostree_diff_print (removed, added, modified_old, modified_new);
+      else if (g_str_equal (opt_format, "block"))
+        rpmostree_diff_print_formatted (removed, added, modified_old, modified_new);
+      else
+        {
+          glnx_throw (error, "Format argument is invalid, pick one of: diff, block");
+          return EXIT_FAILURE;
+        }
     }
 
   return EXIT_SUCCESS;

src/app/rpmostree-libbuiltin.c

@@ -110,7 +110,7 @@ rpmostree_print_treepkg_diff (OstreeSysroot    *sysroot,
                                cancellable, error))
         return FALSE;
 
-      rpmostree_diff_print (removed, added, modified_old, modified_new);
+      rpmostree_diff_print_formatted (removed, added, modified_old, modified_new);
     }
 
   return TRUE;

src/libpriv/rpmostree-util.c

@@ -731,12 +731,12 @@ rpmostree_stdout_is_journal (void)
   return stdout_is_socket;
 }
 
-/* Given the result of rpm_ostree_db_diff(), print it. */
+/* Given the result of rpm_ostree_db_diff(), print it in a nice formatted way for humans. */
 void
-rpmostree_diff_print (GPtrArray *removed,
-                      GPtrArray *added,
-                      GPtrArray *modified_old,
-                      GPtrArray *modified_new)
+rpmostree_diff_print_formatted (GPtrArray *removed,
+                                GPtrArray *added,
+                                GPtrArray *modified_old,
+                                GPtrArray *modified_new)
 {
   gboolean first;
 
@@ -805,6 +805,65 @@ rpmostree_diff_print (GPtrArray *removed,
     }
 }
 
+static int
+pkg_cmp_end (RpmOstreePackage *a, RpmOstreePackage *b)
+{
+  if (!b)
+    return -1;
+  if (!a)
+    return +1;
+
+  return rpm_ostree_package_cmp (a, b);
+}
+
+/* Given the result of rpm_ostree_db_diff(), print it in diff format for scripts. */
+void
+rpmostree_diff_print (GPtrArray *removed,
+                      GPtrArray *added,
+                      GPtrArray *modified_old,
+                      GPtrArray *modified_new)
+{
+  g_assert_cmpuint (modified_old->len, ==, modified_new->len);
+
+  const guint an = added->len;
+  const guint rn = removed->len;
+  const guint mn = modified_old->len;
+
+  guint cur_a = 0;
+  guint cur_r = 0;
+  guint cur_m = 0;
+  while (cur_a < an || cur_r < rn || cur_m < mn)
+    {
+      RpmOstreePackage *pkg_a = cur_a < an ? added->pdata[cur_a] : NULL;
+      RpmOstreePackage *pkg_r = cur_r < rn ? removed->pdata[cur_r] : NULL;
+      RpmOstreePackage *pkg_m = cur_m < mn ? modified_old->pdata[cur_m] : NULL;
+
+      if (pkg_cmp_end (pkg_m, pkg_r) < 0)
+        if (pkg_cmp_end (pkg_m, pkg_a) < 0)
+          { /* mod is first */
+            g_print ("!%s\n", rpm_ostree_package_get_nevra (pkg_m));
+            g_print ("=%s\n", rpm_ostree_package_get_nevra (modified_new->pdata[cur_m]));
+            cur_m++;
+          }
+        else
+          { /* add is first */
+            g_print ("+%s\n", rpm_ostree_package_get_nevra (pkg_a));
+            cur_a++;
+          }
+      else
+        if (pkg_cmp_end (pkg_r, pkg_a) < 0)
+          { /* del is first */
+            g_print ("-%s\n", rpm_ostree_package_get_nevra (pkg_r));
+            cur_r++;
+          }
+        else
+          { /* add is first */
+            g_print ("+%s\n", rpm_ostree_package_get_nevra (pkg_a));
+            cur_a++;
+          }
+    }
+}
+
 /* Copy of ot_variant_bsearch_str() from libostree
  * @array: A GVariant array whose first element must be a string
  * @str: Search for this string

src/libpriv/rpmostree-util.h

@@ -86,10 +86,17 @@ gs_file_get_path_cached (GFile *file)
 
 gboolean rpmostree_stdout_is_journal (void);
 
-void rpmostree_diff_print (GPtrArray *removed,
-                           GPtrArray *added,
-                           GPtrArray *modified_old,
-                           GPtrArray *modified_new);
+void
+rpmostree_diff_print_formatted (GPtrArray *removed,
+                                GPtrArray *added,
+                                GPtrArray *modified_old,
+                                GPtrArray *modified_new);
+
+void
+rpmostree_diff_print (GPtrArray *removed,
+                      GPtrArray *added,
+                      GPtrArray *modified_old,
+                      GPtrArray *modified_new);
 
 gboolean
 rpmostree_str_has_prefix_in_strv (const char *str,

tests/common/libtest.sh

@@ -373,6 +373,13 @@ assert_status_jq() {
     assert_status_file_jq status.json "$@"
 }
 
+get_obj_path() {
+  repo=$1; shift
+  csum=$1; shift
+  objtype=$1; shift
+  echo "${repo}/objects/${csum:0:2}/${csum:2}.${objtype}"
+}
+
 # builds a new RPM and adds it to the testdir's repo
 # $1 - name
 # $2+ - optional, treated as directive/value pairs

tests/common/libvm.sh

@@ -302,6 +302,11 @@ vm_get_booted_csum() {
   vm_get_booted_deployment_info checksum
 }
 
+# retrieve the checksum of the pending deployment
+vm_get_pending_csum() {
+  vm_get_deployment_info 0 checksum
+}
+
 # make multiple consistency checks on a test pkg
 # - $1    package to check for
 # - $2    either "present" or "absent"

tests/vmcheck/test-db.sh

@@ -0,0 +1,91 @@
+#!/bin/bash
+#
+# Copyright (C) 2017 Jonathan Lebon <jlebon@redhat.com>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the
+# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+# Boston, MA 02111-1307, USA.
+
+set -euo pipefail
+
+. ${commondir}/libtest.sh
+. ${commondir}/libvm.sh
+
+set -x
+
+# SUMMARY: check that `db` commands work correctly. Right now, we're only
+# testing `db diff`.
+
+YUMREPO=/tmp/vmcheck/yumrepo/packages/x86_64
+
+check_diff() {
+  from=$1; shift
+  to=$1; shift
+  vm_rpmostree db diff --format=diff $from $to > diff.txt
+  assert_file_has_content diff.txt "$@"
+}
+
+vm_build_rpm pkg-to-remove
+vm_build_rpm pkg-to-replace
+vm_rpmostree install pkg-to-remove pkg-to-replace
+
+booted_csum=$(vm_get_booted_csum)
+pending_csum=$(vm_get_pending_csum)
+check_diff $booted_csum $pending_csum \
+  +pkg-to-remove \
+  +pkg-to-replace
+
+# now let's make the pending csum become an update
+vm_cmd ostree commit -b vmcheck --tree=ref=$pending_csum
+vm_rpmostree cleanup -p
+vm_rpmostree upgrade
+pending_csum=$(vm_get_pending_csum)
+check_diff $booted_csum $pending_csum \
+  +pkg-to-remove \
+  +pkg-to-replace
+echo "ok setup"
+
+vm_rpmostree override remove pkg-to-remove
+vm_build_rpm pkg-to-replace version 2.0
+vm_rpmostree override replace $YUMREPO/pkg-to-replace-2.0-1.x86_64.rpm
+vm_build_rpm pkg-to-overlay
+vm_rpmostree install pkg-to-overlay
+pending_layered_csum=$(vm_get_pending_csum)
+check_diff $booted_csum $pending_layered_csum \
+  +pkg-to-overlay \
+  +pkg-to-replace-2.0
+check_diff $pending_csum $pending_layered_csum \
+  +pkg-to-overlay \
+  -pkg-to-remove \
+  !pkg-to-replace-1.0 \
+  =pkg-to-replace-2.0
+echo "ok db diff"
+
+# this is a bit convoluted; basically, we prune the commit and only keep its
+# metadata to check that `db diff` is indeed using the rpmdb.pkglist metadata
+commit_path=$(get_obj_path /ostree/repo $pending_layered_csum commit)
+vm_cmd test -f $commit_path
+vm_cmd cp $commit_path $commit_path.bak
+vm_rpmostree cleanup -p
+vm_cmd test ! -f $commit_path
+vm_cmd mv $commit_path.bak $commit_path
+if vm_cmd ostree checkout --subpath /usr/share/rpm $pending_layered_csum; then
+  assert_not_reached "Was able to checkout /usr/share/rpm?"
+fi
+check_diff $pending_csum $pending_layered_csum \
+  +pkg-to-overlay \
+  -pkg-to-remove \
+  !pkg-to-replace-1.0 \
+  =pkg-to-replace-2.0
+echo "ok db from pkglist.metadata"