https://bugzilla.redhat.com/show_bug.cgi?id=976793
Otto Urpelainen <otu...@iki.fi> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags| |needinfo?(jskarvad@redhat.c
| |om)
--- Comment #37 from Otto Urpelainen <otu...@iki.fi> ---
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
