From 11f3c829e5e3ad6a13db7d47c5a1283719c8cf71 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 14 Apr 2020 16:44:02 -0400 Subject: [PATCH 1/5] app/compose: Rename lockfile variables Minor cosmetic change; rename the variables so they match the name of the options they represent. --- src/app/rpmostree-compose-builtin-tree.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index 560348d881..458b2a50cc 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -77,8 +77,8 @@ static gboolean opt_print_only; static char *opt_write_commitid_to; static char *opt_write_composejson_to; static gboolean opt_no_parent; -static char *opt_write_lockfile; -static char **opt_read_lockfiles; +static char *opt_write_lockfile_to; +static char **opt_lockfiles; static char *opt_parent; /* shared by both install & commit */ @@ -102,8 +102,8 @@ static GOptionEntry install_option_entries[] = { { "touch-if-changed", 0, 0, G_OPTION_ARG_STRING, &opt_touch_if_changed, "Update the modification time on FILE if a new commit was created", "FILE" }, { "workdir", 0, 0, G_OPTION_ARG_STRING, &opt_workdir, "Working directory", "WORKDIR" }, { "workdir-tmpfs", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &opt_workdir_tmpfs, "Use tmpfs for working state", NULL }, - { "ex-write-lockfile-to", 0, 0, G_OPTION_ARG_STRING, &opt_write_lockfile, "Write RPM versions information to FILE", "FILE" }, - { "ex-lockfile", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_read_lockfiles, "Read RPM version information from FILE", "FILE" }, + { "ex-write-lockfile-to", 0, 0, G_OPTION_ARG_STRING, &opt_write_lockfile_to, "Write lockfile to FILE", "FILE" }, + { "ex-lockfile", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_lockfiles, "Read lockfile from FILE", "FILE" }, { NULL } }; @@ -347,7 +347,7 @@ install_packages (RpmOstreeTreeComposeContext *self, rpmostree_print_transaction (dnfctx); - if (opt_write_lockfile) + if (opt_write_lockfile_to) { g_autoptr(GPtrArray) pkgs = rpmostree_context_get_packages (self->corectx); g_assert (pkgs); @@ -355,7 +355,7 @@ install_packages (RpmOstreeTreeComposeContext *self, g_autoptr(GPtrArray) rpmmd_repos = rpmostree_get_enabled_rpmmd_repos (rpmostree_context_get_dnf (self->corectx), DNF_REPO_ENABLED_PACKAGES); - if (!ror_lockfile_write (opt_write_lockfile, pkgs, rpmmd_repos, error)) + if (!ror_lockfile_write (opt_write_lockfile_to, pkgs, rpmmd_repos, error)) return FALSE; } @@ -697,13 +697,13 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (!self->corectx) return FALSE; - if (opt_read_lockfiles) + if (opt_lockfiles) { - g_autoptr(GHashTable) map = ror_lockfile_read (opt_read_lockfiles, error); + g_autoptr(GHashTable) map = ror_lockfile_read (opt_lockfiles, error); if (!map) return FALSE; rpmostree_context_set_vlockmap (self->corectx, map); - g_print ("Loaded lockfiles:\n %s\n", g_strjoinv ("\n ", opt_read_lockfiles)); + g_print ("Loaded lockfiles:\n %s\n", g_strjoinv ("\n ", opt_lockfiles)); } const char *arch = dnf_context_get_base_arch (rpmostree_context_get_dnf (self->corectx)); From cbd8a63a746e0355e87bba2c0fe14e46c6783dd0 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 14 Apr 2020 16:44:03 -0400 Subject: [PATCH 2/5] core: Factor out functions to enable/disable repos Prep for future patch. --- src/libpriv/rpmostree-core.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c index b7f705c477..255ee080ab 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -262,7 +262,7 @@ rpmostree_treespec_new_from_keyfile (GKeyFile *keyfile, add_canonicalized_string_array (&builder, "removed-base-packages", NULL, keyfile); add_canonicalized_string_array (&builder, "cached-replaced-base-packages", NULL, keyfile); - /* We allow the "repo" key to be missing. This means that we rely on hif's + /* We allow the "repos" key to be missing. This means that we rely on libdnf's * normal behaviour (i.e. look at repos in repodir with enabled=1). */ { g_auto(GStrv) val = g_key_file_get_string_list (keyfile, "tree", "repos", NULL, NULL); if (val && *val) @@ -646,11 +646,8 @@ enable_one_repo (GPtrArray *sources, return glnx_throw (error, "Unknown rpm-md repository: %s", reponame); } -/* Enable all repos in @enabled_repos, and disable everything else */ -static gboolean -context_repos_enable_only (RpmOstreeContext *context, - const char *const *enabled_repos, - GError **error) +static void +disable_all_repos (RpmOstreeContext *context) { GPtrArray *sources = dnf_context_get_repos (context->dnfctx); for (guint i = 0; i < sources->len; i++) @@ -658,8 +655,16 @@ context_repos_enable_only (RpmOstreeContext *context, DnfRepo *src = sources->pdata[i]; dnf_repo_set_enabled (src, DNF_REPO_ENABLED_NONE); } +} - for (const char *const *iter = enabled_repos; iter && *iter; iter++) +/* Enable all repos in @repos */ +static gboolean +enable_repos (RpmOstreeContext *context, + const char *const *repos, + GError **error) +{ + GPtrArray *sources = dnf_context_get_repos (context->dnfctx); + for (const char *const *iter = repos; iter && *iter; iter++) { if (!enable_one_repo (sources, *iter, error)) return FALSE; @@ -687,7 +692,6 @@ rpmostree_context_setup (RpmOstreeContext *self, GError **error) { const char *releasever = NULL; - g_autofree char **enabled_repos = NULL; g_autofree char **instlangs = NULL; /* This exists (as a canonically empty dir) at least on RHEL7+ */ static const char emptydir_path[] = "/usr/share/empty"; @@ -750,15 +754,19 @@ rpmostree_context_setup (RpmOstreeContext *self, /* disable all repos in pkgcache-only mode, otherwise obey "repos" key */ if (self->pkgcache_only) { - if (!context_repos_enable_only (self, NULL, error)) - return FALSE; + disable_all_repos (self); } else { - /* NB: missing "repos" --> let hif figure it out for itself */ + /* NB: missing "repos" --> let libdnf figure it out for itself (we're likely doing a + * client-side compose where we want to use /etc/yum.repos.d/) */ + g_autofree char **enabled_repos = NULL; if (g_variant_dict_lookup (self->spec->dict, "repos", "^a&s", &enabled_repos)) - if (!context_repos_enable_only (self, (const char *const*)enabled_repos, error)) - return FALSE; + { + disable_all_repos (self); + if (!enable_repos (self, (const char *const*)enabled_repos, error)) + return FALSE; + } } g_autoptr(GPtrArray) repos = From 27c2ada19f1159be4b28ecf88526b56588b02e05 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 14 Apr 2020 16:44:04 -0400 Subject: [PATCH 3/5] tests/compose: Don't use lockfiles by default Otherwise, it muddles testing in `test-lockfile.sh` where we want to be in full control of all the lockfiles fed to `rpm-ostree compose tree`. --- tests/compose/libcomposetest.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/compose/libcomposetest.sh b/tests/compose/libcomposetest.sh index 39e7cb9a73..555570f631 100644 --- a/tests/compose/libcomposetest.sh +++ b/tests/compose/libcomposetest.sh @@ -60,9 +60,7 @@ else: export compose_base_argv="\ --unified-core \ --repo=${repo} \ - --cachedir=${test_tmpdir}/cache \ - --ex-lockfile=config/manifest-lock.x86_64.json \ - --ex-lockfile=config/manifest-lock.overrides.x86_64.yaml" + --cachedir=${test_tmpdir}/cache" # and create this now for tests which only use `compose_base_argv` mkdir -p cache From 27a6e8e80c4ef1484cce75e78001180bfd6e9abd Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 14 Apr 2020 16:44:05 -0400 Subject: [PATCH 4/5] tests/compose: Go back to freezing FCOS commit The garbage collection issue should be fixed now, and it's just nicer on developers' cache to stay on the same commit. And again, it's a nice sanity-check to know that we're always able to compose an older tree. That said, we probably should still bump this from time to time. While we're here, add some comments for making it easier to match `popd` calls with the original `pushd`. --- tests/compose.sh | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/compose.sh b/tests/compose.sh index ce522f8769..1254f4ad0e 100755 --- a/tests/compose.sh +++ b/tests/compose.sh @@ -3,8 +3,7 @@ set -euo pipefail # freeze on a specific commit for tests for reproducibility and since it should # always work to target older treefiles -# XXX: disable this for now: https://pagure.io/releng/issue/9281 -# FEDORA_COREOS_CONFIG_COMMIT=088fc2dec535aca392958e9c30c17cf19ef4b568 +FEDORA_COREOS_CONFIG_COMMIT=d18bcd20d359ba03e5b47faba25810cb26a0fa32 dn=$(cd "$(dirname "$0")" && pwd) topsrcdir=$(cd "$dn/.." && pwd) @@ -39,15 +38,14 @@ if [ ! -d compose-cache ]; then git clone https://github.com/coreos/fedora-coreos-config config pushd config - # XXX: disable this for now -- see above - # git checkout "${FEDORA_COREOS_CONFIG_COMMIT}" + git checkout "${FEDORA_COREOS_CONFIG_COMMIT}" # we flatten the treefile to make it easier to manipulate in tests (we have # lots of tests that check for include logic already) rpm-ostree compose tree --print-only manifest.yaml > manifest.json rm manifest.yaml mv manifests/{passwd,group} . rm -rf manifests/ - popd + popd # config if ! has_compose_privileges; then # Unlike cosa, we don't need as much flexibility since we don't e.g. build @@ -133,9 +131,9 @@ json.dump(y, sys.stdout)' < manifest.json > manifest.json.new git add . git -c user.email="composetest@localhost.com" -c user.name="composetest" \ commit -am 'modifications for tests' - popd + popd # config - popd + popd # compose-cache fi echo "Running ${#tests[*]} tests ${JOBS} at a time" From cc6a8a9c212763ff55669a89c0f3d5a7a79c0dc5 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 14 Apr 2020 16:44:06 -0400 Subject: [PATCH 5/5] core: Use `hy_query_run_set()` for excludes Instead of manually recreating the packageset ourselves. --- src/libpriv/rpmostree-core.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c index 255ee080ab..7e6e32bdd5 100644 --- a/src/libpriv/rpmostree-core.c +++ b/src/libpriv/rpmostree-core.c @@ -2041,10 +2041,7 @@ rpmostree_context_prepare (RpmOstreeContext *self, const char *pkgname = *iter; hy_autoquery HyQuery query = hy_query_create (sack); hy_query_filter (query, HY_PKG_NAME, HY_EQ, pkgname); - g_autoptr(GPtrArray) pkglist = hy_query_run (query); - DnfPackageSet *pset = dnf_packageset_new (sack); - for (guint i = 0; i < pkglist->len; i++) - dnf_packageset_add (pset, pkglist->pdata[i]); + DnfPackageSet *pset = hy_query_run_set (query); dnf_sack_add_excludes (sack, pset); dnf_packageset_free (pset); }