[Bug 976793] Review Request: lunchbox - C++ library for
multi-threaded programming
by bugzilla@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #41 from Jaroslav Škarvada <jskarvad(a)redhat.com> ---
(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.
2 years, 7 months