jigdo: v2: Use jigdoset in Requires, and commit hash in Provides

Now that we have the jigdoset in `Requires`, let's make a hard
switch to using it and drop the jigdoset from the jigdoRPM data.

One lingering concern here is that the `Requires` are not quite
as strict as what we had before; for example one apparently can't
add a `Requires:` that refers to an architecture (x86_64 vs noarch).
And a lot more strongly than that we had the repodata checksums
in the old format.  I'm still thinking of a way to use those.

But moving on, this allows us to rework the client side to do a lot more
up-front calculation before downloading the jigdoRPM. In the spirit of that, at
the same time let's add a `Provides: rpmostree-jigdo-commit(e7bdb7443d8...)` so
that we can determine ahead of time whether or not we have the actual commit.

A major change we could now take would be to download the jigdoRPM
in parallel with the jigdo set, but doing that would require
driving a lot more of the jigdo logic into the core; it'd need
to know to specially handle the jigdoRPM download.

Closes: #1140
Approved by: jlebon
This commit is contained in:
Colin Walters 2017-12-08 17:41:29 -05:00 committed by Atomic Bot
parent 9e146f2752
commit d3e50e9a5d
7 changed files with 150 additions and 124 deletions

View File

@ -89,7 +89,7 @@ PKG_CHECK_MODULES(PKGDEP_RPMOSTREE, [gio-unix-2.0 >= 2.40.0 json-glib-1.0
ostree-1 >= 2017.14
libsystemd
polkit-gobject-1
rpm librepo
rpm librepo libsolv
libarchive])
dnl bundled libdnf
PKGDEP_RPMOSTREE_CFLAGS="-I $(pwd)/libdnf -I $(pwd)/libdnf-build $PKGDEP_RPMOSTREE_CFLAGS"

View File

@ -591,6 +591,7 @@ build_objid_map_for_package (RpmOstreeCommit2JigdoContext *self,
static char *
generate_spec (RpmOstreeCommit2JigdoContext *self,
const char *spec_path,
const char *ostree_commit_sha256,
GPtrArray *jigdo_packages,
GCancellable *cancellable,
GError **error)
@ -610,7 +611,10 @@ generate_spec (RpmOstreeCommit2JigdoContext *self,
g_autoptr(GString) replacement = g_string_new ("");
g_string_append_len (replacement, spec_contents, meta - spec_contents);
g_string_append (replacement, "# Generated by rpm-ostree\n");
g_string_append (replacement, "Provides: " RPMOSTREE_JIGDO_PROVIDE_V1 "\n");
g_string_append (replacement, "Provides: " RPMOSTREE_JIGDO_PROVIDE_V2 "\n");
/* Add provides for the commit hash */
g_string_append (replacement, "Provides: " RPMOSTREE_JIGDO_PROVIDE_COMMIT);
g_string_append_printf (replacement, "(%s)\n", ostree_commit_sha256);
/* Add Requires: on our dependent packages */
for (guint i = 0; i < jigdo_packages->len; i++)
@ -623,7 +627,7 @@ generate_spec (RpmOstreeCommit2JigdoContext *self,
g_string_append (replacement, meta + strlen (RPMOSTREE_JIGDO_SPEC_META_MAGIC) + 1);
g_string_append (replacement, "# End data generated by rpm-ostree\n");
char *tmppath = g_strdup ("/tmp/rpmostree-jigdo-spec.XXXXXX");
g_autofree char *tmppath = g_strdup ("/tmp/rpmostree-jigdo-spec.XXXXXX");
glnx_autofd int fd = g_mkstemp_full (tmppath, O_WRONLY | O_CLOEXEC, 0644);
if (glnx_loop_write (fd, replacement->str, replacement->len) < 0)
return glnx_null_throw_errno_prefix (error, "write");
@ -694,36 +698,13 @@ write_commit2jigdo (RpmOstreeCommit2JigdoContext *self,
return FALSE;
}
/* write out the variant containing packages */
/* Sort our package set */
g_autoptr(GPtrArray) jigdo_packages = g_ptr_array_new ();
GLNX_HASH_TABLE_FOREACH (pkgs_with_content, DnfPackage *, pkg)
{
g_ptr_array_add (jigdo_packages, pkg);
}
g_ptr_array_sort (jigdo_packages, compare_pkgs);
{ g_autoptr(GVariantBuilder) pkgbuilder = g_variant_builder_new (RPMOSTREE_JIGDO_PKGS_VARIANT_FORMAT);
for (guint i = 0; i < jigdo_packages->len; i++)
{
DnfPackage *pkg = jigdo_packages->pdata[i];
g_autofree char *repodata_checksum = NULL;
if (!rpmostree_get_repodata_chksum_repr (pkg, &repodata_checksum, error))
return FALSE;
g_variant_builder_add (pkgbuilder, "(stssss)",
dnf_package_get_name (pkg),
dnf_package_get_epoch (pkg),
dnf_package_get_version (pkg),
dnf_package_get_release (pkg),
dnf_package_get_arch (pkg),
repodata_checksum);
}
g_autoptr(GVariant) jigdo_packages_v = g_variant_ref_sink (g_variant_builder_end (pkgbuilder));
if (!glnx_file_replace_contents_at (oirpm_tmpd.fd, RPMOSTREE_JIGDO_PKGS,
g_variant_get_data (jigdo_packages_v),
g_variant_get_size (jigdo_packages_v),
GLNX_FILE_REPLACE_NODATASYNC,
cancellable, error))
return FALSE;
}
/* dirtree/dirmeta */
if (!glnx_shutil_mkdir_p_at (oirpm_tmpd.fd, RPMOSTREE_JIGDO_DIRMETA_DIR, 0755, cancellable, error))
@ -946,7 +927,7 @@ write_commit2jigdo (RpmOstreeCommit2JigdoContext *self,
if (!opt_only_contentdir)
{
g_autofree char *tmp_spec =
generate_spec (self, oirpm_spec, jigdo_packages, cancellable, error);
generate_spec (self, oirpm_spec, commit, jigdo_packages, cancellable, error);
if (!tmp_spec)
return FALSE;

View File

@ -24,6 +24,9 @@
#include <glib-unix.h>
#include <gio/gunixoutputstream.h>
#include <libdnf/libdnf.h>
// For the jigdo Requires parsing
#include <libdnf/dnf-reldep-private.h>
#include <libdnf/dnf-sack-private.h>
#include <sys/mount.h>
#include <stdio.h>
#include <libglnx.h>
@ -102,23 +105,19 @@ rpm_ostree_jigdo2commit_context_new (RpmOstreeJigdo2CommitContext **out_context,
}
static DnfPackage *
query_nevra (DnfContext *dnfctx,
const char *name,
guint64 epoch,
const char *version,
const char *release,
const char *arch,
GError **error)
query_jigdo_pkg (DnfContext *dnfctx,
const char *name,
const char *evr,
GError **error)
{
hy_autoquery HyQuery query = hy_query_create (dnf_context_get_sack (dnfctx));
/* See also similar examples of queries in e.g. dnf_context_update() */
hy_query_filter (query, HY_PKG_NAME, HY_EQ, name);
hy_query_filter_num (query, HY_PKG_EPOCH, HY_EQ, epoch);
hy_query_filter (query, HY_PKG_VERSION, HY_EQ, version);
hy_query_filter (query, HY_PKG_RELEASE, HY_EQ, release);
hy_query_filter (query, HY_PKG_ARCH, HY_EQ, arch);
hy_query_filter (query, HY_PKG_ARCH, HY_NEQ, "src");
hy_query_filter (query, HY_PKG_EVR, HY_EQ, evr);
g_autoptr(GPtrArray) pkglist = hy_query_run (query);
if (pkglist->len == 0)
return glnx_null_throw (error, "Failed to find package '%s'", name);
return glnx_null_throw (error, "Failed to find package %s-%s", name, evr);
return g_object_ref (pkglist->pdata[0]);
}
@ -151,6 +150,15 @@ compare_pkgs_reverse (gconstpointer ap,
return dnf_package_cmp (*b, *a); // Reverse
}
static int
compare_pkgs (gconstpointer ap,
gconstpointer bp)
{
DnfPackage **a = (gpointer)ap;
DnfPackage **b = (gpointer)bp;
return dnf_package_cmp (*a, *b);
}
static gboolean
impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
const char *repoid_and_oirpm_name,
@ -187,6 +195,7 @@ impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
DnfContext *dnfctx = rpmostree_context_get_dnf (self->ctx);
g_autoptr(DnfPackage) oirpm_pkg = NULL;
g_autofree char *provided_commit = NULL;
{ hy_autoquery HyQuery query = hy_query_create (dnf_context_get_sack (dnfctx));
hy_query_filter (query, HY_PKG_REPONAME, HY_EQ, oirpm_repoid);
hy_query_filter (query, HY_PKG_NAME, HY_EQ, oirpm_name);
@ -204,30 +213,110 @@ impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
oirpm_pkg = g_object_ref (pkglist->pdata[0]);
/* Iterate over provides directly to provide a nicer error on mismatch */
gboolean found_v1_provide = FALSE;
gboolean found_vprovide = FALSE;
g_autoptr(DnfReldepList) provides = dnf_package_get_provides (oirpm_pkg);
const gint n_provides = dnf_reldep_list_count (provides);
for (int i = 0; i < n_provides; i++)
{
DnfReldep *provide = dnf_reldep_list_index (provides, i);
if (g_str_equal (dnf_reldep_to_string (provide), RPMOSTREE_JIGDO_PROVIDE_V1))
const char *provide_str = dnf_reldep_to_string (provide);
if (g_str_equal (provide_str, RPMOSTREE_JIGDO_PROVIDE_V2))
{
found_v1_provide = TRUE;
break;
found_vprovide = TRUE;
}
else if (g_str_has_prefix (provide_str, RPMOSTREE_JIGDO_PROVIDE_COMMIT))
{
const char *rest = provide_str + strlen (RPMOSTREE_JIGDO_PROVIDE_COMMIT);
if (*rest != '(')
return glnx_throw (error, "Invalid %s", provide_str);
rest++;
const char *closeparen = strchr (rest, ')');
if (!closeparen)
return glnx_throw (error, "Invalid %s", provide_str);
provided_commit = g_strndup (rest, closeparen - rest);
if (strlen (provided_commit) != OSTREE_SHA256_STRING_LEN)
return glnx_throw (error, "Invalid %s", provide_str);
}
}
if (!found_v1_provide)
if (!found_vprovide)
return glnx_throw (error, "Package '%s' does not have Provides: %s",
dnf_package_get_nevra (oirpm_pkg), RPMOSTREE_JIGDO_PROVIDE_V1);
if (!rpmostree_context_set_packages (self->ctx, pkglist, cancellable, error))
return FALSE;
dnf_package_get_nevra (oirpm_pkg), RPMOSTREE_JIGDO_PROVIDE_V2);
if (!provided_commit)
return glnx_throw (error, "Package '%s' does not have Provides: %s",
dnf_package_get_nevra (oirpm_pkg), RPMOSTREE_JIGDO_PROVIDE_COMMIT);
}
g_print ("oirpm: %s (%s)\n", dnf_package_get_nevra (oirpm_pkg),
dnf_package_get_reponame (oirpm_pkg));
g_print ("oirpm: %s (%s) commit=%s\n", dnf_package_get_nevra (oirpm_pkg),
dnf_package_get_reponame (oirpm_pkg), provided_commit);
{ OstreeRepoCommitState commitstate;
gboolean has_commit;
if (!ostree_repo_has_object (self->repo, OSTREE_OBJECT_TYPE_COMMIT, provided_commit,
&has_commit, cancellable, error))
return FALSE;
if (has_commit)
{
if (!ostree_repo_load_commit (self->repo, provided_commit, NULL,
&commitstate, error))
return FALSE;
if (!(commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL))
{
g_print ("Commit is already written, nothing to do\n");
return TRUE; /* 🔚 Early return */
}
}
}
g_autoptr(GPtrArray) pkgs_required = g_ptr_array_new_with_free_func (g_object_unref);
/* Look at the Requires of the jigdoRPM. Note that we don't want to do
* dependency resolution here - that's part of the whole idea, we're doing
* deterministic imaging.
*/
g_autoptr(DnfReldepList) requires = dnf_package_get_requires (oirpm_pkg);
const gint n_requires = dnf_reldep_list_count (requires);
Pool *pool = dnf_sack_get_pool (dnf_context_get_sack (dnfctx));
for (int i = 0; i < n_requires; i++)
{
DnfReldep *req = dnf_reldep_list_index (requires, i);
Id reqid = dnf_reldep_get_id (req);
if (!ISRELDEP (reqid))
continue;
Reldep *rdep = GETRELDEP (pool, reqid);
/* This is the core hack; we're searching for Requires that
* have exact '=' versions. This assumes that the rpmbuild
* process won't inject such requirements.
*/
if (!(rdep->flags & REL_EQ))
continue;
const char *name = pool_id2str (pool, rdep->name);
const char *evr = pool_id2str (pool, rdep->evr);
DnfPackage *pkg = query_jigdo_pkg (dnfctx, name, evr, error);
// FIXME: Possibly we shouldn't require a package to be in the repos if we
// already have it imported? This would help support downgrades if the
// repo owner has pruned.
if (!pkg)
return FALSE;
g_ptr_array_add (pkgs_required, g_object_ref (pkg));
}
g_ptr_array_sort (pkgs_required, compare_pkgs);
g_print ("Jigdo from %u packages\n", pkgs_required->len);
/* For now we first serially download the oirpm, but down the line we can do
* this async. Doing so will require putting more of the jigdo logic into the
* core, so it knows not to import the jigdoRPM.
*/
{ g_autoptr(GPtrArray) oirpm_singleton_pkglist = g_ptr_array_new ();
g_ptr_array_add (oirpm_singleton_pkglist, oirpm_pkg);
if (!rpmostree_context_set_packages (self->ctx, oirpm_singleton_pkglist, cancellable, error))
return FALSE;
}
if (!rpmostree_context_download (self->ctx, cancellable, error))
return FALSE;
@ -242,29 +331,13 @@ impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
g_autofree char *checksum = NULL;
g_autoptr(GVariant) commit = NULL;
g_autoptr(GVariant) commit_meta = NULL;
g_autoptr(GVariant) pkgs = NULL;
if (!rpmostree_jigdo_assembler_read_meta (jigdo, &checksum, &commit, &commit_meta, &pkgs,
cancellable, error))
if (!rpmostree_jigdo_assembler_read_meta (jigdo, &checksum, &commit, &commit_meta,
cancellable, error))
return FALSE;
g_print ("OSTree commit: %s\n", checksum);
{ OstreeRepoCommitState commitstate;
gboolean has_commit;
if (!ostree_repo_has_object (self->repo, OSTREE_OBJECT_TYPE_COMMIT, checksum,
&has_commit, cancellable, error))
return FALSE;
if (has_commit)
{
if (!ostree_repo_load_commit (self->repo, checksum, NULL, &commitstate, error))
return FALSE;
if (!(commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL))
{
g_print ("Commit is already written, nothing to do\n");
return TRUE; /* 🔚 Early return */
}
}
}
if (!g_str_equal (checksum, provided_commit))
return glnx_throw (error, "Package '%s' commit mismatch; Provides=%s, actual=%s",
dnf_package_get_nevra (oirpm_pkg), provided_commit, checksum);
g_printerr ("TODO implement GPG verification\n");
@ -278,32 +351,13 @@ impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
if (!commit_and_print (self, &txn, cancellable, error))
return FALSE;
/* Download any packages we don't already have imported */
g_autoptr(GPtrArray) pkgs_required = g_ptr_array_new_with_free_func (g_object_unref);
const guint n_pkgs = g_variant_n_children (pkgs);
for (guint i = 0; i < n_pkgs; i++)
{
const char *name, *version, *release, *architecture;
const char *repodata_checksum;
guint64 epoch;
g_variant_get_child (pkgs, i, "(&st&s&s&s&s)",
&name, &epoch, &version, &release, &architecture,
&repodata_checksum);
// TODO: use repodata checksum, but probably only if covered by the ostree
// gpg sig?
DnfPackage *pkg = query_nevra (dnfctx, name, epoch, version, release, architecture, error);
// FIXME: We shouldn't require a package to be in the repos if we already
// have it imported otherwise we'll break upgrades for ancient systems
if (!pkg)
return FALSE;
g_ptr_array_add (pkgs_required, g_object_ref (pkg));
}
g_print ("Jigdo from %u packages\n", pkgs_required->len);
/* And now, process the jigdo set */
if (!rpmostree_context_set_packages (self->ctx, pkgs_required, cancellable, error))
return FALSE;
/* See what packages we need to import, print their size. TODO clarify between
* download/import.
*/
g_autoptr(GHashTable) pkgset_to_import = g_hash_table_new (NULL, NULL);
{ g_autoptr(GPtrArray) pkgs_to_import = rpmostree_context_get_packages_to_import (self->ctx);
guint64 dlsize = 0;
@ -317,6 +371,7 @@ impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
g_print ("%u packages to import, download size: %s\n", pkgs_to_import->len, dlsize_fmt);
}
/* Parse the xattr data in the jigdoRPM */
g_autoptr(GHashTable) pkg_to_xattrs = g_hash_table_new_full (NULL, NULL,
(GDestroyNotify)g_object_unref,
(GDestroyNotify)g_variant_unref);
@ -335,6 +390,7 @@ impl_jigdo2commit (RpmOstreeJigdo2CommitContext *self,
g_hash_table_insert (pkg_to_xattrs, g_object_ref (pkg), g_steal_pointer (&objid_to_xattrs));
}
/* Start the download and import, using the xattr data from the jigdoRPM */
if (!rpmostree_context_download (self->ctx, cancellable, error))
return FALSE;
g_autoptr(GVariant) xattr_table = rpmostree_jigdo_assembler_get_xattr_table (jigdo);

View File

@ -68,6 +68,8 @@ struct RpmOstreeImporter
DnfPackage *pkg;
char *hdr_sha256;
guint n_jigdo_skipped;
guint n_jigdo_total;
char *ostree_branch;
gboolean jigdo_mode;
@ -476,6 +478,13 @@ build_metadata_variant (RpmOstreeImporter *self,
g_variant_builder_add (&metadata_builder, "{sv}",
"rpmostree.jigdo",
g_variant_new_boolean (TRUE));
g_variant_builder_add (&metadata_builder, "{sv}",
"rpmostree.jigdo_n_skipped",
g_variant_new_uint32 (self->n_jigdo_skipped));
g_variant_builder_add (&metadata_builder, "{sv}",
"rpmostree.jigdo_total",
g_variant_new_uint32 (self->n_jigdo_total));
}
if (self->doc_files)
@ -696,6 +705,8 @@ jigdo_filter_cb (OstreeRepo *repo,
if (error_was_set)
return OSTREE_REPO_COMMIT_FILTER_SKIP;
self->n_jigdo_total++;
if (g_file_info_get_file_type (file_info) != G_FILE_TYPE_DIRECTORY)
{
self->jigdo_next_xattrs = NULL;
@ -706,7 +717,10 @@ jigdo_filter_cb (OstreeRepo *repo,
return OSTREE_REPO_COMMIT_FILTER_SKIP;
/* No xattrs means we don't need to import it */
if (!self->jigdo_next_xattrs)
return OSTREE_REPO_COMMIT_FILTER_SKIP;
{
self->n_jigdo_skipped++;
return OSTREE_REPO_COMMIT_FILTER_SKIP;
}
}
return OSTREE_REPO_COMMIT_FILTER_ALLOW;

View File

@ -64,7 +64,6 @@ struct RpmOstreeJigdoAssembler
DnfPackage *pkg;
GVariant *commit;
GVariant *meta;
GVariant *pkgs;
char *checksum;
GVariant *xattrs_table;
struct archive *archive;
@ -82,7 +81,6 @@ rpmostree_jigdo_assembler_finalize (GObject *object)
archive_read_free (self->archive);
g_clear_pointer (&self->commit, (GDestroyNotify)g_variant_unref);
g_clear_pointer (&self->meta, (GDestroyNotify)g_variant_unref);
g_clear_pointer (&self->pkgs, (GDestroyNotify)g_variant_unref);
g_free (self->checksum);
g_clear_object (&self->pkg);
g_clear_pointer (&self->xattrs_table, (GDestroyNotify)g_variant_unref);
@ -260,14 +258,13 @@ parse_checksum_from_pathname (const char *pathname,
}
/* First step: read metadata: the commit object and its metadata, suitable for
* GPG verification, as well as the component package NEVRAs.
* GPG verification.
*/
gboolean
rpmostree_jigdo_assembler_read_meta (RpmOstreeJigdoAssembler *self,
char **out_checksum,
GVariant **out_commit,
GVariant **out_detached_meta,
GVariant **out_pkgs,
GCancellable *cancellable,
GError **error)
{
@ -314,27 +311,13 @@ rpmostree_jigdo_assembler_read_meta (RpmOstreeJigdoAssembler *self,
self->next_entry = entry; /* Stash for next call */
}
/* And the component packages */
entry = jigdo_require_next_entry (self, cancellable, error);
entry_path = peel_entry_pathname (entry, error);
if (!entry_path)
return FALSE;
if (!g_str_equal (entry_path, RPMOSTREE_JIGDO_PKGS))
return glnx_throw (error, "Unexpected state for path: %s", entry_path);
g_autoptr(GVariant) pkgs = jigdo_read_variant (RPMOSTREE_JIGDO_PKGS_VARIANT_FORMAT,
self->archive, entry, cancellable, error);
if (!pkgs)
return FALSE;
self->state = STATE_DIRMETA;
self->checksum = g_strdup (actual_checksum);
self->commit = g_variant_ref (commit);
self->meta = meta ? g_variant_ref (meta) : NULL;
self->pkgs = g_variant_ref (pkgs);
*out_checksum = g_steal_pointer (&checksum);
*out_commit = g_steal_pointer (&commit);
*out_detached_meta = g_steal_pointer (&meta);
*out_pkgs = g_steal_pointer (&pkgs);
return TRUE;
}

View File

@ -43,7 +43,6 @@ rpmostree_jigdo_assembler_read_meta (RpmOstreeJigdoAssembler *jigdo,
char **out_checksum,
GVariant **commit,
GVariant **detached_meta,
GVariant **pkgs,
GCancellable *cancellable,
GError **error);

View File

@ -33,12 +33,6 @@
* so that can be GPG verified first - if that fails, we can then cleanly
* abort.
*
* Next, we have the "jigdo set" - the NEVRAs + repodata checksum of the RPM
* packages we need. These requires are also included in the RPM, but we also
* have the repodata checksum here so that it's covered by the RPM GPG
* signature, increasing security. The plan is to ensure that the repodata
* checksums match the ones in this set.
*
* The dirmeta/dirtree objects that are referenced by the commit follow.
*
* A special optimization is made for "content-identical" new objects,
@ -61,8 +55,6 @@
/* Use a numeric prefix to ensure predictable ordering */
#define RPMOSTREE_JIGDO_COMMIT_DIR "00commit"
#define RPMOSTREE_JIGDO_PKGS "01pkgs"
#define RPMOSTREE_JIGDO_PKGS_VARIANT_FORMAT (G_VARIANT_TYPE ("a(stssss)")) // NEVRA,repodata checksum
#define RPMOSTREE_JIGDO_DIRMETA_DIR "02dirmeta"
#define RPMOSTREE_JIGDO_DIRTREE_DIR "03dirtree"
//#define RPMOSTREE_JIGDO_NEW_PKGIDENT "04new-pkgident"
@ -78,7 +70,8 @@
/* NEVRA + xattr table */
#define RPMOSTREE_JIGDO_XATTRS_PKG_VARIANT_FORMAT (G_VARIANT_TYPE ("a(su)"))
#define RPMOSTREE_JIGDO_PROVIDE_V1 "rpmostree-jigdo(v1)"
#define RPMOSTREE_JIGDO_PROVIDE_V2 "rpmostree-jigdo(v2)"
#define RPMOSTREE_JIGDO_PROVIDE_COMMIT "rpmostree-jigdo-commit"
/* This one goes in the spec file to use as our replacement */
#define RPMOSTREE_JIGDO_SPEC_META_MAGIC "#@@@rpmostree_jigdo_meta@@@"