https://bugzilla.redhat.com/show_bug.cgi?id=2092965
Bug ID: 2092965 Summary: Review Request: libkrunfw - A dynamic library bundling the guest payload consumed by libkrun Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: slopezpa@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/slp/libkrunfw/fedora-rawh... SRPM URL: https://download.copr.fedorainfracloud.org/results/slp/libkrunfw/fedora-rawh... Description: A dynamic library bundling the guest payload consumed by libkrun
Fedora Account System Username: slp
https://bugzilla.redhat.com/show_bug.cgi?id=2092965
Cole Robinson crobinso@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? | |needinfo?(slopezpa@redhat.c | |om) Status|NEW |ASSIGNED
--- Comment #1 from Cole Robinson crobinso@redhat.com --- Before the kernel `Source:` line can you add a comment that bundling of the kernel package was approved by fedora kernel maintainers, and link to the discussion: https://lists.fedorahosted.org/archives/list/kernel@lists.fedoraproject.org/...
* Add a comment explaining the exclusivearch. `package only buildable for $ARCHES` is enough IMO
* Typo 'pacakge' * Use %make_install PREFIX=%{_prefix}
BuildRequires: I see the list is copied from kernel distgit but a lot of that is redundant and possibly not even used with your libkrunfw config. First you can remove everything that's explicitly listed in the buildroot: https://pagure.io/fedora-comps/blob/main/f/comps-f37.xml.in#_324 For a f36 x86_64 build I got it down to:
# libkrunfw + packaging requirements BuildRequires: gcc BuildRequires: git-core BuildRequires: make BuildRequires: python3-pyelftools
# kernel build requirements BuildRequires: bison BuildRequires: flex BuildRequires: bc BuildRequires: elfutils-devel
But no runtime testing, and I didn't try dropping bison+flex+bc, I just saw they were in build output.
https://bugzilla.redhat.com/show_bug.cgi?id=2092965
Cole Robinson crobinso@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |crobinso@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=2092965
Sergio Lopez slopezpa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(slopezpa@redhat.c | |om) |
--- Comment #2 from Sergio Lopez slopezpa@redhat.com --- (In reply to Cole Robinson from comment #1)
Before the kernel `Source:` line can you add a comment that bundling of the kernel package was approved by fedora kernel maintainers, and link to the discussion: https://lists.fedorahosted.org/archives/list/kernel@lists.fedoraproject.org/ thread/2TMXPCE2VWF7USZA7OHQ3P2SBJAEGCSX/
- Add a comment explaining the exclusivearch. `package only buildable for
$ARCHES` is enough IMO
- Typo 'pacakge'
- Use %make_install PREFIX=%{_prefix}
BuildRequires: I see the list is copied from kernel distgit but a lot of that is redundant and possibly not even used with your libkrunfw config. First you can remove everything that's explicitly listed in the buildroot: https://pagure.io/fedora-comps/blob/main/f/comps-f37.xml.in#_324 For a f36 x86_64 build I got it down to:
# libkrunfw + packaging requirements BuildRequires: gcc BuildRequires: git-core BuildRequires: make BuildRequires: python3-pyelftools
# kernel build requirements BuildRequires: bison BuildRequires: flex BuildRequires: bc BuildRequires: elfutils-devel
But no runtime testing, and I didn't try dropping bison+flex+bc, I just saw they were in build output.
You nailed the required packages, I've tried removing "bison", "flex" and "bc" individually and all of them are required. We also need "perl-interpreter" on aarch64.
I've produced a new COPR build with the new spec. I think I've applied all your suggestions, please let me know if I missed something:
- https://download.copr.fedorainfracloud.org/results/slp/libkrunfw/fedora-rawh... - https://download.copr.fedorainfracloud.org/results/slp/libkrunfw/fedora-rawh...
Thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=2092965
Cole Robinson crobinso@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #3 from Cole Robinson crobinso@redhat.com --- Looks good to me now, setting fedora-review+
https://bugzilla.redhat.com/show_bug.cgi?id=2092965
--- Comment #4 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libkrunfw
https://bugzilla.redhat.com/show_bug.cgi?id=2092965
Sergio Lopez slopezpa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE Last Closed| |2022-06-17 11:38:03
--- Comment #5 from Sergio Lopez slopezpa@redhat.com --- The package has hit rawhide, thanks Cole and Gwyn!
package-review@lists.fedoraproject.org