Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compose: Add --ex-lockfile and --ex-write-lockfile-to #1745

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/app/rpmostree-compose-builtin-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,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_lockfile;

/* shared by both install & commit */
static GOptionEntry common_option_entries[] = {
Expand All @@ -92,6 +94,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, &opt_read_lockfile, "Read RPM version information from FILE", "FILE" },
{ NULL }
};

Expand Down Expand Up @@ -334,6 +338,10 @@ install_packages (RpmOstreeTreeComposeContext *self,

rpmostree_print_transaction (dnfctx);

if (opt_write_lockfile &&
!rpmostree_composeutil_write_lockfilejson (self->corectx, opt_write_lockfile, error))
return FALSE;

/* FIXME - just do a depsolve here before we compute download requirements */
g_autofree char *ret_new_inputhash = NULL;
if (!rpmostree_composeutil_checksum (dnf_context_get_goal (dnfctx),
Expand Down Expand Up @@ -682,6 +690,16 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr,
if (!self->corectx)
return FALSE;

if (opt_read_lockfile)
{
g_autoptr(GHashTable) map = rpmostree_composeutil_get_vlockmap (opt_read_lockfile,
error);
if (!map)
return FALSE;
rpmostree_context_set_vlockmap (self->corectx, map);
g_print ("Loaded lockfile: %s\n", opt_read_lockfile);
}

const char *arch = dnf_context_get_base_arch (rpmostree_context_get_dnf (self->corectx));
if (!parse_treefile_to_json (gs_file_get_path_cached (self->treefile_path),
self->workdir_dfd, arch,
Expand Down
93 changes: 93 additions & 0 deletions src/app/rpmostree-composeutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,96 @@ rpmostree_composeutil_write_composejson (OstreeRepo *repo,

return TRUE;
}

/* Implements --write-lockfile-to.
* If `path` is NULL, this is a NO-OP.
*/
gboolean
rpmostree_composeutil_write_lockfilejson (RpmOstreeContext *ctx,
const char *path,
GError **error)
{
if (!path)
return TRUE;

g_autoptr(GPtrArray) pkgs = rpmostree_context_get_packages (ctx);
g_assert (pkgs);

g_auto(GVariantBuilder) builder;
g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));

g_autoptr(GVariant) pkglist_v = NULL;
if (!rpmostree_create_pkglist_variant (pkgs, &pkglist_v, NULL, error))
return FALSE;
g_variant_builder_add (&builder, "{sv}", "packages", pkglist_v);

g_autoptr(GVariant) lock_v = g_variant_builder_end (&builder);
g_assert (lock_v != NULL);
g_autoptr(JsonNode) lock_node = json_gvariant_serialize (lock_v);
g_assert (lock_node != NULL);
glnx_unref_object JsonGenerator *generator = json_generator_new ();
json_generator_set_root (generator, lock_node);
r4f4 marked this conversation as resolved.
Show resolved Hide resolved
/* Let's make it somewhat introspectable by humans */
json_generator_set_pretty (generator, TRUE);

char *dnbuf = strdupa (path);
const char *dn = dirname (dnbuf);
g_auto(GLnxTmpfile) tmpf = { 0, };
if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC, &tmpf, error))
return FALSE;
g_autoptr(GOutputStream) out = g_unix_output_stream_new (tmpf.fd, FALSE);
/* See also similar code in status.c */
if (json_generator_to_stream (generator, out, NULL, error) <= 0 ||
(error != NULL && *error != NULL))
return FALSE;

/* World readable to match --write-commitid-to which uses mask */
if (!glnx_fchmod (tmpf.fd, 0644, error))
return FALSE;

if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE, AT_FDCWD, path, error))
return FALSE;

return TRUE;
}

/* compose tree accepts JSON package version lock via file;
* convert it to a hash table of a{sv}; suitable for further extension.
*/
GHashTable *
rpmostree_composeutil_get_vlockmap (const char *path,
GError **error)
{
g_autoptr(JsonParser) parser = json_parser_new_immutable ();
if (!json_parser_load_from_file (parser, path, error))
return glnx_null_throw (error, "Could not load lockfile %s", path);

JsonNode *metarootval = json_parser_get_root (parser);
g_autoptr(GVariant) jsonmetav = json_gvariant_deserialize (metarootval, "a{sv}", error);
if (!jsonmetav)
return glnx_null_throw (error, "Could not parse %s", path);

g_autoptr(GVariant) value = g_variant_lookup_value (jsonmetav, "packages", G_VARIANT_TYPE ("av"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a(ss)? That should simplify the loop below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried, it doesn't work. The parsed json is like this

Read: {'packages': <[<[<'acl-2.2.53-2.fc29.x86_64'>, <'sha256:0a7446de7de962fbbfdb204da0a7aef70cc8bd4ee4b2c03b0d730bf62b9a2223'>]>, <[<'atomic-registries-1.22.1-27.gitb507039.fc29.x86_64'>, <'sha256:19fdd5bcb79bc84c4c3d17d63d2ee39173d3d2504ea11ed9f0b7d0d0cf86550c'>]>, <[<'audit-libs-3.0-0.7.20190326git03e7489.fc29.x86_64'>, <'sha256:a907544ffac01209ce49026a0ebedef58d62639aa9d234537577c72d458cc850'>]>, <[<'basesystem-11-6.fc29.noarch'>, <'sha256:e4b86fe4f119873e8705dae47163f4afb6ccf8885426e04d960ff18eed7320bb'>]>, <[<'bash-4.4.23-6.fc29.x86_64'>, <'sha256:1899be2cb4a4793d6a6c62285a6b77f5c22eb5534e76d94d2a5d421cf2aa59ad'>]>,

as you can see, everything is wrapped inside a GVariant. Unless I'm missing something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh hmm, yeah I see what you mean. Seems like that's just how json_gvariant_deserialize works. Which makes sense, since it can't know that the JSON is following some particular scheme, so it just wraps all leaf nodes in the JSON into a GVariant. Yuck. So, I do think that parsing with serde would make this much nicer and give us scheme checking as well, but happy to stick with this for now!

if (!value)
return glnx_null_throw (error, "Failed to find \"packages\" section in lockfile");

g_autoptr(GHashTable) nevra_to_chksum =
g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);

GVariantIter iter;
g_variant_iter_init (&iter, value);
GVariant *child;
while (g_variant_iter_loop (&iter, "v", &child))
{
char *nevra = NULL, *repochksum = NULL;
g_autoptr(GVariant) nv = g_variant_get_child_value (child, 0);
g_autoptr(GVariant) nvv = g_variant_get_variant (nv);
nevra = g_variant_dup_string (nvv, NULL);
g_autoptr(GVariant) cv = g_variant_get_child_value (child, 1);
g_autoptr(GVariant) cvv = g_variant_get_variant (cv);
repochksum = g_variant_dup_string (cvv, NULL);
g_hash_table_insert (nevra_to_chksum, nevra, repochksum);
}

return g_steal_pointer (&nevra_to_chksum);
}
8 changes: 8 additions & 0 deletions src/app/rpmostree-composeutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,13 @@ rpmostree_composeutil_write_composejson (OstreeRepo *repo,
GVariantBuilder *builder,
GError **error);

gboolean
rpmostree_composeutil_write_lockfilejson (RpmOstreeContext *ctx,
const char *path,
GError **error);

GHashTable *
rpmostree_composeutil_get_vlockmap (const char *path,
GError **error);
G_END_DECLS

2 changes: 2 additions & 0 deletions src/libpriv/rpmostree-core-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ struct _RpmOstreeContext {
GHashTable *pkgs_to_remove; /* pkgname --> gv_nevra */
GHashTable *pkgs_to_replace; /* new gv_nevra --> old gv_nevra */

GHashTable *vlockmap; /* nevra --> repochecksum */

GLnxTmpDir tmpdir;

gboolean kernel_changed;
Expand Down
18 changes: 17 additions & 1 deletion src/libpriv/rpmostree-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ rpmostree_context_finalize (GObject *object)
g_clear_pointer (&rctx->pkgs_to_remove, g_hash_table_unref);
g_clear_pointer (&rctx->pkgs_to_replace, g_hash_table_unref);

g_clear_pointer (&rctx->vlockmap, g_hash_table_unref);

(void)glnx_tmpdir_delete (&rctx->tmpdir, NULL, NULL);
(void)glnx_tmpdir_delete (&rctx->repo_tmpdir, NULL, NULL);

Expand Down Expand Up @@ -1952,12 +1954,19 @@ rpmostree_context_prepare (RpmOstreeContext *self,

/* And finally, handle repo packages to install */
g_autoptr(GPtrArray) missing_pkgs = NULL;
DnfSack *sack = dnf_context_get_sack (dnfctx);
for (char **it = pkgnames; it && *it; it++)
{
const char *pkgname = *it;
g_autoptr(GError) local_error = NULL;
g_autoptr(DnfPackage) pkg = rpmostree_get_locked_package (sack, self->vlockmap, pkgname);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either should conditionalize this bit on self->vlockmap != NULL here or you can also check it in rpmostree_get_locked_package.

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))
{
/* Only keep going if it's ENOENT, so we coalesce into one msg at the end */
Expand Down Expand Up @@ -2064,6 +2073,13 @@ rpmostree_context_get_packages_to_import (RpmOstreeContext *self)
return g_ptr_array_ref (self->pkgs_to_import);
}

void
rpmostree_context_set_vlockmap (RpmOstreeContext *self, GHashTable *map)
{
g_clear_pointer (&self->vlockmap, (GDestroyNotify)g_hash_table_unref);
self->vlockmap = g_hash_table_ref (map);
}

/* XXX: push this into libdnf */
static const char*
convert_dnf_action_to_string (DnfStateAction action)
Expand Down
2 changes: 2 additions & 0 deletions src/libpriv/rpmostree-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ gboolean rpmostree_context_set_packages (RpmOstreeContext *self,

GPtrArray *rpmostree_context_get_packages_to_import (RpmOstreeContext *self);

void rpmostree_context_set_vlockmap (RpmOstreeContext *self, GHashTable *map);

gboolean rpmostree_context_download (RpmOstreeContext *self,
GCancellable *cancellable,
GError **error);
Expand Down
54 changes: 54 additions & 0 deletions src/libpriv/rpmostree-rpm-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,60 @@ rpmostree_create_rpmdb_pkglist_variant (int dfd,
return TRUE;
}

gboolean
rpmostree_create_pkglist_variant (GPtrArray *pkglist,
GVariant **out_variant,
GCancellable *cancellable,
GError **error)
{
/* we insert it sorted here so it can efficiently be searched on retrieval */
g_ptr_array_sort (pkglist, (GCompareFunc)rpmostree_pkg_array_compare);

GVariantBuilder pkglist_v_builder;
g_variant_builder_init (&pkglist_v_builder, G_VARIANT_TYPE ("a(ss)"));
for (guint i = 0; i < pkglist->len; i++)
{
DnfPackage *pkg = pkglist->pdata[i];
g_autofree gchar *repodata_chksum = NULL;

if (!rpmostree_get_repodata_chksum_repr (pkg, &repodata_chksum, error))
return FALSE;

g_variant_builder_add (&pkglist_v_builder, "(ss)",
dnf_package_get_nevra (pkg),
repodata_chksum);
}

*out_variant = g_variant_ref_sink (g_variant_builder_end (&pkglist_v_builder));
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I know I suggested this, but I'm not completely satisfied either with how "indirect" it all seems. One root issue here of course is #731; libsolv simply doesn't make it easy to provide resolution information. (It's not entirely its fault either; depsolving is a hard problem that isn't exactly disposed to provide clear cut answers to these sorts of questions).

Anyway, good to roll with this for now! Though it might need more thinking and tweaks in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just couldn't help thinking about this a bit more :)

ISTM like in the lockfile case, maybe we do want to require manifests to not use "provides" but instead purely pkgnames. So e.g. when using --lockfile or --write-lockfile-to, we would only search by pkgname rather than the default dnf_context_install() fuzzy logic and error out if the pkgname isn't found. That would really simplify the semantics of the lockfile.

But yeah, I'm happy getting this in as is for now, and then tweak it in follow-ups!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tiny downside of course is that it requires slightly more churn on treefile writers to react to things like Obsoletes, package splits, etc... Doesn't seem like an unreasonable burden though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up to this in #1849.


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,
Expand Down
9 changes: 9 additions & 0 deletions src/libpriv/rpmostree-rpm-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ rpmostree_create_rpmdb_pkglist_variant (int dfd,
GCancellable *cancellable,
GError **error);

gboolean
rpmostree_create_pkglist_variant (GPtrArray *pkglist,
GVariant **out_variant,
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);
Expand Down
42 changes: 42 additions & 0 deletions tests/compose-tests/test-lockfile.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/bash
set -xeuo pipefail

dn=$(cd $(dirname $0) && pwd)
. ${dn}/libcomposetest.sh

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"
# The test suite writes to pwd, but we need repos in composedata
# Also we need to disable gpgcheck
echo gpgcheck=0 >> yumrepo.repo
ln yumrepo.repo composedata/test-repo.repo
pyappendjsonmember "packages" '["test-pkg"]'
pysetjsonmember "documentation" 'False'
mkdir cache
# Create lockfile
runcompose --ex-write-lockfile-to=$PWD/versions.lock --cachedir $(pwd)/cache
npkgs=$(rpm-ostree --repo=${repobuild} db list ${treeref} |grep -v '^ostree commit' | wc -l)
echo "npkgs=${npkgs}"
rpm-ostree --repo=${repobuild} db list ${treeref} test-pkg >test-pkg-list.txt
assert_file_has_content test-pkg-list.txt 'test-pkg-1.0-1.x86_64'
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'
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"
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
assert_file_has_content test-pkg-list.txt 'test-pkg-1.0-1.x86_64'
echo "lockfile read"