https://bugzilla.redhat.com/show_bug.cgi?id=2468312
--- Comment #8 from Cristian Le <[email protected]> --- > I tried that, and got a changelog entry of > * Fri May 15 2026 John Doe <[email protected]> - 1.24.0-1.fc44 > - local build To give more context, that is fine and working as it's supposed to. The changelog after import should be sanitized anyway, and the changelog here doesn't make much difference, the fedora-review bot would help instead (seems to be offline, will check up with the folks). > There are some license issues. I scanned the project for any copyrights other > than Rony Shapiro, and found two. Main thing to do is notify upstream properly and ask them to track these. REUSE is an interesting project they can consider for this, otherwise just a simple note in the readme, COPYING, or similar is fine. Both upstream and downstream though should check if the licensed files are pulled in always or conditionally. > src/core/pugixml/pugixml.cpp - includes what seems to be MIT license at the > end of the cpp file. In %build I could copy that section of the cpp file into > something like pugixml.LICENSE and install that from %files. > > src/core/crypto/external/Chromium/LICENSE - seems to be BSD-3-Clause. > Upstream packaging does not install this. Since the filename is just > "LICENSE", I assume we should install it as Chromium/LICENSE or > Chromium.LICENSE Yes, install the appropriate files, preference with the `pugixml.LICENSE` or equivalent single-file format, but also include them in the `License` glued by `AND`, i.e. `Artistic-2.0 AND MIT AND BSD-3-Clause` with a breakdown of the sources (format is free-form, but check some examples [1,2]) of the external files and `Provides: bundled` [3]. Extracting the license header from pugixml.cpp seems hard, I would not recommend on that. See instead about getting it from upstream [4], or preferably, please reach out to upstream and ask them to use `FetchContent` approach to bundling that way we can get the whole repo files for the bundled content (or de-bundle) and it is easier for them to update, win-win. And more notes on the current state - Doing a `touch --no-create %{_datadir}/icons/hicolor` is incorrect. Instead you have to `Requires: hicolor-icon-theme`, see chatterino example. The other `%post*` are also unnecessary and if not, they should be done at build-level - Afaict `appstream-util validate-relax` is also a step that should be done at `%check` - See the locale guideline [5] for how to handle these. It is the first time for me to review these, so I am using that and other reference usages [6]. This is a review blocker because ``` passwordsafe.x86_64: W: file-not-in-%lang /usr/share/locale/da/LC_MESSAGES/pwsafe.mo ``` - Please de-glob the `%{_bindir}/pwsafe*`, and to that extent, what is the difference and relation between `pwsafe` and `pwsafe-cli`? - Some rpmlint output (should have been given by the fedora-review bot) ``` passwordsafe.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/passwordsafe-1.24.0/config.txt passwordsafe.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/passwordsafe-1.24.0/help.txt passwordsafe.x86_64: W: unused-direct-shlib-dependency /usr/bin/pwsafe-cli /lib64/libmagic.so.1 passwordsafe.x86_64: W: summary-ended-with-dot Password Safe is a password management utility. passwordsafe.x86_64: W: obsolete-not-provided pwsafe passwordsafe.x86_64: W: no-manual-page-for-binary pwsafe-cli passwordsafe.x86_64: W: file-not-in-%lang /usr/share/locale/da/LC_MESSAGES/pwsafe.mo ``` `obsolete-not-provided` and `no-manual-page-for-binary` issues are fine to ignore - Can you remove `src/ui/Windows` and `src/os/windows` in `%prep`. Those have more differently licensed files and should make sure these are not pulled in without updating the metadata. Otherwise, I think all would be lgtm. I will post the fedora-review checklist after these [1]: https://src.fedoraproject.org/rpms/chatterino2/blob/rawhide/f/chatterino2.spec#_61 [2]: https://src.fedoraproject.org/rpms/openmw/blob/rawhide/f/openmw.spec#_34 [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling [4]: https://github.com/zeux/pugixml [5]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#handling_locale_files [6]: https://sourcegraph.com/search?q=context:global+repo:src.fedoraproject.org+%25find_lang&patternType=keyword&sm=0 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2468312 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202468312%23c8 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected] Do not reply to spam, report it: https://forge.fedoraproject.org/infra/tickets/issues/new
