https://bugzilla.redhat.com/show_bug.cgi?id=976793 --- Comment #41 from Jaroslav Škarvada <jskarvad@xxxxxxxxxx> --- (In reply to Otto Urpelainen from comment #37) > 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 > Upstream ticket: https://github.com/Eyescale/Lunchbox/issues/331 The file is dual licensed, it seems upstream chose BSD and released the result under LGPL. > 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. > Didn't you think "Boost and LGPLv3"? I asked upstream about the status: https://github.com/Eyescale/Lunchbox/issues/331 > 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. > IMHO the resulting work can be released under LGPL. > 3. > > Provides: bundled(eyescale-cmake-common) > > Use versioned Provides. > Reference: > https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling > Fixed. > 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 > Hopefully fixed. > 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 > Dropped. > 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. > Hopefully fixed. > 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. > Moved to devel, hopefully OK. > 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. > Dropped > 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 > Just nice to have. > 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. > Reformatted. -- 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. _______________________________________________ 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
