libsolv %_dbpath fixes rollup

[jlebon]

We trigger a librpm macro file load in many of our paths. Since the
default value shipped by rpm's macro file sets _dbpath to
/var/lib/rpm, we have to explicitly set that back to /usr/share/rpm
in those paths.

This became more problematic recently with libsolv v0.7.17 which fully
keys off of _dbpath to find the rpmdb path to load:

openSUSE/libsolv@04d4d03

And it's not technically wrong; we really should make that macro not
lie. This is what this patch does by injecting an RPM macro file in our
composes which sets it to /usr/lib/sysimage/rpm (which currently is a
symlink to /usr/share/rpm, but eventually will become the canonical
location). So then e.g. the rpm CLI doesn't actually need the
/var/lib/rpm backcompat link anymore, though there's no harm in
leaving it.

In the future, we should be able to drop this once we move all of Fedora
to /usr/lib/sysimage/rpm (see
coreos/fedora-coreos-tracker#639).

Closes: #2548

[cgwalters]

/retest
Weird, not seeing failure logs in Prow artifacts.

[cgwalters]

Ohh, I think Prow may not do CI for draft PRs by default and so switching to draft cancelled those jobs? Let's see if the explicit test request works.

[cgwalters]

OK, successfully failed here:

initramfs: error: syscore cleanup: regenerating refs: pkgcache cleanup: Deployment index=1: Failed to find any packages in root

[cgwalters]

Looks like commit message needs adjusting too, otherwise LGTM!

[cgwalters]

One thing to bear in mind is this will also break the use case of e.g. dnf --installroot directly on an rpm-ostree based host system. Which admittedly is pretty obscure...but until we unify the path it's going to cause obscure and hard to fix fallout.

[cgwalters]

Anyways, LGTM - let's lift the draft and remove "DNM:" tag?

[jlebon]

Anyways, LGTM - let's lift the draft and remove "DNM:" tag?

I'm waiting on CI to make sure it's green with the new libsolv before dropping that commit and tweaking the commit message.

Although... probably not a bad idea to just keep that override anyway.

[cgwalters]

Yeah let's keep the override for a little bit at least I think; we have coverage of older libsolv via kola tests.
/lgtm

[cgwalters]

Huh interesting, upgrade test failed with

Feb  9 00:07:36.124276 systemd-coredump[1164]: Process 1104 (rpm-ostree) of user 978 dumped core.
                                               
                                               Stack trace of thread 1104:
                                               #0  0x00007f29ec6fd9d5 raise (libc.so.6 + 0x3d9d5)
                                               #1  0x00007f29ec6e68a4 abort (libc.so.6 + 0x268a4)
                                               #2  0x00007f29ed159b6c g_assertion_message.cold (libglib-2.0.so.0 + 0x1db6c)
                                               #3  0x00007f29ed1b498a g_assertion_message_cmpnum (libglib-2.0.so.0 + 0x7898a)
                                               #4  0x00007f29ed67a9a5 _rpm_ostree_package_list_for_commit (librpmostree-1.so.1 + 0x69a5)
                                               #5  0x00007f29ed67a240 rpm_ostree_db_diff_ext (librpmostree-1.so.1 + 0x6240)
                                               #6  0x00007f29ed67a393 rpm_ostree_db_diff (librpmostree-1.so.1 + 0x6393)
                                               #7  0x0000562f94ee02aa rpmostree_print_treepkg_diff_from_sysroot_path (rpm-ostree + 0x27c2aa)
                                               #8  0x0000562f94ed20a2 rpmostree_builtin_deploy (rpm-ostree + 0x26e0a2)
                                               #9  0x0000562f94ed0af8 _ZN12rpmostreecxx14rpmostree_mainEN4rust10cxxbridge15SliceIKNS1_3StrEEE (rpm-ostree + 0x26caf8)
                                               #10 0x0000562f94ecefe0 rpmostreecxx$cxxbridge1$rpmostree_main (rpm-ostree + 0x26afe0)
                                               #11 0x0000562f94cc75d0 _ZN10rpm_ostree4main17h038527a03685d1f3E (rpm-ostree + 0x635d0)
                                               #12 0x0000562f94cc6d33 _ZN3std10sys_common9backtrace28__rust_begin_short_backtrace17h729ff9d57aa85eedE (rpm-ostree + 0x62d33)
                                               #13 0x0000562f94cc7b58 main (rpm-ostree + 0x63b58)
                                               #14 0x00007f29ec6e81e2 __libc_start_main (libc.so.6 + 0x281e2)
                                               #15 0x0000562f94cc6c6e _start (rpm-ostree + 0x62c6e)
                                               
                                               Stack trace of thread 1106:
                                               #0  0x00007f29ec7b6a5f __poll (libc.so.6 + 0xf6a5f)
                                               #1  0x00007f29ed1e16f6 g_main_context_iterate.constprop.0 (libglib-2.0.so.0 + 0xa56f6)
                                               #2  0x00007f29ed18f033 g_main_loop_run (libglib-2.0.so.0 + 0x53033)
                                               #3  0x00007f29ed3d9d1a gdbus_shared_thread_func.lto_priv.0 (libgio-2.0.so.0 + 0x10fd1a)
                                               #4  0x00007f29ed1bd2b2 g_thread_proxy (libglib-2.0.so.0 + 0x812b2)
                                               #5  0x00007f29ec8943f9 start_thread (libpthread.so.0 + 0x93f9)
                                               #6  0x00007f29ec7c1b53 __clone (libc.so.6 + 0x101b53)
                                               
                                               Stack trace of thread 1105:
                                               #0  0x00007f29ec7b6a5f __poll (libc.so.6 + 0xf6a5f)
                                               #1  0x00007f29ed1e16f6 g_main_context_iterate.constprop.0 (libglib-2.0.so.0 + 0xa56f6)
                                               #2  0x00007f29ed18cd43 g_main_context_iteration (libglib-2.0.so.0 + 0x50d43)
                                               #3  0x00007f29ed18e961 glib_worker_main (libglib-2.0.so.0 + 0x52961)
                                               #4  0x00007f29ed1bd2b2 g_thread_proxy (libglib-2.0.so.0 + 0x812b2)
                                               #5  0x00007f29ec8943f9 start_thread (libpthread.so.0 + 0x93f9)
                                               #6  0x00007f29ec7c1b53 __clone (libc.so.6 + 0x101b53)
                                               
                                               Stack trace of thread 1162:
                                               #0  0x00007f29ec7bc55d syscall (libc.so.6 + 0xfc55d)
                                               #1  0x00007f29ed18cf72 g_source_attach (libglib-2.0.so.0 + 0x50f72)
                                               #2  0x00007f29ed377558 g_task_return.part.0 (libgio-2.0.so.0 + 0xad558)
                                               #3  0x00007f29ed3777ea g_task_thread_pool_thread (libgio-2.0.so.0 + 0xad7ea)
                                               #4  0x00007f29ed1bfe54 g_thread_pool_thread_proxy.lto_priv.0 (libglib-2.0.so.0 + 0x83e54)
                                               #5  0x00007f29ed1bd2b2 g_thread_proxy (libglib-2.0.so.0 + 0x812b2)
                                               #6  0x00007f29ec8943f9 start_thread (libpthread.so.0 + 0x93f9)
                                               #7  0x00007f29ec7c1b53 __clone (libc.so.6 + 0x101b53)
                                               
Feb  9 00:07:36.138341 zincati[1055]: [ERROR] failed to stage deployment: rpm-ostree deploy failed:
Feb  9 00:07:36.138341 zincati[1055]:     **
Feb  9 00:07:36.138341 zincati[1055]:     ERROR:src/lib/rpmostree-package.c:256:_rpm_ostree_package_list_for_commit: assertion failed (pkglist->len > 0): (0 > 0)
Feb  9 00:07:36.138341 zincati[1055]:     

This is the upgrade-from-current kola test which certainly looks like a legitimate failure.

OK so the main CoreOS CI flow all uses the updated rpm-ostree compose side, where as this test will end up exercising "new rpm-ostree client side, old server side".

[cgwalters]

OK coreos-assembler already has the new libsolv, so that was used at compose time. It'll be in the target ostree too. The upgrade test grabbed...ah yes, the testing-devel build with libsolv reverted. It was also built with an rpm-ostree before this patch, obviously.

So our starting state is (old rpm-ostree, old libsolv) and then kola pulls an ostree commit with (new rpm-ostree, new libsolv). I...think what's happening is it's actually the old rpm-ostree failing to load pkglist from the new ostree commit.

[cgwalters]

Aaand...trying to reproduce this locally it passes, of course.

[cgwalters]

@jlebon OK if we restart CI on this? I want to see if this is somehow a flake (seems unlikely though). Or we could try to push more debugging bits to this PR.

[cgwalters]

Hmm shouldn't cargo respect the make jobserver? rust-lang/cargo#4110

[cgwalters]

(Eh we can look at this later)

[jlebon]

Hmm, seems like it would.

Ohhh, I think it might be the cmake call for libdnf which is not capped. I wonder if the issue is that the jobserver isn't surviving the make -> cmake -> make chain. Briefly searching around, it's not clear to me whether cmake supports using a parent job server for the children make processes.

Next time we hit the libgomp thing, let's try something like this instead:

diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile
index 5cd9e699..29d11897 100644
--- a/.cci.jenkinsfile
+++ b/.cci.jenkinsfile
@@ -6,7 +6,7 @@ parallel rpms: {
   cosaPod(buildroot: true, runAsUser: 0, memory: "2Gi", cpu: "${n}") {
       checkout scm
       // 2:1 job to CPU at most should keep us from getting kicked out
-      shwrap("""RPM_BUILD_NCPUS=${n} CARGO_BUILD_JOBS=${n} ./ci/coreosci-rpmbuild.sh
+      shwrap("""RPM_BUILD_NCPUS=${n} CMAKE_BUILD_PARALLEL_LEVEL=${n} ./ci/coreosci-rpmbuild.sh
                 mv packaging/*/*.rpm .
              """)
       // make it easy for anyone to download the RPMs

Hmm, or maybe that should be in the spec file instead since RPM_BUILD_NCPUS really should be the only top-level knob.

[cgwalters]

Ohhh, I think it might be the cmake call for libdnf which is not capped. I wonder if the issue is that the jobserver isn't surviving the make -> cmake -> make chain.

I think it should all be inherited by the environment by default? Unless the child make explicitly invokes make -jN which AIUI resets things.

[cgwalters]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [cgwalters,jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[cgwalters]

OK next failure is

layering-scripts: error: Ran rpm -q bash in %post

which must be related; I bet we were implicitly relying on /var/lib/rpm not being accessible, but now it is. So...hmm, maybe let's do --bind-ro /usr/share/empty /usr/share/rpm in scripts?

[cgwalters]

Testing out locally

diff --git a/src/libpriv/rpmostree-scripts.cxx b/src/libpriv/rpmostree-scripts.cxx
index f7adb08f..f871fc0f 100644
--- a/src/libpriv/rpmostree-scripts.cxx
+++ b/src/libpriv/rpmostree-scripts.cxx
@@ -378,6 +378,9 @@ rpmostree_run_script_in_bwrap_container (int rootfs_fd,
   if (glnx_fstatat (rootfs_fd, "usr/lib/opt", &stbuf, AT_SYMLINK_NOFOLLOW, NULL) && S_ISDIR(stbuf.st_mode))
     rpmostree_bwrap_append_bwrap_argv (bwrap, "--symlink", "usr/lib/opt", "/opt", NULL);
 
+  /* Don't let scripts see the base rpm database by default */
+  rpmostree_bwrap_bind_read (bwrap, "usr/share/empty", "usr/share/rpm");
+
   /* Add ostree-booted API; some scriptlets may work differently on OSTree systems; e.g.
    * akmods. Just create it manually; /run is usually tmpfs, but scriptlets shouldn't be
    * adding stuff there anyway. */

[openshift-ci-robot]

New changes are detected. LGTM label has been removed.

[cgwalters]

Yep that works, pushed.

[cgwalters]

We're definitely reaping the value of all that work on the test suite here!

[jlebon]

I bet we were implicitly relying on /var/lib/rpm not being accessible, but now it is. So...hmm, maybe let's do --bind-ro /usr/share/empty /usr/share/rpm in scripts?

Yup thanks, makes sense to me! I think a lot of the work we're doing here is good prep for when we actually move the rpmdb to /usr/lib/sysimage/rpm!

[openshift-ci-robot]

New changes are detected. LGTM label has been removed.

[cgwalters]

Fixed the compose test case to look for /usr/share/rpm again.

[rajveermalviya]

will this be backported in https://src.fedoraproject.org/rpms/rpm-ostree ?

[cgwalters]

Yes, either that or we do a new release. Probably makes the most sense to backport.