https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Bug ID: 2388375 Summary: Review Request: gshogi - GTK front-end for GNU Shogi Product: Fedora Version: rawhide OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: akito5623@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: 「attached」 SRPM URL: 「attached」
Description: Simple GTK-based GUI for GNU Shogi engine.
License: GPL-2.0-or-later Upstream URL: https://github.com/johncheetham/gshogi Build tested on: Fedora 42 and Rawhide (mock) FAS Username: redadmin
Reproducible: Always
Steps to Reproduce: 1. Download attached SRPM and SPEC. 2. Build with: mock -r fedora-rawhide-x86_64 --rebuild gshogi-*.src.rpm 3. Verify installation and runtime. Actual Results: Package builds successfully on Fedora 42 and Rawhide (mock).
Expected Results: Package should be accepted into Fedora Rawhide after review.
Additional Information: - New package: gshogi (GTK-based GUI for GNU Shogi engine) - License: GPL-2.0-or-later - Upstream: https://github.com/johncheetham/gshogi - Build tested on Fedora 42 and Rawhide using mock.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #1 from Akiyoshi Kurita akito5623@gmail.com --- Created attachment 2103564 --> https://bugzilla.redhat.com/attachment.cgi?id=2103564&action=edit gshogi - review bundle (SPEC, SRPM, Fedora 42 and Rawhide build logs)
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #2 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Cannot find any valid SRPM URL for this ticket. Common causes are:
- You didn't specify `SRPM URL: ...` in the ticket description or any of your comments - The URL schema isn't HTTP or HTTPS - The SRPM package linked in your URL doesn't match the package name specified in the ticket summary
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #3 from Akiyoshi Kurita akito5623@gmail.com --- SRPM and SPEC files for Fedora package review:
Fedora 42: SRPM URL: https://github.com/redadmin-k/gshogi-fedora42/releases/download/v0.0.8/gshog... SPEC URL: https://github.com/redadmin-k/gshogi-fedora42/releases/download/v0.0.8/gshog...
Fedora 44: SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/releases/download/v0.0.8/gshog... SPEC URL: https://github.com/redadmin-k/gshogi-fedora44/releases/download/v0.0.8/gshog...
Built and tested using mock on both Fedora 42 and Fedora 44.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/johnchee | |tham/gshogi.git
--- Comment #4 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9411132 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- gshogi : /usr/share/gshogi/engine/book.h gshogi : /usr/share/gshogi/engine/debug.h gshogi : /usr/share/gshogi/engine/eval.h gshogi : /usr/share/gshogi/engine/gnushogi.h gshogi : /usr/share/gshogi/engine/opts.h gshogi : /usr/share/gshogi/engine/pattern.h gshogi : /usr/share/gshogi/engine/rawdsp.h gshogi : /usr/share/gshogi/engine/version.h Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages - License file LICENSE is not marked as %license Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuideline...
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #5 from Akiyoshi Kurita akito5623@gmail.com --- SRPM and SPEC files for Fedora package review:
Fedora 42: - SRPM: https://github.com/redadmin-k/gshogi-fedora42/releases/download/v0.0.9/gshog... - SPEC: https://github.com/redadmin-k/gshogi-fedora42/releases/download/v0.0.9/gshog...
Fedora 44: - SRPM: https://github.com/redadmin-k/gshogi-fedora44/releases/download/v0.0.9/gshog... - SPEC: https://github.com/redadmin-k/gshogi-fedora44/releases/download/v0.0.9/gshog...
---
**Description:** `gshogi` is a GTK-based frontend for GNU Shogi. It provides a simple graphical interface to play Shogi against the `gnushogi` engine.
**Build/Test:** Built and tested successfully in `mock` environments: - Fedora 42 (`fedora-42-x86_64`) - Fedora 44 (`fedora-44-x86_64`)
**License:** LICENSE file is included and installed via `%license`.
**Upstream:** https://github.com/johncheetham/gshogi Source tarball based on commit `7c4bd90`.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #6 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103603 --> https://bugzilla.redhat.com/attachment.cgi?id=2103603&action=edit The .spec file difference from Copr build 9411132 to 9411248
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #7 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9411248 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- gshogi : /usr/share/gshogi/engine/book.h gshogi : /usr/share/gshogi/engine/debug.h gshogi : /usr/share/gshogi/engine/eval.h gshogi : /usr/share/gshogi/engine/gnushogi.h gshogi : /usr/share/gshogi/engine/opts.h gshogi : /usr/share/gshogi/engine/pattern.h gshogi : /usr/share/gshogi/engine/rawdsp.h gshogi : /usr/share/gshogi/engine/version.h Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #8 from Akiyoshi Kurita akito5623@gmail.com --- These *.h files are part of a bundled engine source, not used for development or library reuse. They are loaded by the application as part of the engine logic and are not intended for -devel packaging.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Cristian Le fedora@lecris.me changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@lecris.me
--- Comment #9 from Cristian Le fedora@lecris.me --- Please use a single repo and point the spec file to the github raw links for easier review, e.g. https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi.spe...
There are quite a few issues here, and if you want it can be discussed more synchronously in the #devel chat room. - Source0 should point to the github source, unless it contains non-free files - Upstream is dead. If there are expected maintenance to be done, you better fork it, or research how other package ecosystems (e.g. Debian) are packaging this, if there are any - The summary and description could be better formulated - The project has a `setup.py`, please use it. See python packaging guidelines [1] - The header packaging is highly suspicious, let's revise it in light of the issue above - Python requires should be handled by the python buildsystem instead - There is some context that would be relevant to know regarding the relation between this project and `gnushogi`. Why would the user not run that directly? - The Version should point to the upstream's version. Release is used for downstream packaging only - The license is actually `GPL-3.0-or-later` according to the readme. Did you have anything else you were referencing that license from. We of course need to do a proper license review and find any potentially missing license
[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #10 from Akiyoshi Kurita akito5623@gmail.com --- Thanks for the detailed review, Cristian.
- I’ve consolidated everything into a single repository and updated the spec file to use GitHub raw sources for easier review: - Spec file (raw): https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi.spe... - SRPM: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.1...
- Source0 now points to the upstream GitHub tarball (tagged commit). - Switched to pyproject macros as recommended by the Python packaging guidelines. - Fixed Version/Release fields to match the upstream. - License is now set to GPL-3.0-or-later according to upstream’s LICENSE file. - Improved the Summary and Description as suggested. - Clarified the relationship with gnushogi: gshogi is a GTK front-end for the gnushogi engine; it provides a user-friendly GUI in contrast to the CLI provided by gnushogi itself. (Note: Debian only packages the engine, not this GUI. I’ve kept `Requires: gnushogi` accordingly.)
If you have further suggestions, I’m happy to join the #devel chat for a synchronous discussion.
Thank you for your time and help!
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #11 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103713 --> https://bugzilla.redhat.com/attachment.cgi?id=2103713&action=edit The .spec file difference from Copr build 9411248 to 9425724
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged
--- Comment #12 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9425724 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Cristian Le fedora@lecris.me changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fedora@lecris.me
--- Comment #13 from Cristian Le fedora@lecris.me --- Great, now to iron out more details of the review: - Please use explicitly the format ``` Spec URL: ... SRPM URL: ... ``` This is used both by fedora-review bot, and other post-review automations. You can freely add any other lines before and after - Are you already a packager or do you need sponsorship? If the latter, add `FE-NEEDSPONSOR` to the blocks of this bug - If my bash-foo doesn't fail me, the ` || :` means that you are ignoring the return code of the commands. Please let it run and fail if needed, especially in order to detect changes later on - The project version in source [1] and the release is 0.5.1, please also use that - If you are going to use a commit snapshot, see the snapshot guideline [2]. Forgemeta can automate this, e.g. here is one of mine that uses it [3] (%forgesource0 and %forgeautosetup is also relevant there) - Consider using autospec macros (%autorelease and %autochangelog). It is not a blocking issue because it is not a requirement and some people do not like using it, but it is a very useful approach especially for others contributing PRs to the dist-git repo - The line `# Remove any wrongly-installed paths under sitearch/usr (happens with old setuptools)` would need more context (here in the review is enough). What files are in there? If relevant, preferably change the `setup.py` to install to `/usr` instead of `usr` and it should land in the correct spot? This part [4] suggests that it should already install in the correct location though. - Please use `%pyproject_save_files`, and try to add `-l` to it. If it works, then you can remove the `%license` from `%files` since it would be done automatically. `%{python3_sitearch}/...` etc. is already covered by `%pyproject_save_files` - The `BuildRequires: py3_dist` should already be handled by the `%pyproject_buildrequires` - The project seems to be bundling and have some changes on top of the gnushogi engine right? In that case you should add a `Provides: bundled(gnushogi)`. If it doesn't call the `gnushogi` cli (which I didn't see in the repo?), you can remove the the `Requires: gnushogi` - %description lines must be wrapped to 80 character length - Can you add a `%pyproject_check_import` to the `%check` - Do you know how the `/locale` folder is used? - Can you remove `gshogi/data/opening.bbk` and create it manually with `create_book.py` and similarly for the other generated files? Given that this project is effectively dead, we can use the pre-compiled ones though, after checking if there is any significant difference if we run them manually now. But if this project is resurrected, you should be re-generating all files manually. - Feel free to ping me (@lecris:matrix.org) in the #devel chat room if you have quick questions and such
That is all I have for now. I still need to run the licensecheck to double check that everything is alright, but at a glance I do not see anything problematic. I assume that all the images are public domain and not copyrighted.
[1]: https://github.com/johncheetham/gshogi/blob/7c4bd90199c5bda61984347ea68c3252... [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snaps... [3]: https://src.fedoraproject.org/rpms/span/blob/rawhide/f/span.spec#_3-11 [4]: https://github.com/johncheetham/gshogi/blob/7c4bd90199c5bda61984347ea68c3252...
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #14 from Akiyoshi Kurita akito5623@gmail.com --- Thank you for your review and continued guidance.
I've updated the package as per your previous review:
Spec URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi.spe... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
The spec file now addresses all recent review comments.
Version/Release are set to 0.5.1-1 (upstream version).
Switched to %pyproject_save_files and related macros as suggested.
Removed unnecessary manual path cleanup.
Description and summary are reformatted to fit guidelines.
Documented image license status and prebuilt/generated data files in comments.
All build and install steps verified; COPR/mock build succeeded.
If there are any remaining issues, please let me know. Thank you for your continued feedback and review.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #15 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103733 --> https://bugzilla.redhat.com/attachment.cgi?id=2103733&action=edit The .spec file difference from Copr build 9425724 to 9434128
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #16 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9434128 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #17 from Cristian Le fedora@lecris.me --- Could you go over the list one more time, and check if you pushed the latest version? There is a discrepancy between your comment and the state of the spec file you've linked?
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #18 from Akiyoshi Kurita akito5623@gmail.com --- Thank you very much for your detailed review and guidance throughout this process.
I have carefully addressed all of your feedback and incorporated the suggested changes into the current spec and SRPM:
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
If you notice any further issues or have additional suggestions, please let me know—I am happy to continue improving the package as needed.
Otherwise, I believe the package is now ready for approval.
Thank you again for your time, support, and helpful review comments.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #19 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103807 --> https://bugzilla.redhat.com/attachment.cgi?id=2103807&action=edit The .spec file difference from Copr build 9434128 to 9434735
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #20 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9434735 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #21 from Akiyoshi Kurita akito5623@gmail.com --- Thank you for reviewing the package.
I proactively updated the spec file and SRPM to address all earlier feedback — even before receiving further review comments. Here are the current sources:
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
Please let me know if there are any outstanding issues. Otherwise, this package should now be ready for final approval.
Thank you for your continued guidance!
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #22 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103810 --> https://bugzilla.redhat.com/attachment.cgi?id=2103810&action=edit The .spec file difference from Copr build 9434735 to 9435055
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #23 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9435055 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #24 from Akiyoshi Kurita akito5623@gmail.com --- Thank you very much for your continued review and patience.
I apologize for submitting the package for review multiple times. I have carefully reviewed and fixed all mistakes by myself, and have addressed issues raised by the automatic tests before the reviewer had to point them out.
The current spec and SRPM now reflect all review feedback and pass all automated checks.
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main...
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
If you notice any further issues, please let me know. Thank you again for your time and guidance.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #25 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103813 --> https://bugzilla.redhat.com/attachment.cgi?id=2103813&action=edit The .spec file difference from Copr build 9435055 to 9435194
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #26 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9435194 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #27 from Akiyoshi Kurita akito5623@gmail.com --- Thank you again for your patience and for reviewing my package. I apologize for the repeated submissions.
I have now thoroughly reviewed everything myself and made all necessary corrections in accordance with the Fedora packaging guidelines. If you have any further comments or suggestions, I will address them promptly.
Here are the updated files:
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
Thank you very much for your continued support and guidance.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #28 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2103930 --> https://bugzilla.redhat.com/attachment.cgi?id=2103930&action=edit The .spec file difference from Copr build 9435194 to 9437642
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #29 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9437642 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #30 from Akiyoshi Kurita akito5623@gmail.com --- Thank you for asking. I am a new contributor, so I will need sponsorship. I would be grateful if you could sponsor me.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Cristian Le fedora@lecris.me changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
--- Comment #31 from Cristian Le fedora@lecris.me --- I cannot sponsor, but I'll make sure you have a ticket. You can post in devel chat room afterwards to see if someone is available. But right now let's focus on the review and getting a hang of the process.
There are a few points from the original comment that were not addressed. Not necessarily as changes in the spec file, but an ack and saying that you prefer a different way if it is the case for the non-blocking comments.
- Consider using autospec macros
Unclear which way you want to go with this. Note that if you want to go with manual changelog, the current changelog is meaningless and will have to be recreated at import.
- The line `# Remove any wrongly-installed paths under sitearch/usr (happens with old setuptools)` would need more context
Different comment message now, but it is still unresolved. I have edited locally to confirm that they are indeed written to site-packages (which is weird because `sys.prefix` expands to `/usr`. But I see that there are more relevant reasons for removing those, namely that you are editing them and installing in different location. I suggest you can either change the comment to reflect how you are editing and installing in different location or you patch it out of the `setup.py` (remove the `data_files` kw from `setup` part)
- If you are going to use a commit snapshot, see the snapshot guideline
Not fully addressed, the version format should be `<version>^<snapshot_details>`, up to your choice from the guideline if you want to do it manually, or let forgemeta handle it behind the scenes
---
Some other comments
- The `%pyproject_save_files -l` works, so you can remove `%license LICENSE` since there is already a file bundled in there (`/usr/lib64/python*/site-packages/gshogi-*.dist-info/licenses`) - Why is `python3-gobject` and `gtk3` a build dependency? I assume these are for compilation, but then it should be `gtk3-devel` and not `gtk3`, but it seems to have compiled without it. For the python3-gobject, it should be added in the setup.py if it is really needed - I have noticed that they are doing some weird stuff in `ez_setup.pay`, please remove that file and the reference in `setup.py` - Non-blocker: Using the full git commit in the source reference is an eye-sore, you can truncate it to the first 7 characters - At some point you removed the `desktop-file-validate`, please add that back in
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #32 from Akiyoshi Kurita akito5623@gmail.com --- Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
This update addresses all previous review comments:
- The version and release naming now follows the snapshot guideline: 0.5.1^20250818.git7c4bd90-1. - The spec uses %pyproject_save_files -l, and the redundant %license line was removed. - Removed all ez_setup related code and data_files from setup.py. - Changed BuildRequires to use gtk3-devel instead of gtk3, and python3-gobject as required. - desktop-file-validate is now run at install. - The single-quoted RPM file names are display-only (no functional impact, safe to use). - All other suggestions and packaging best practices are incorporated.
If you notice any outstanding issues, please let me know. Thank you again for your thorough review and guidance.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #33 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2104038 --> https://bugzilla.redhat.com/attachment.cgi?id=2104038&action=edit The .spec file difference from Copr build 9437642 to 9440509
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #34 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9440509 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #35 from Cristian Le fedora@lecris.me ---
- desktop-file-validate is now run at install.
Please run it in %check like before
- The single-quoted RPM file names are display-only (no functional impact, safe to use).
What is this a reference to?
- Removed all ez_setup related code and data_files from setup.py.
Also `ez_setup.py` file itself. But I would encourage using a patch instead of sed. The latter can silently fail and is not as readable. Here is an example workflow for how to make patches ``` $ tar -xf /gshogi-*.tar.gz $ cp -r gshogi-* gshogi-*-new $ vi ghsogi-*-new/setup.py ... do your edits $ diff -ru gshogi-* gshogi-*-new > gshogi-my_patch.patch $ vi gshogi.spec ... Add it to `Patch:` and make sure you have `%autosetup` or equivalent to pull it in ```
- Changed BuildRequires to use gtk3-devel instead of gtk3, and python3-gobject as required.
Using ``` BuildRequires: pyproject-rpm-macros ``` is not supported. Please use `%pyproject_buildrequires`. If you have issues with it, let me know, there are a few modes it can operate.
But as previously mentioned, it seemed to have built fine without `BuildRequires: gtk3-devel`. It is fairly odd, but I didn't dig into the sources to understand why. At least make the `BuildRequires` and `Requires` as minimal as it can be and comment on the manual additions.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #36 from Akiyoshi Kurita akito5623@gmail.com --- Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
Dear Cristian Le,
Thank you very much for reviewing my package so many times with such care. As a beginner, I truly appreciate the time you have taken to provide me with detailed feedback. Thanks to your guidance, the quality of this RPM package has undoubtedly improved. I am sorry for repeatedly taking your valuable time, but I am sincerely grateful for your support.
Once again, I would like to express my gratitude and kindly ask for your review again.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #37 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2104056 --> https://bugzilla.redhat.com/attachment.cgi?id=2104056&action=edit The .spec file difference from Copr build 9440509 to 9441927
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords|AutomationTriaged |
--- Comment #38 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9441927 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #39 from Akiyoshi Kurita akito5623@gmail.com --- [fedora-review-service-build]
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
(Please run a build-only test; not a final review yet.)
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #40 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9441986 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #41 from Akiyoshi Kurita akito5623@gmail.com --- Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
Dear Cristian Le,
First of all, I would like to thank you again for your continuous and patient reviews. As a beginner, I have truly benefited from your detailed feedback, and the quality of this RPM package has undoubtedly improved thanks to your guidance. I really appreciate the time and effort you have dedicated to helping me.
I have prepared the updated Spec and SRPM above. The Copr build succeeded successfully: https://copr.fedorainfracloud.org/coprs/build/9441986
The only reported issue is an “Upstream MD5sum check error”, which is a known false positive with GitHub auto-generated tarballs (their compression can vary, causing checksum mismatches even for the same commit). This is expected and not an actual issue with the package.
I kindly ask for your review once again at your convenience. Thank you very much for your time and support.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #42 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9441991 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #43 from Akiyoshi Kurita akito5623@gmail.com --- [fedora-review-service-build]
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
(Please run a build-only test; not a final review yet.)
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #44 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9442066 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #45 from Akiyoshi Kurita akito5623@gmail.com --- Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
Dear Cristian Le,
First of all, let me sincerely thank you again for your continuous and patient guidance throughout this review process. As a beginner, I have greatly benefited from your detailed feedback, and it is clear that the quality of this package has improved thanks to your reviews.
I have now updated the Spec and SRPM according to all your previous comments. The Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/9442066
The only reported issue is the well-known *"Upstream MD5sum check error"* related to GitHub auto-generated tarballs, which is expected and not a real problem. All other previous review comments have been addressed, and rpmlint now only reports the non-blocking warning about the missing man page (which should be acceptable for a GUI frontend like this).
At this point, I believe all issues have been resolved, but I kindly ask for your final review and confirmation.
Thank you very much again for your time, effort, and patience in helping me improve this package.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #46 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9442236 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #47 from Akiyoshi Kurita akito5623@gmail.com --- Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5...
Dear Cristian,
Thank you again for your continued guidance. I’ve gone through your notes (esp. Comment #35) and addressed them. Summary of changes and current status:
• desktop-file-validate: moved back to %check as requested. ✅ • sed vs patch: replaced sed with a proper patch (Patch0: 0001-drop-data_files-and_ez_setup.patch) that removes ez_setup.py and data_files usage from setup.py. ✅ • %pyproject_buildrequires: now using %pyproject_buildrequires -r to resolve build requirements from pyproject/setup definitions. ✅ • %pyproject_save_files -l: in use; license files are captured via dist-info. ✅ • %pyproject_check_import: added `%pyproject_check_import gshogi` to %check. ✅ • Snapshot versioning: using the recommended snapshot format `0.5.1^20250818.git7c4bd90`. ✅ • Provides: bundled(gnushogi): present, as the engine sources are bundled. ✅ • desktop-file-validate during install: no longer done there; it only runs in %check. ✅
Build deps minimization: – I kept the build-time dependencies minimal and treated GI/GTK as runtime deps. If you prefer further trimming (e.g., dropping gtk3-devel if superfluous on your side), I’m happy to adjust immediately. (Non-blocking)
Copr builds succeed. The only recurring note is the known “Upstream MD5sum check error” with GitHub auto-generated tarballs, which is a false positive per Fedora packaging guidelines discussion around SourceURL/tarball reproducibility.
Blocks already includes FE-NEEDSPONSOR as I’m a new contributor. If anything else needs tweaking, I’ll fix it promptly. Many thanks again for your time and review.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #48 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2104195 --> https://bugzilla.redhat.com/attachment.cgi?id=2104195&action=edit The .spec file difference from Copr build 9442236 to 9450887
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #49 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9450887 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #50 from Cristian Le fedora@lecris.me --- Only have 3 more comments:
Copr builds succeed. The only recurring note is the known “Upstream MD5sum check error” with GitHub auto-generated tarballs, which is a false positive per Fedora packaging guidelines discussion around SourceURL/tarball reproducibility.
This does not apply here, it is a genuine issue, I just haven't looked into what exactly causes the difference in your case. If you want to make sure it is clean, build the srpm from mock, various ways of doing so ``` $ fbrnch srpm $ mock --buildsrpm $ fedpkg srpm --srpm-mock ``` Some of these also do the download which would make sure the Source0 is also consistent.
– I kept the build-time dependencies minimal and treated GI/GTK as runtime deps. If you prefer further trimming (e.g., dropping gtk3-devel if superfluous on your side), I’m happy to adjust immediately. (Non-blocking)
Reducing build-time dependencies is important. We have automations like Koschei and manual rebuilds whenever a dependency is updated. Reducing the dependencies reduces the maintenance overhead of other packagers if the project becomes FTBFS.
---
I have tried to run this locally in a clean container and there are missing runtime dependencies. These are not detected automatically because they are not linked at build-time, instead they are loaded dynamically by the python modules. You need to add these dependencies as runtime dependencies, as manual `Requires` is acceptable. Upstream documented these to be `python3-cairo`, `python3-gobject` which do not seem to have PyPI counterparts which I guess is why these are not added to the `setup.py`. I did try to install those in the container, but it still didn't run, but I don't know if it is due to it being in a container.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #51 from Akiyoshi Kurita akito5623@gmail.com --- Thank you for your detailed review and guidance.
I have addressed all the feedback and the package now builds successfully. The new spec file and SRPM are attached.
They are also available for review at the following URLs: Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/main/gshogi.spe... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E202508...
Summary of changes: - Added the missing runtime `Requires` (`python3-gobject`, `python3-cairo`). - Added all necessary `BuildRequires` to pass the `%check` phase. - Corrected the `%license` file path. - Verified `Source0` consistency by building the SRPM successfully with `fedpkg srpm --srpm-mock`.
Please take another look. Thanks again for your help!
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #52 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2104213 --> https://bugzilla.redhat.com/attachment.cgi?id=2104213&action=edit The .spec file difference from Copr build 9450887 to 9451233
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #53 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9451233 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/gshogi/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #54 from Cristian Le fedora@lecris.me --- Be honest, are you using any LLM tools? You keep mentioning changes that are not related, and missing the points of what I write? Why did you suddenly add `%license LICENSE` even though I haven't mentioned it anyhwere.
- Verified `Source0` consistency by building the SRPM successfully with `fedpkg srpm --srpm-mock`.
Have you? The tarball in the srpm is very much different from the original tarball.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #55 from Cristian Le fedora@lecris.me --- Created attachment 2104214 --> https://bugzilla.redhat.com/attachment.cgi?id=2104214&action=edit pkgdiff report: SRPM vs original
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #56 from Akiyoshi Kurita akito5623@gmail.com --- Thanks for the feedback. For transparency, I use an AI assistant only for translation and drafting; all technical decisions and validations are my own. I have not yet performed the Source0/SRPM reproducibility check. I will run it tomorrow and attach the SHA256 results and mock logs. To keep the diff focused, I’ll avoid unrelated changes going forward.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #57 from Akiyoshi Kurita akito5623@gmail.com --- I have uploaded all the files necessary to verify SRPM source reproducibility:
- Upstream tarball with md5sum: `8a96995c92222e86d0dba249fc028f50` - SRPM-included tarball built with `fedpkg srpm --srpm-mock` has the same md5sum. - Full mock build logs are available under the `logs/` folder.
Repository link: https://github.com/redadmin-k/gshogi-fedora44/tree/main
If any additional information is required, I'll provide it promptly.
Thank you very much for your detailed reviews and kind guidance throughout this process. This is my very first time submitting an RPM package to Fedora. I have been learning as I go, researching documentation, and running each command step by step. Your patient explanations and advice about Fedora RPM packaging have been incredibly helpful, and I sincerely appreciate the time and care you have put into guiding me.
I will continue to address all review points in sequence and update the package accordingly. Thank you again for your support and for sharing your expertise.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #58 from Akiyoshi Kurita akito5623@gmail.com --- Hi Cristian-san
Just a gentle follow-up on this review request.
I provided the SRPM source reproducibility verification in Comment 57. Could you please let me know if there is anything else needed from my side to proceed?
Thank you very much for your time and guidance.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #59 from Cristian Le fedora@lecris.me --- Regarding the SRPM, it is indeed bundling the correct Source0, but as you might have noticed, the build is not successful. The logs seem to indicate that the patch was not packaged in the SRPM, but I am able to see it in there (tip, your desktop archive program should be able to open it and you would be able to navigate through it, at least for me ark does that nicely). I suspect the logs are outdated.
But even so, looking at the patch, it would not apply successfully because it does not appear to be created against the correct initial files [1]. Recommendation is to recreate the patch from scratch again. Normally I would also recommend not to do the `rm ez_setup.py` inside the patch because this is a persistent change that you would carry, and instead have it inside the spec file, but not a blocker for this case because the upstream is dead.
Regarding the AI discussion, I would hope that drafting there would be word drafting and not code/implementation drafting, in which case, it would be fine, although we are perfectly fine with your natural speech. There are plenty of contributors with varying English capabilities and I find that it is more easy to pick up on what someone was truly trying to say when there is no transcriber in the middle.
And again, feel free to just message me directly in the matrix room or anyone in the devel chatroom when you are stuck or don't quite understand something. We are more than happy to chime in and help on small issues. Package reviews take a bit more effort to navigate what was changed, what's the status, etc., quick messages in the chatroom on the other hand is much easier to hop in and out as we randomly find the time for it.
[1]: https://github.com/redadmin-k/gshogi-fedora44/blob/610c207fbfb73b7d02ffa1faa...
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #60 from Akiyoshi Kurita akito5623@gmail.com --- Dear Cristian-san
Thank you for your continued guidance and for taking the time to review my package in detail.
I have carefully investigated your recent comments regarding the patch application. To address your concerns, I performed the following steps to verify everything from scratch:
1 Extracted the SRPM's Source0 tarball.
2 Applied the included patch (0001-drop-data_files-and-ez_setup.patch) directly to the extracted Source0.
3 The patch applied successfully and the build completed without errors.
I have also checked that the latest logs, SRPM, and patch are fully in sync (the patch was regenerated from the current upstream Source0). If you are seeing issues, please let me know the exact commands and files you are using—I will promptly reproduce and fix any remaining discrepancy.
Thank you again for your patient review and for helping me improve the package quality.
Best regards, Akiyoshi Kurita
You can also review the build logs here: https://github.com/redadmin-k/gshogi-fedora44/tree/main/logs
They are also available for review at the following URLs: Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/main/gshogi.spe... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E202508...
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #61 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2105422 --> https://bugzilla.redhat.com/attachment.cgi?id=2105422&action=edit The .spec file difference from Copr build 9451233 to 9509759
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged
--- Comment #62 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9509759 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #63 from Cristian Le fedora@lecris.me --- The patch looks better now. As a tip, keep the patch difference as minimal as possible so that it can be easily rebased (doesn't apply here, so do whatever).
In your spec file you have
# No need to delete ez_setup.py, build/, dist/, *.egg-info here, # because the tarball is already cleaned.
Where did that come from? And I really mean, that I would like to know your thought process on this so we can find ways to better ways to guide you through typical packaging process and pitfalls you would encounter.
Tarball cleaning is a **very** specialized tool and you **must** provide the script you have used to do the cleaning. This **should** only be done if there is really no other way around it, primarily free license violations. This does not apply here so **do not do it**. Furthermore the cleaning mentioned in the comment did not occur, `ez_setup.py` is still there.
To address your concerns, I performed the following steps to verify everything from scratch:
1 Extracted the SRPM's Source0 tarball.
I did not ask you to do that, did I? If there is any part that were unclear in my comments **PLEASE TELL ME** so I can better formulate and make them clearer. If you feel more comfortable communicating in Japanese, let me know, my Japanese is nowhere near a native level, but I can understand it to some extents and I understand the intricacies of the grammar especially with respect to translations. This bugzilla bug would not be the best part to do so though because the official language is English here, but PM me on matrix and we can continue there however you feel more comfortable.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #64 from Akiyoshi Kurita akito5623@gmail.com --- Hi Cristian-san Thanks for the review and the guidance. On the “tarball cleaning” comment:
That was my mistake. I did not perform any tarball cleaning, and that comment was misleading. I’ve removed it from the spec. I also understand that tarball cleaning is a very specialized process that requires a published script and a strong justification (e.g. license issues), which does not apply here. I will not do that. What I changed and why (minimal patch):
I kept the patch as small as possible and limited it to setup.py only: Drop import ez_setup and the ez_setup.use_setuptools() call (they’re obsolete and unnecessary for pyproject/setuptools builds). Replace the dynamic data_files=[ … ] block with data_files=[] so that files are not installed outside the wheel. The .desktop file and icon are installed from the spec in %install, which is the distro-preferred way. I did not remove ez_setup.py from the source tree; it stays in the tarball, but it’s no longer referenced by setup.py. Spec updates:
Removed the misleading comment about tarball cleaning. Kept %autosetup -p1 -n gshogi-%{commit} and pyproject macros. Continue to install the .desktop and icon via %install. If anything still looks off, I’m happy to adjust further. Thank you again for the detailed feedback! Best regards,
Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #65 from Akiyoshi Kurita akito5623@gmail.com --- Hi Cristian-san
Thank you again for your detailed reviews and guidance.
I have submitted the updated spec file and SRPM, which remove the misleading comment as you requested.
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/main/gshogi.spe... SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E202508... Build logs: https://github.com/redadmin-k/gshogi-fedora44/tree/main/logs
Please take another look when you have a moment.
Best regards,
Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #66 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2105469 --> https://bugzilla.redhat.com/attachment.cgi?id=2105469&action=edit The .spec file difference from Copr build 9509759 to 9512006
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #67 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9512006 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #68 from Akiyoshi Kurita akito5623@gmail.com --- I encountered an error during the build, so I will update and resubmit the package after correcting it.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #69 from Cristian Le fedora@lecris.me ---
I also understand that tarball cleaning is a very specialized process that requires a published script and a strong justification (e.g. license issues), which does not apply here. I will not do that.
Source0 still references an arbitrary tarball. Revert it back to pointing to the github tar ball
I did not remove ez_setup.py from the source tree; it stays in the tarball, but it’s no longer referenced by setup.py.
Remove it. Specifically remove it by adding `rm ez_setup.py` in the `%prep` section.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #70 from Akiyoshi Kurita akito5623@gmail.com --- Hi Cristian-san,
Thank you very much for your repeated, detailed guidance.
Changes made in this update:
・Set Source0 to point directly to the upstream GitHub tarball, as requested. ・Added rm ez_setup.py to the %prep section to explicitly remove this obsolete file, following your instruction.
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/main/gshogi.spe...
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E202508...
Build logs: https://github.com/redadmin-k/gshogi-fedora44/tree/main/logs
I would appreciate it if you could review the changes at your convenience.
Thank you again for your kind support.
Best regards, Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #71 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2105484 --> https://bugzilla.redhat.com/attachment.cgi?id=2105484&action=edit The .spec file difference from Copr build 9512006 to 9512251
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #72 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/9512251 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
Cristian Le fedora@lecris.me changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+
--- Comment #73 from Cristian Le fedora@lecris.me --- There are still some comments, but at this point they are non-blockers and you can deal with them in your own time. Probably your sponsor will have some different opinions on these though. - %license file is unnecessary as I've mentioned before - gobject-introspection was added as BuildRequires because of %check, but it is not included in Requires despite the issue that the project is missing dependencies being specified - hicolor-icon-theme Requires dependency is needed because of directory ownership. These should be documented
Next steps, is to find a sponsor and discuss with them on the next steps. I would recommend to continue to get familiar with the packaging process, throughout this process there have been quite a lot of back-and-forth, and it would be good to show that you have learned from this process and that you have at least internalized the core principles of a package review. I recommend you do some non-binding package reviews [1] and let your sponsor know about them (can CC me in those also). (The review back-and-forth should definitely not be this long)
[1]: Can find some from https://fedoraproject.org/PackageReviewStatus/
---
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [-]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Package does not contain any libtool archives (.la) [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs.
Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* GNU General Public License, Version 3", "*No copyright* GNU General Public License", "GNU General Public License v1.0 or later and/or GNU General Public License v3.0 or later [obsolete FSF postal address (Mass Ave)]", "*No copyright* GNU General Public License v3.0 or later", "GNU General Public License v3.0 or later". 107 files have unknown license. Detailed output of licensecheck in /var/lib/copr- rpmbuild/results/gshogi/licensecheck.txt [x]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib64/python3.14/site- packages, /usr/lib64/python3.14 (This note is odd, these are owned by python3.14 afaict) [-]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [-]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [-]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: The License field must be a valid SPDX expression. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 8666 bytes in 1 files. [x]: Packages must not store files under /srv, /opt or /usr/local
Python: [x]: Python eggs must not download any dependencies during the build process. [-]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Packages MUST NOT have dependencies (either build-time or runtime) on packages named with the unversioned python- prefix unless no properly versioned package exists. Dependencies on Python packages instead MUST use names beginning with python2- or python3- as appropriate. [x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files [x]: Binary eggs must be removed in %prep
===== SHOULD items =====
Generic: [!]: Reviewer should test that the package builds in mock. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Patches link to upstream bugs/comments/lists or are otherwise justified. (Upstream is archived) [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [?]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Large data in /usr/share should live in a noarch subpackage if package is arched.
Rpmlint ------- Checking: gshogi-0.5.1^20250818.git7c4bd90-7.fc44.x86_64.rpm gshogi-0.5.1^20250818.git7c4bd90-7.fc44.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.7.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpweo36vip')] checks: 32, packages: 2
gshogi.src: W: strange-permission 0001-drop-data_files-and-ez_setup.patch 666 gshogi.src: W: strange-permission gshogi-7c4bd90199c5bda61984347ea68c32521e82a637.tar.gz 666 gshogi.src: W: strange-permission gshogi.spec 666 gshogi.x86_64: W: no-manual-page-for-binary gshogi gshogi.spec:12: W: mixed-use-of-spaces-and-tabs (spaces: line 5, tab: line 12) 2 packages and 0 specfiles checked; 0 errors, 5 warnings, 8 filtered, 0 badness; has taken 0.5 s
Rpmlint (debuginfo) ------------------- Checking: gshogi-debuginfo-0.5.1^20250818.git7c4bd90-7.fc44.x86_64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.7.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpan5t65cf')] checks: 32, packages: 1
1 packages and 0 specfiles checked; 0 errors, 0 warnings, 5 filtered, 0 badness; has taken 0.1 s
Rpmlint (installed packages) ---------------------------- (none): E: there is no installed rpm "gshogi-debuginfo". ============================ rpmlint session starts ============================ rpmlint: 2.7.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 2
0 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 filtered, 0 badness; has taken 0.0 s (none): E: there is no installed rpm "gshogi". There are no files to process nor additional arguments. Nothing to do, aborting.
Unversioned so-files -------------------- gshogi: /usr/lib64/python3.14/site-packages/gshogi/engine.cpython-314-x86_64-linux-gnu.so
Source checksums ---------------- https://github.com/johncheetham/gshogi/archive/7c4bd90199c5bda61984347ea68c3... : CHECKSUM(SHA256) this package : bc5017c1aa2e6411bf86daca6219ca0be063e1e16e5be74d8f0f4a2237da9f77 CHECKSUM(SHA256) upstream package : bc5017c1aa2e6411bf86daca6219ca0be063e1e16e5be74d8f0f4a2237da9f77
Requires -------- gshogi (rpmlib, GLIBC filtered): /usr/bin/python3 hicolor-icon-theme libc.so.6()(64bit) python(abi) python3-cairo python3-gobject rtld(GNU_HASH)
Provides -------- gshogi: application() application(gshogi.desktop) bundled(gnushogi) gshogi gshogi(x86-64) python3.14dist(gshogi) python3dist(gshogi)
Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/bin/fedora-review --no-colors --prebuilt --rpm-spec --name gshogi --mock-config /var/lib/copr-rpmbuild/results/configs/child.cfg Buildroot used: fedora-rawhide-x86_64 Active plugins: C/C++, Generic, Shell-api, Python Disabled plugins: Haskell, SugarActivity, Java, R, PHP, fonts, Ocaml, Perl Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #74 from Akiyoshi Kurita akito5623@gmail.com --- Hi Cristian-san,
Thank you very much for your thorough reviews, detailed feedback, and continuous support throughout this package review process. I have learned a lot from your guidance and the constructive comments you provided. I will take your suggestions into account for future improvements and discuss them further with my sponsor.
Thank you again for your patience and for helping me understand the Fedora packaging process better.
Best regards,
Akiyoshi Kurita
https://bugzilla.redhat.com/show_bug.cgi?id=2388375
--- Comment #75 from Akiyoshi Kurita akito5623@gmail.com --- 個人的な事情により申請を取り下げます。これまでありがとうございました。 I would like to withdraw this request due to personal reasons. Thank you for your time and understanding.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com