https://bugzilla.redhat.com/show_bug.cgi?id=2332914 --- Comment #10 from Fabio Valentini <decathorpe@xxxxxxxxx> --- > Spec URL: > https://download.copr.fedorainfracloud.org/results/alexhaydock/quickemu/ > fedora-42-x86_64/08832097-quickemu/quickemu.spec > SRPM URL: > https://download.copr.fedorainfracloud.org/results/alexhaydock/quickemu/ > fedora-42-x86_64/08832097-quickemu/quickemu-4.9.7-5.fc42.patched.src.rpm Thanks for the update - I'll take another look now! > I rebuilt this since I posted, using the latest upstream 4.9.7 code. It also > builds (tested and working nicely on a daily basis) on F42. Great! > There are a few things I know probably need resolving before this could be > accepted though. > * Patch1 is based on upstream (merged) PR which fixes an active bug in F41+ > but hasn't made it to a release yet: > https://github.com/quickemu-project/quickemu/pull/1565 That's fine, just put a quick comment + a link to this PR (or the commit corresponding to the merged PR) alongside Patch1. > * After writing the SPEC above I found in the patching guidelines that I > need to add a comment referencing where the patch exists upstream > * With the above in mind, I probably can't include Patch2, 3, or 4. They're > all based on this PR I authored, but it hasn't been accepted upstream yet: > https://github.com/quickemu-project/quickemu/pull/1579 That's fine too. A pull request that has been sent to upstream is a perfectly fine reference for local patches. Just document what those patches do and / or why they are required, and include a link to the PR itself in the spec file. > * I probably also need to reset the package versioning to 4.9.7-1 as the > changelog includes a bunch of my own experimentation while learning how to > properly patch SPEC files. This will happen automatically when you import the package, assuming you don't import your whole git history too :) ================================================================================ There's some other issues with the package I see: 0. I would recommend to consistently indent tag values, I usually use 16 spaces because that leaves enough room for all common tags names. You're currently using a mix of 12 spaces / three tabs and no indentation at all, which looks a bit strange, but this is a matter of "style" (or "taste"), so you there is no hard requirement for this. 1. You will need to drop the ".patched" suffix for %autorelease at some point, but at the very latest before importing the package to Fedora. 2. The "Group" tag is obsolete and SHOULD NOT be used: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections You can just drop it. 3. The Source0 URL doesn't use the recommended format for GitHub tarballs. They are documented here: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_tags It would look like this in your case: Source: %{url}/archive/%{version}/%{name}-%{version}.tar.gz 4. Since all your patches are upstream commits and / or PRs, you could just reference them as such: Patch: https://github.com/quickemu-project/quickemu/pull/1565.patch Patch: https://github.com/quickemu-project/quickemu/pull/1579.patch This way their "upstream status" is immediately documented, and you just need to add some comments why you include them. (GitHub provides actual .patch files under those URLs if you just attach ".patch" to a Pull Request URL, you can download them with "spectool -g" as usual.) 5. I don't understand what the sha256 sum thing here is about: > # Define upstream SHA256SUM for the .tar.gz matching this release version > # curl -fsSL https://github.com/quickemu-project/quickemu/archive/refs/tags/4.9.7.tar.gz -o - 2>/dev/null | sha256sum > %define SHA256SUM0 38a93301a2b233bc458c62d1228d310a9c29c63c10d008c2905029ca66188606 The checksum of source archives will be determined and checked before uploading them to Fedora servers, and every time they are used for builds - so there is no need to do this manually. You can drop the manual checksum creation and checking entirely. 6. All Requires and Recommends MUST be resolvable from Fedora repositories: > # Based on: https://github.com/quickemu-project/quickemu/wiki/01-Installation#install-requirements-on-fedora > # The `zsync` package is listed under Recommends as it is no longer packaged in mainline Fedora So you will need to drop "Recommends: zsync" if it is no longer available from Fedora: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_package_dependencies 7. "%global debug_package %{nil}" This should not be necessary. Your package contains no ELF objects with debuginfo, so it's obvious that there *can* be no debuginfo. Add this instead (somewhere between the "Patch" lines and "Requires": ``` BuildArch: noarch ``` This means that your package has only architecture-independent content and metadata, and causes generation of debuginfo to be skipped. 8. Use best practices for %prep scriptlet. > %prep > # Validate our checksum > echo "%SHA256SUM0 %SOURCE0" | sha256sum -c - > %setup -q > > %autopatch This looks very strange. You should be able to just replace this with: ``` %prep %autosetup -p1 ``` https://docs.fedoraproject.org/en-US/packaging-guidelines/#_autosetup (It's a bit strange that this page mentions %autosetup as an "alternative" - it really should be the default for new packages nowadays.) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2332914 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202332914%23c10 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
