https://bugzilla.redhat.com/show_bug.cgi?id=976793 Otto Urpelainen <oturpe@xxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(jskarvad@redhat.c | |om) --- Comment #43 from Otto Urpelainen <oturpe@xxxxxx> --- (In reply to Jaroslav Škarvada from comment #41) Thank you, Apart from the license related issues, everything is reported earlier is solved now. There is one new issue: > Note: Could not download Source3: https://www.gnu.org/licenses/old- > licenses/lgpl-3.0.txt I think it should be: https://www.gnu.org/licenses/lgpl-3.0.txt The licensing is still bothering me, so I commented on the upstream issue. The problems are minor, related to how the licenses are formally written rather than real doubt on if the authors of the involved code would actually object to using their code in Fedora. Still, from my point of view, things are not quite right, so I cannot accept this package yet. I have noticed that there is some deviation within Fedora, so if you think my requests are not valid, I suggest you ask for a second opinion — it may be that I am wrong here. > (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. I commented in the upstream ticket. > > 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 I commented in the upstream ticket. > > 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. Yes, there is nothing in BSD that conflicts with conditions in LGPL. But the original BSD conditions still have to be fulfilled. It is a very short license, basically *only* asking you to include the license. Anyhow, there is nothing to fix here. The said file is not used in compilation of installed binaries and when installed in .h form, it includes the license in the file itself. > > 3. > > > Provides: bundled(eyescale-cmake-common) > > > > Use versioned Provides. > > Reference: > > https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling > > > Fixed. OK > > 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. OK > > 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. OK > > 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. OK > > 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. 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 OK > > 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. OK > > 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. OK -- 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=976793 _______________________________________________ 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
