Bug 2388375

Summary: Review Request: gshogi - GTK front-end for GNU Shogi
Product: [Fedora] Fedora Reporter: Akiyoshi Kurita <akito5623>
Component: Package ReviewAssignee: Cristian Le <fedora>
Status: NEW --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: fedora, package-review, rjones
Target Milestone: ---Keywords: AutomationTriaged
Target Release: ---Flags: fedora: fedora-review+
Hardware: Unspecified   
OS: Linux   
URL: https://github.com/johncheetham/gshogi.git
Whiteboard:
Fixed In Version: Doc Type: ---
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 177841    
Attachments:
Description Flags
gshogi - review bundle (SPEC, SRPM, Fedora 42 and Rawhide build logs)
none
The .spec file difference from Copr build 9411132 to 9411248
none
The .spec file difference from Copr build 9411248 to 9425724
none
The .spec file difference from Copr build 9425724 to 9434128
none
The .spec file difference from Copr build 9434128 to 9434735
none
The .spec file difference from Copr build 9434735 to 9435055
none
The .spec file difference from Copr build 9435055 to 9435194
none
The .spec file difference from Copr build 9435194 to 9437642
none
The .spec file difference from Copr build 9437642 to 9440509
none
The .spec file difference from Copr build 9440509 to 9441927
none
The .spec file difference from Copr build 9442236 to 9450887
none
The .spec file difference from Copr build 9450887 to 9451233
none
pkgdiff report: SRPM vs original
none
The .spec file difference from Copr build 9451233 to 9509759
none
The .spec file difference from Copr build 9509759 to 9512006
none
The .spec file difference from Copr build 9512006 to 9512251 none

Description Akiyoshi Kurita 2025-08-13 19:37:06 UTC
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.

Comment 1 Akiyoshi Kurita 2025-08-13 19:38:28 UTC
Created attachment 2103564 [details]
gshogi - review bundle (SPEC, SRPM, Fedora 42 and Rawhide build logs)

Comment 2 Fedora Review Service 2025-08-13 22:07:43 UTC
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.

Comment 4 Fedora Review Service 2025-08-14 00:22:02 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9411132
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09411132-gshogi/fedora-review/review.txt

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/LicensingGuidelines/#_license_text

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.

Comment 5 Akiyoshi Kurita 2025-08-14 01:58:12 UTC
SRPM and SPEC files for Fedora package review:

Fedora 42:
- SRPM: https://github.com/redadmin-k/gshogi-fedora42/releases/download/v0.0.9/gshogi-0-0.8.fc42.src.rpm
- SPEC: https://github.com/redadmin-k/gshogi-fedora42/releases/download/v0.0.9/gshogi.spec

Fedora 44:
- SRPM: https://github.com/redadmin-k/gshogi-fedora44/releases/download/v0.0.9/gshogi-0-0.8.fc44.src.rpm
- SPEC: https://github.com/redadmin-k/gshogi-fedora44/releases/download/v0.0.9/gshogi.spec

---

**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`.

Comment 6 Fedora Review Service 2025-08-14 02:03:56 UTC
Created attachment 2103603 [details]
The .spec file difference from Copr build 9411132 to 9411248

Comment 7 Fedora Review Service 2025-08-14 02:03:58 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9411248
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09411248-gshogi/fedora-review/review.txt

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.

Comment 8 Akiyoshi Kurita 2025-08-14 02:07:46 UTC
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.

Comment 9 Cristian Le 2025-08-15 07:54:03 UTC
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.spec

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/

Comment 10 Akiyoshi Kurita 2025-08-15 11:52:36 UTC
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.spec
  - SRPM: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.1-1.fc44.src.rpm

- 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!

Comment 11 Fedora Review Service 2025-08-15 12:04:15 UTC
Created attachment 2103713 [details]
The .spec file difference from Copr build 9411248 to 9425724

Comment 12 Fedora Review Service 2025-08-15 12:04:17 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9425724
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09425724-gshogi/fedora-review/review.txt

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.

Comment 13 Cristian Le 2025-08-15 12:42:47 UTC
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/7c4bd90199c5bda61984347ea68c32521e82a637/gshogi/constants.py#L21
[2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots
[3]: https://src.fedoraproject.org/rpms/span/blob/rawhide/f/span.spec#_3-11
[4]: https://github.com/johncheetham/gshogi/blob/7c4bd90199c5bda61984347ea68c32521e82a637/setup.py#L31-L33

Comment 14 Akiyoshi Kurita 2025-08-15 15:37:32 UTC
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.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1-1.fc44.src.rpm

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.

Comment 15 Fedora Review Service 2025-08-15 15:45:10 UTC
Created attachment 2103733 [details]
The .spec file difference from Copr build 9425724 to 9434128

Comment 16 Fedora Review Service 2025-08-15 15:45:12 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9434128
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09434128-gshogi/fedora-review/review.txt

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.

Comment 17 Cristian Le 2025-08-15 15:45:57 UTC
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?

Comment 18 Akiyoshi Kurita 2025-08-15 20:21:46 UTC
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/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1-1.fc44.src.rpm

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.

Comment 19 Fedora Review Service 2025-08-15 20:29:03 UTC
Created attachment 2103807 [details]
The .spec file difference from Copr build 9434128 to 9434735

Comment 20 Fedora Review Service 2025-08-15 20:29:05 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9434735
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09434735-gshogi/fedora-review/review.txt

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.

Comment 21 Akiyoshi Kurita 2025-08-15 23:35:39 UTC
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/gshogi.spec  
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1-1.fc44.src.rpm

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!

Comment 22 Fedora Review Service 2025-08-15 23:44:20 UTC
Created attachment 2103810 [details]
The .spec file difference from Copr build 9434735 to 9435055

Comment 23 Fedora Review Service 2025-08-15 23:44:23 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9435055
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09435055-gshogi/fedora-review/review.txt

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.

Comment 24 Akiyoshi Kurita 2025-08-16 01:36:43 UTC
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/gshogi.spec

SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1-1.fc44.src.rpm

If you notice any further issues, please let me know.
Thank you again for your time and guidance.

Comment 25 Fedora Review Service 2025-08-16 01:43:54 UTC
Created attachment 2103813 [details]
The .spec file difference from Copr build 9435055 to 9435194

Comment 26 Fedora Review Service 2025-08-16 01:43:56 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9435194
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09435194-gshogi/fedora-review/review.txt

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.

Comment 27 Akiyoshi Kurita 2025-08-17 05:55:48 UTC
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/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1-1.fc44.src.rpm

Thank you very much for your continued support and guidance.

Comment 28 Fedora Review Service 2025-08-17 06:03:17 UTC
Created attachment 2103930 [details]
The .spec file difference from Copr build 9435194 to 9437642

Comment 29 Fedora Review Service 2025-08-17 06:03:20 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9437642
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09437642-gshogi/fedora-review/review.txt

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.

Comment 30 Akiyoshi Kurita 2025-08-18 07:34:14 UTC
Thank you for asking. I am a new contributor, so I will need sponsorship. I would be grateful if you could sponsor me.

Comment 31 Cristian Le 2025-08-18 11:21:45 UTC
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

Comment 32 Akiyoshi Kurita 2025-08-18 13:50:55 UTC
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec  
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1%5E20250818.git7c4bd90-1.fc44.src.rpm

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.

Comment 33 Fedora Review Service 2025-08-18 14:00:44 UTC
Created attachment 2104038 [details]
The .spec file difference from Copr build 9437642 to 9440509

Comment 34 Fedora Review Service 2025-08-18 14:00:47 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9440509
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09440509-gshogi/fedora-review/review.txt

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.

Comment 35 Cristian Le 2025-08-18 15:51:39 UTC
> - 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.

Comment 36 Akiyoshi Kurita 2025-08-19 00:41:52 UTC
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1%5E20250818.git7c4bd90-2.fc44.src.rpm

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

Comment 37 Fedora Review Service 2025-08-19 00:48:29 UTC
Created attachment 2104056 [details]
The .spec file difference from Copr build 9440509 to 9441927

Comment 38 Fedora Review Service 2025-08-19 00:48:36 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9441927
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09441927-gshogi/fedora-review/review.txt

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.

Comment 39 Akiyoshi Kurita 2025-08-19 01:15:16 UTC
[fedora-review-service-build]

Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1%5E20250818.git7c4bd90-2.fc44.src.rpm

(Please run a build-only test; not a final review yet.)

Comment 40 Fedora Review Service 2025-08-19 01:22:01 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9441986
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09441986-gshogi/fedora-review/review.txt

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.

Comment 41 Akiyoshi Kurita 2025-08-19 01:26:52 UTC
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1%5E20250818.git7c4bd90-2.fc44.src.rpm

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

Comment 42 Fedora Review Service 2025-08-19 01:33:22 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9441991
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09441991-gshogi/fedora-review/review.txt

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.

Comment 43 Akiyoshi Kurita 2025-08-19 01:59:30 UTC
[fedora-review-service-build]

Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1%5E20250818.git7c4bd90-2.fc44.src.rpm

(Please run a build-only test; not a final review yet.)

Comment 44 Fedora Review Service 2025-08-19 02:06:35 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9442066
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09442066-gshogi/fedora-review/review.txt

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.

Comment 45 Akiyoshi Kurita 2025-08-19 02:11:58 UTC
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1%5E20250818.git7c4bd90-2.fc44.src.rpm

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

Comment 46 Fedora Review Service 2025-08-19 02:20:40 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9442236
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09442236-gshogi/fedora-review/review.txt

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.

Comment 47 Akiyoshi Kurita 2025-08-20 16:21:35 UTC
Spec URL: https://raw.githubusercontent.com/redadmin-k/gshogi-fedora44/refs/heads/main/gshogi.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/refs/heads/main/gshogi-0.5.1^20250818.git7c4bd90-3.fc44.src.rpm

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

Comment 48 Fedora Review Service 2025-08-20 16:28:20 UTC
Created attachment 2104195 [details]
The .spec file difference from Copr build 9442236 to 9450887

Comment 49 Fedora Review Service 2025-08-20 16:28:22 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9450887
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09450887-gshogi/fedora-review/review.txt

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.

Comment 50 Cristian Le 2025-08-20 17:33:37 UTC
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.

Comment 51 Akiyoshi Kurita 2025-08-20 19:16:59 UTC
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.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E20250818.git7c4bd90-7.fc44.src.rpm

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!

Comment 52 Fedora Review Service 2025-08-20 19:24:54 UTC
Created attachment 2104213 [details]
The .spec file difference from Copr build 9450887 to 9451233

Comment 53 Fedora Review Service 2025-08-20 19:24:56 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9451233
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09451233-gshogi/fedora-review/review.txt

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.

Comment 54 Cristian Le 2025-08-20 19:40:36 UTC
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.

Comment 55 Cristian Le 2025-08-20 19:42:25 UTC
Created attachment 2104214 [details]
pkgdiff report: SRPM vs original

Comment 56 Akiyoshi Kurita 2025-08-20 21:41:42 UTC
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.

Comment 57 Akiyoshi Kurita 2025-08-21 02:04:08 UTC
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

Comment 58 Akiyoshi Kurita 2025-08-28 12:51:00 UTC
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

Comment 59 Cristian Le 2025-08-29 08:26:10 UTC
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/610c207fbfb73b7d02ffa1faad88fcf358e2e508/0001-drop-data_files-and-ez_setup.patch#L442-L447

Comment 60 Akiyoshi Kurita 2025-08-31 05:58:37 UTC
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.spec 
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E20250818.git7c4bd90-7.fc44.src.rpm

Comment 61 Fedora Review Service 2025-08-31 23:17:55 UTC
Created attachment 2105422 [details]
The .spec file difference from Copr build 9451233 to 9509759

Comment 62 Fedora Review Service 2025-08-31 23:17:58 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9509759
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09509759-gshogi/fedora-review/review.txt

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.

Comment 63 Cristian Le 2025-09-01 09:33:09 UTC
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.

Comment 64 Akiyoshi Kurita 2025-09-01 09:54:34 UTC
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

Comment 65 Akiyoshi Kurita 2025-09-01 10:48:06 UTC
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.spec
SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E20250818.git7c4bd90-7.fc44.src.rpm
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

Comment 66 Fedora Review Service 2025-09-01 10:52:31 UTC
Created attachment 2105469 [details]
The .spec file difference from Copr build 9509759 to 9512006

Comment 67 Fedora Review Service 2025-09-01 10:52:34 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9512006
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09512006-gshogi/fedora-review/review.txt

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.

Comment 68 Akiyoshi Kurita 2025-09-01 10:59:37 UTC
I encountered an error during the build, so I will update and resubmit the package after correcting it.

Comment 69 Cristian Le 2025-09-01 11:00:07 UTC
> 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.

Comment 70 Akiyoshi Kurita 2025-09-01 12:04:51 UTC
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.spec

SRPM URL: https://github.com/redadmin-k/gshogi-fedora44/raw/main/gshogi-0.5.1%5E20250818.git7c4bd90-7.fc44.src.rpm

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

Comment 71 Fedora Review Service 2025-09-01 12:09:22 UTC
Created attachment 2105484 [details]
The .spec file difference from Copr build 9512006 to 9512251

Comment 72 Fedora Review Service 2025-09-01 12:09:24 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9512251
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2388375-gshogi/fedora-rawhide-x86_64/09512251-gshogi/fedora-review/review.txt

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.

Comment 73 Cristian Le 2025-09-01 14:20:11 UTC
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/7c4bd90199c5bda61984347ea68c32521e82a637/gshogi-7c4bd90199c5bda61984347ea68c32521e82a637.tar.gz :
  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

Comment 74 Akiyoshi Kurita 2025-09-01 14:50:47 UTC
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

Comment 75 Akiyoshi Kurita 2025-09-03 02:08:29 UTC
個人的な事情により申請を取り下げます。これまでありがとうございました。
I would like to withdraw this request due to personal reasons. Thank you for your time and understanding.