From 37273705ddc0b39ff4113dedfdd4626a9bea4be8 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Wed, 20 Dec 2017 22:46:04 +0000 Subject: [PATCH] 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 | 79 +++++++++++++++---------- src/app/rpmostree-libbuiltin.c | 2 +- src/libpriv/rpmostree-util.c | 69 ++++++++++++++++++++-- src/libpriv/rpmostree-util.h | 15 +++-- tests/common/libtest.sh | 7 +++ tests/common/libvm.sh | 5 ++ tests/vmcheck/test-db.sh | 91 +++++++++++++++++++++++++++++ 7 files changed, 226 insertions(+), 42 deletions(-) create mode 100755 tests/vmcheck/test-db.sh diff --git a/src/app/rpmostree-db-builtin-diff.c b/src/app/rpmostree-db-builtin-diff.c index 79b5f01b2a..4b72e24997 100644 --- a/src/app/rpmostree-db-builtin-diff.c +++ b/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; diff --git a/src/app/rpmostree-libbuiltin.c b/src/app/rpmostree-libbuiltin.c index c9ba58831e..a6bc0cef2b 100644 --- a/src/app/rpmostree-libbuiltin.c +++ b/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; diff --git a/src/libpriv/rpmostree-util.c b/src/libpriv/rpmostree-util.c index af7a73963e..5f1ba0374b 100644 --- a/src/libpriv/rpmostree-util.c +++ b/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 diff --git a/src/libpriv/rpmostree-util.h b/src/libpriv/rpmostree-util.h index f6d39db009..6f32dd3de5 100644 --- a/src/libpriv/rpmostree-util.h +++ b/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, diff --git a/tests/common/libtest.sh b/tests/common/libtest.sh index 2f9264277a..3b8b41f8f3 100644 --- a/tests/common/libtest.sh +++ b/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 diff --git a/tests/common/libvm.sh b/tests/common/libvm.sh index 77075f26df..e80291fb26 100644 --- a/tests/common/libvm.sh +++ b/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" diff --git a/tests/vmcheck/test-db.sh b/tests/vmcheck/test-db.sh new file mode 100755 index 0000000000..8e0c86d7eb --- /dev/null +++ b/tests/vmcheck/test-db.sh @@ -0,0 +1,91 @@ +#!/bin/bash +# +# Copyright (C) 2017 Jonathan Lebon +# +# 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"