https://bugzilla.redhat.com/show_bug.cgi?id=2257770 --- Comment #3 from Jerry James <loganjerry@xxxxxxxxx> --- Thank you for the review, Kai! (In reply to Kai A. Hiller from comment #2) > I looked at the package, and it is mostly fine, with the biggest issue being > licensing. There are some files that are used in the build, but do not have > any > licensing information, namely: > > - breakidConfig.cmake.in > - cmake_uninstall.cmake.in > - breakid_c.cpp > - breakid_c.h > > You should try to talk with upstream to add the missing licensing > information. > If that is not successful, maybe working around those files is possible. Okay, I have requested that upstream add the licensing information. > Under the assumption the above files are MIT-licensed, I think the License > tag > should become MIT AND BSL-1.0. The BSL-1.0 in there comes from > GetGitRevisionDescription.cmake(.in). To keep the License tag simple and be > more > sure it’s correct, I‘d suggest removing files under different licenses that > are > not actually used in the build: > > rm -rf scripts > rm src/Makefile > rm cmake/{AddGTestSuite,AddSTPGTest,CheckFloatPrecision,Findcargo,FindM4RI,\ > FindPerftools,FindPkgMacros,Findrustc,Findrustdoc,FindSqlite3,FindTBB,\ > FindValgrind,Rust}.cmake None of these files are shipped in the binary RPMs. According to https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_field: "The License: field refers to the licenses of the contents of the *binary* rpm." So I don't think removing files or adding BSL-1.0 to the License field is necessary. > Then here are some suggestions I find sensible. They are optional, though, > and > you may choose to ignore them: > > - Spell out the patch file name in the spec file, because the %{name} macro > confuses rpmlint. That is an rpmlint bug: https://github.com/rpm-software-management/rpmlint/issues/1074. Hopefully rpmlint upstream will fix it at some point. > - Disable setting of the optimization level flag (-O2) in CMakeList.txt to > use > Fedora’s default instead. Okay, done. Things were getting complicated, so this is now a patch instead of a sed invocation. > - Fix setting the installation directory upstream instead of downstream in > the > spec file. Okay, I asked upstream to address this issue. > - Communicate with upstream and suggest cleaning up the many unnecessary > files. Yes, I think a lot of those came from cryptominisat. I think they just copied the cryptominisat cmake directory and then tweaked a few things. I'll suggest that to upstream. All of the upstream requests are here: https://github.com/meelgroup/breakid/issues/1. The spec file and SRPM have been updated, at the same URLs as above. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2257770 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202257770%23c3 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
