https://bugzilla.redhat.com/show_bug.cgi?id=976793 Otto Urpelainen <oturpe@xxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(jskarvad@redhat.c | |om) --- Comment #37 from Otto Urpelainen <oturpe@xxxxxx> --- Review taken, here is everything I could spot. 1. > # CMake/common/ittnotify.h is under GPLv2, the rest is LGPL I am not sure if it counts, since it is part of the build script only, and License field should describe the packaged content, not srpm. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_field 2. Looking at licensecheck.txt generated by fedora-review and disregarding the build script, it looks like the actual license is more like this: ( and Boost # lunchbox/atomic.h, lunchbox/any.h and LGPLv2 # most files and LGPLv3 # some files, like any.cpp and lfVector.h ) According to GPL Compatibility Matrix from Fedora Wiki [1] LGPLv2 and LGPLv3 mix only by "converting to GPLv3". I suppose the correct action would then be to mark the license as "Boost and GPLv3", note somewhere that the conversion option was used (I am not aware of specific instructions on how to do this) and include GPLv3 license text as %license. The Boost license is already included in ACKNOWLEDGEMENTS.txt, so that should be installed as %license. Additionally, lunchbox/test.h is under BSD. It is a test runner, contains its license in the file header and is not compiled during the build. As long as it goes to devel package or is simply not installed, its license is ok. Getting licensing right is messy as usual, there may well be something we have missed. Let us try to get as close to reality as possible, even if upstream situation is quite unclear. [1]: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#GPL_Compatibility_Matrix 3. > Provides: bundled(eyescale-cmake-common) Use versioned Provides. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling 4. > %{_libdir}/libLunchbox.so.1* Globbing that hides important parts of the shared object file name should not be used. Because the found suffixes here are 1.17.0 and 10.0.0, it would be better to select both of these separately. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_library_files 5. > Requires: pkgconfig > Requires: cmake I do not think these are needed. The build does not produce a pkgconfig file and CMake files are installed under self-owner directory %{_datadir}/Lunchbox 6. I agree Petr regarding the contents of /usr/share/Lunchbox, those should not go to the binary package. > /usr/share/Lunchbox/CMake/LunchboxConfig.cmake > /usr/share/Lunchbox/CMake/LunchboxConfigVersion.cmake > /usr/share/Lunchbox/CMake/LunchboxTargets-debug.cmake > /usr/share/Lunchbox/CMake/LunchboxTargets.cmake > /usr/share/Lunchbox/CMake/options.cmake I am not familiar enough with CMake builds in Fedora to say how cmake scripts should be packaged (the CMake guidelines should be extended to cover this) but assuming that the directory structure is acceptable these should be package in the -devel package. > /usr/share/Lunchbox/benchmarks/perf-lfVector > /usr/share/Lunchbox/benchmarks/perf-mutex > /usr/share/Lunchbox/benchmarks/perf-rwLock Test executables, should go to -devel if packaged at all. Also, binaries have no place in %{_datadir}, rpmlint complains about this. So if they are package in any package, they should go to %{_bindir}. > /usr/share/Lunchbox/tests/any.cpp > /usr/share/Lunchbox/tests/anySerialization.cpp > /usr/share/Lunchbox/tests/buffer.cpp > /usr/share/Lunchbox/tests/clock.cpp > /usr/share/Lunchbox/tests/debug.cpp > /usr/share/Lunchbox/tests/dso.cpp > /usr/share/Lunchbox/tests/file.cpp > /usr/share/Lunchbox/tests/future.cpp > /usr/share/Lunchbox/tests/init.cpp > /usr/share/Lunchbox/tests/issue1.cpp > /usr/share/Lunchbox/tests/lfQueue.cpp > /usr/share/Lunchbox/tests/memoryMap.cpp > /usr/share/Lunchbox/tests/monitor.cpp > /usr/share/Lunchbox/tests/mtQueue.cpp > /usr/share/Lunchbox/tests/perThread.cpp > /usr/share/Lunchbox/tests/perf/lfVector.cpp > /usr/share/Lunchbox/tests/perf/mutex.cpp > /usr/share/Lunchbox/tests/perf/rwLock.cpp > /usr/share/Lunchbox/tests/pluginFactory.cpp > /usr/share/Lunchbox/tests/refPtr.cpp > /usr/share/Lunchbox/tests/requestHandler.cpp > /usr/share/Lunchbox/tests/rng.cpp > /usr/share/Lunchbox/tests/thread.cpp > /usr/share/Lunchbox/tests/threadPool.cpp Test source files. I do not see any reason to package these, they are already available in srpm and it is not usual to package sources in -devel. 7. > $ rpm -q --requires ./review-lunchbox/results/lunchbox-1.17.0-3.fc35.x86_64.rpm > libboost_unit_test_framework.so.1.75.0()(64bit) As this package is not a test runner or similar tool there should be no unit test framework dependencies. Probably this is solved when all the test related files are moved to -devel or removed from installation. 8. I would consider removing directory pthreads in %prep as it contains a shady tarball apprently only needed for Windows builds. This is not blocking, because as far as I can tell, there is nothing that is strictly disallowed there. Just a general suggestion. 9. Man pages are absent. There is a SHOULD in the guidelines about trying to get them added. My understanding is that upstream does not really respond to queries so in my opinion, there is no need to take action. Of course it is better if you do ask upstream about it. Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages 10. > Rpmlint > ------- > Checking: lunchbox-1.17.0-3.fc35.x86_64.rpm > lunchbox-devel-1.17.0-3.fc35.x86_64.rpm > lunchbox-debuginfo-1.17.0-3.fc35.x86_64.rpm > lunchbox-debugsource-1.17.0-3.fc35.x86_64.rpm > lunchbox-1.17.0-3.fc35.src.rpm > lunchbox.x86_64: E: description-line-too-long C Lunchbox is C++ library for multi-threaded programming, providing OS abstraction, Please shorten it. -- 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 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
