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

Migrate test-suite macro configuration out of root's ~/.rpmmacros #3524

Closed

Conversation

pmatilai
Copy link
Member

See commits for details.

Somehow nobody has noticed we don't create the per-host configuration
directory on install, probably didn't even in autoconf days.
We'll need this now in the next commits though.
@pmatilai pmatilai requested a review from dmnks January 16, 2025 11:34
@pmatilai pmatilai requested a review from a team as a code owner January 16, 2025 11:34
@pmatilai pmatilai requested review from ffesti and removed request for a team January 16, 2025 11:34
@ffesti
Copy link
Contributor

ffesti commented Jan 16, 2025

tests 211 and 213 break for 39578b4 and a54e29f

#                             -*- compilation -*-
211. rpmsigdig.at:198: testing rpmkeys key update (fs) ...
/rpmsigdig.at:203:
runroot rpmkeys --import /data/keys/rpm.org-rsa-2048-test.pub
runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm

/rpmsigdig.at:215:
runroot rpmkeys --list | wc -l
runroot rpmkeys --import /data/keys/rpm.org-rsa-2048-add-subkey.asc
runroot rpmkeys --list | wc -l
runroot_other find /usr/lib/sysimage/rpm/pubkeys -name "*.key" | wc -l

--- /dev/null   2024-11-25 09:28:36.241999924 +0000
+++ /srv/rpmtests.dir/at-groups/211/stderr      2025-01-16 14:46:48.670360456 +0000
@@ -0,0 +1 @@
+find: '/usr/lib/sysimage/rpm/pubkeys': No such file or directory
--- -   2025-01-16 14:46:48.674806192 +0000
+++ /srv/rpmtests.dir/at-groups/211/stdout      2025-01-16 14:46:48.673360522 +0000
@@ -1,4 +1,4 @@
 1
 1
-1
+0
#                             -*- compilation -*-
213. rpmsigdig.at:277: testing rpmkeys key update (openpgp) ...
/rpmsigdig.at:282:
runroot rpmkeys --import /data/keys/rpm.org-rsa-2048-test.pub
runroot rpmkeys -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-new-subkey.rpm

/rpmsigdig.at:294:
runroot_other sq --cert-store=/usr/lib/sysimage/rpm/pubkeys cert list --gossip 2>&1 | grep -e "^ - [[:xdigit:]]\{40\}" | cut -c4-

--- -   2025-01-16 14:46:48.500560679 +0000
+++ /srv/rpmtests.dir/at-groups/213/stdout      2025-01-16 14:46:48.498356664 +0000
@@ -1,2 +1 @@
-771B18D3D7BAA28734333C424344591E1964C5FC
 
213. rpmsigdig.at:277: 213. rpmkeys key update (openpgp) (rpmsigdig.at:277): FAILED (rpmsigdig.at:294)

Copy link
Contributor

@dmnks dmnks left a comment

Choose a reason for hiding this comment

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

Some suggestions around the use of ARG, see comments inline. Otherwise, looks sane :)

tests/Dockerfile.fedora Outdated Show resolved Hide resolved
tests/Dockerfile.fedora Outdated Show resolved Hide resolved
tests/Dockerfile.fedora Outdated Show resolved Hide resolved
tests/Dockerfile.fedora Outdated Show resolved Hide resolved
tests/mktree.oci Outdated Show resolved Hide resolved
@pmatilai
Copy link
Member Author

pmatilai commented Jan 17, 2025

tests 211 and 213 break for 39578b4 and a54e29f

What does that mean? It's passing the CI... If it's your local build that breaks then we'll need more details about that.

@ffesti
Copy link
Contributor

ffesti commented Jan 17, 2025

it breaks is I checkout those patches instead of the whole branch.

@dmnks
Copy link
Contributor

dmnks commented Jan 17, 2025

it breaks is I checkout those patches instead of the whole branch.

We've discussed this in the past - it would be nice if we ensured that every commit passes the CI separately, but then, it would make you wait a lot longer for it to finish. So it's really a compromise. Most of the time, I think it's fine to just CI the overall patchset. When you need to bisect/backport individual patches, that's where you may get burned, of course.

These /usr/lib/sysimage/rpm/pubkeys paths are valid for the OS rpm
in the test-container, but that might not have any correlation to
the *built* rpm we're supposed to be testing here. This is masked
by the %_dbpath override in the Dockerfile which we'll be removing
in the next step.
This override exists to make system rpmdb accessible in eg "make shell"
but it causes complications wrt the prefix (see below + next commits) and
also masks test bugs (see previous commit).

We can resurrect this functionality later on if we find solutions that
avoid the issues.

This is the first step to fixing rpm-software-management#3521.
Signing key configuration is obviously something that belongs to the
user's macros, but lets get on with the times and use the new XDG config
directory for that.

The remaining references to ~/.rpmmacros in rpmmacro.at are related to
testing behavior with the old-style configuration and need to be preserved.

This is the fourth and final step to rpm-software-management#3521.

Fixes: rpm-software-management#3521
@pmatilai
Copy link
Member Author

Dropped the whole %_dbpath override thing now - as this exercise points out it can actually mask test-bugs so we'd better find some other way of achieving that before reintroducing.

All individual commits should also pass now. That's a standard we should always try to adhere to, but easy to miss...

@pmatilai
Copy link
Member Author

pmatilai commented Jan 17, 2025

Should, but apparently I only tested "make check" 🤦 (or something). Argh.

@dmnks
Copy link
Contributor

dmnks commented Jan 17, 2025

Yeah, it's the prefix biting us back in return... 🤦

@pmatilai pmatilai marked this pull request as draft January 17, 2025 10:25
@pmatilai
Copy link
Member Author

This turned out quite differently from what I initially planned so I'll just close this and start a new PR instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants