jigdo: V4: Use archful provides for jigdoRPM Requires

When I tried to use my WIP client patches to do:
`rpm-ostree rebase rojig://fahc:fedora-atomic-host`,
I got a missing file object which turned out to
be the client importing the i686 RPMs.

This was passing in the test suite because we don't mirror i686 of course, but
on the client side right now we end up using all enabled repos, and since Fedora
is multiarch, the behavior is going to be...not predictable.

Thinking a bit about on this problem I actually happened to recall
the RPM `%{_isa}` macro which is used in Fedora in various places;
for example to "arch bind" `-devel` packages to their base.  See
for example [this case](33c7dc02bc/f/ostree.spec (_79)) in libostree.

As I noted at first, the core problem here is that the "final"
RPM architecture field is not symmetric in any way with the definition
of that `%{_isa}` macro.  See:

d9d47e0114/installplatform (L25)

The *third* solution I ended up on here is to iterate over the
`Provides` on the server side and we take the first thing
that matches `Provides: %{name}(whatever)`.

I briefly thought about trying to somehow drive into libsolv the
logic to prefer the jigdoRPM's native architecture...IIRC yum did
something like that in the past but it was never done in libsolv?
Anyways the dependencies here are now more correct, so other tools
will also handle it.

Closes: #1213
Approved by: jlebon
This commit is contained in:
Colin Walters 2018-01-17 16:25:39 -05:00 committed by Atomic Bot
parent 4008bcb27c
commit b85ae9e1d6
7 changed files with 96 additions and 32 deletions

View File

@ -24,5 +24,5 @@ if test -x /usr/bin/clang; then
export CFLAGS="-Wall -Werror -Wno-error=deprecated-declarations -Wno-error=macro-redefined -Wno-error=unused-command-line-argument ${CFLAGS:-}"
export CC=clang
git clean -dfx && git submodule foreach git clean -dfx
build
build ${CONFIGOPTS:-}
fi

View File

@ -34,9 +34,5 @@ fi
adduser testuser
export LSAN_OPTIONS=verbosity=1:log_threads=1
BWRAP=/usr/bin/bwrap
# we use smoketested now, which uses a git master-ish version of bwrap
#if [ "$id" == centos ]; then
# BWRAP=/usr/lib64/rpm-ostree/bwrap
#fi
build --enable-installed-tests --enable-gtk-doc --with-bubblewrap=$BWRAP
# And now the build
build --enable-installed-tests --enable-gtk-doc ${CONFIGOPTS:-}

View File

@ -1792,7 +1792,7 @@ setup_jigdo_state (RpmOstreeContext *self,
DnfReldep *provide = dnf_reldep_list_index (provides, i);
const char *provide_str = dnf_reldep_to_string (provide);
if (g_str_equal (provide_str, RPMOSTREE_JIGDO_PROVIDE_V3))
if (g_str_equal (provide_str, RPMOSTREE_JIGDO_PROVIDE_V4))
{
found_vprovide = TRUE;
}
@ -1814,7 +1814,7 @@ setup_jigdo_state (RpmOstreeContext *self,
if (!found_vprovide)
return glnx_throw (error, "Package '%s' does not have Provides: %s",
dnf_package_get_nevra (self->jigdo_pkg), RPMOSTREE_JIGDO_PROVIDE_V3);
dnf_package_get_nevra (self->jigdo_pkg), RPMOSTREE_JIGDO_PROVIDE_V4);
if (!self->jigdo_checksum)
return glnx_throw (error, "Package '%s' does not have Provides: %s",
dnf_package_get_nevra (self->jigdo_pkg), RPMOSTREE_JIGDO_PROVIDE_COMMIT);

View File

@ -57,6 +57,7 @@ pkg_objid_free (PkgObjid *pkgobjid)
typedef struct {
OstreeRepo *repo;
OstreeRepo *pkgcache_repo;
RpmOstreeRefSack *rsack;
guint n_nonunique_objid_basenames;
guint n_objid_basenames;
@ -73,6 +74,7 @@ rpm_ostree_commit2jigdo_context_free (RpmOstreeCommit2JigdoContext *ctx)
{
g_clear_object (&ctx->repo);
g_clear_object (&ctx->pkgcache_repo);
g_clear_pointer (&ctx->rsack, rpmostree_refsack_unref);
g_clear_pointer (&ctx->commit_content_objects, (GDestroyNotify)g_hash_table_unref);
g_clear_pointer (&ctx->content_object_to_pkg_objid, (GDestroyNotify)g_hash_table_unref);
g_clear_pointer (&ctx->objsize_to_object, (GDestroyNotify)g_hash_table_unref);
@ -547,6 +549,49 @@ build_objid_map_for_package (RpmOstreeCommit2JigdoContext *self,
return TRUE;
}
/* Converts e.g. x86_64 to x86-64 (which is the current value of the RPM %{_isa}
* macro). Here's where RPM maintains this currently:
* https://github.com/rpm-software-management/rpm/blob/d9d47e01146a5d4411691a71916b1030ac7da193/installplatform#L25
* For now we scrape all the Provides: looking for a `Provides: %{name}(something)`.
*/
static char *
pkg_get_requires_isa (RpmOstreeCommit2JigdoContext *self,
DnfPackage *pkg,
GError **error)
{
g_autoptr(DnfReldepList) provides = dnf_package_get_provides (pkg);
const gint n_provides = dnf_reldep_list_count (provides);
Pool *pool = dnf_sack_get_pool (self->rsack->sack);
g_autofree char *provides_prefix = g_strconcat (dnf_package_get_name (pkg), "(", NULL);
for (int i = 0; i < n_provides; i++)
{
DnfReldep *req = dnf_reldep_list_index (provides, i);
Id reqid = dnf_reldep_get_id (req);
if (!ISRELDEP (reqid))
continue;
Reldep *rdep = GETRELDEP (pool, reqid);
if (!(rdep->flags & REL_EQ))
continue;
const char *name = pool_id2str (pool, rdep->name);
if (!g_str_has_prefix (name, provides_prefix))
continue;
const char *isa_start = name + strlen (provides_prefix);
const char *endparen = strchr (isa_start, ')');
if (!endparen)
continue;
/* Return the first match. In theory this would blow up if
* e.g. a package started doing a Provides: %{name}(awesome) but...
* why would someone do that? We can address that if it comes up.
*/
return g_strndup (isa_start, endparen - isa_start);
}
return glnx_null_throw (error, "Missing Provides(%s%%{_isa}) in package: %s",
dnf_package_get_name (pkg), dnf_package_get_nevra (pkg));
}
/* Take input spec file and generate a temporary spec file with our metadata
* inserted.
*/
@ -573,18 +618,32 @@ 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_V3 "\n");
g_string_append (replacement, "Provides: " RPMOSTREE_JIGDO_PROVIDE_V4 "\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 */
/* Add Requires: on our dependent packages; note this needs to be
* arch-specific otherwise we may be tripped up by multiarch packages.
*/
for (guint i = 0; i < jigdo_packages->len; i++)
{
DnfPackage *pkg = jigdo_packages->pdata[i];
g_string_append_printf (replacement, "Requires: %s = %s\n",
dnf_package_get_name (pkg),
dnf_package_get_evr (pkg));
if (g_str_equal (dnf_package_get_arch (pkg), "noarch"))
{
g_string_append_printf (replacement, "Requires: %s = %s\n",
dnf_package_get_name (pkg),
dnf_package_get_evr (pkg));
}
else
{
g_autofree char *isa = pkg_get_requires_isa (self, pkg, error);
if (!isa)
return NULL;
g_string_append_printf (replacement, "Requires: %s(%s) = %s\n",
dnf_package_get_name (pkg), isa,
dnf_package_get_evr (pkg));
}
}
g_string_append (replacement, meta + strlen (RPMOSTREE_JIGDO_SPEC_META_MAGIC) + 1);
g_string_append (replacement, "# End data generated by rpm-ostree\n");
@ -827,7 +886,7 @@ write_commit2jigdo (RpmOstreeCommit2JigdoContext *self,
/* Generate empty entries for the "unused set" - the set of packages
* that are part of the install, but carry no content objects actually
* in the tree. ${foo}-filesystem packages are common examples. In
* in the tree. ${foo}-filesystem packages are common examples. Since
* v3 the "jigdo set" is the same as the "install set".
*/
for (guint i = 0; i < pkglist->len; i++)
@ -1021,12 +1080,11 @@ impl_commit2jigdo (RpmOstreeCommit2JigdoContext *self,
g_print ("%u content objects\n", g_hash_table_size (self->commit_content_objects));
g_print ("Finding reachable objects from packages...\n");
g_autoptr(RpmOstreeRefSack) sack =
rpmostree_get_refsack_for_commit (self->repo, commit, cancellable, error);
if (!sack)
self->rsack = rpmostree_get_refsack_for_commit (self->repo, commit, cancellable, error);
if (!self->rsack)
return FALSE;
hy_autoquery HyQuery hquery = hy_query_create (sack->sack);
hy_autoquery HyQuery hquery = hy_query_create (self->rsack->sack);
hy_query_filter (hquery, HY_PKG_REPONAME, HY_EQ, HY_SYSTEM_REPO_NAME);
g_autoptr(GPtrArray) pkglist = hy_query_run (hquery);
g_print ("Building object map from %u packages\n", pkglist->len);

View File

@ -42,18 +42,27 @@
static DnfPackage *
query_jigdo_pkg (DnfContext *dnfctx,
const char *name,
const char *name_arch,
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() */
/* This changed in v4, we now go through the Provides: name(arch) */
const char *arch_start = strchr (name_arch, '(');
g_autofree char *name_owned = NULL;
const char *name;
if (arch_start)
{
name = name_owned = g_strndup (name_arch, arch_start - name_arch);
hy_query_filter (query, HY_PKG_PROVIDES, HY_EQ, name_arch);
}
else
name = name_arch;
hy_query_filter (query, HY_PKG_NAME, HY_EQ, name);
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-%s", name, evr);
return glnx_null_throw (error, "Failed to find package %s = %s", name_arch, evr);
return g_object_ref (pkglist->pdata[0]);
}
@ -124,17 +133,18 @@ rpmostree_context_execute_jigdo (RpmOstreeContext *self,
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.
/* 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);
/* Since v4 the server uses "Provides: name(arch) for archful */
const char *name_arch = pool_id2str (pool, rdep->name);
const char *evr = pool_id2str (pool, rdep->evr);
DnfPackage *pkg = query_jigdo_pkg (dnfctx, name, evr, error);
DnfPackage *pkg = query_jigdo_pkg (dnfctx, name_arch, 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.

View File

@ -70,7 +70,7 @@
/* NEVRA + xattr table */
#define RPMOSTREE_JIGDO_XATTRS_PKG_VARIANT_FORMAT (G_VARIANT_TYPE ("a(su)"))
#define RPMOSTREE_JIGDO_PROVIDE_V3 "rpmostree-jigdo(v3)"
#define RPMOSTREE_JIGDO_PROVIDE_V4 "rpmostree-jigdo(v4)"
#define RPMOSTREE_JIGDO_PROVIDE_COMMIT "rpmostree-jigdo-commit"
/* This one goes in the spec file to use as our replacement */

View File

@ -81,9 +81,9 @@ find jigdo-output -name '*.rpm' | tee rpms.txt
assert_file_has_content rpms.txt 'fedora-atomic-host-42.1.*x86_64'
path=$(head -1 rpms.txt)
rpm -qp --requires ${path} > requires.txt
assert_file_has_content requires.txt 'glibc = '
assert_file_has_content requires.txt 'systemd = '
assert_file_has_content requires.txt 'test-pkg = 1.0-1'
assert_file_has_content requires.txt 'glibc(.*) = '
assert_file_has_content requires.txt 'systemd(.*) = '
assert_file_has_content requires.txt 'test-pkg(.*) = 1.0-1'
# And pull it; we should download the newer version by default
do_jigdo2commit