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.