https://bugzilla.redhat.com/show_bug.cgi?id=976793
Bug ID: 976793 Summary: Review Request: Lunchbox - C++ library for multi-threaded programming Product: Fedora Version: rawhide Component: Package Review Severity: unspecified Priority: unspecified Assignee: nobody@fedoraproject.org Reporter: jskarvad@redhat.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Blocks: 758472
Spec URL: http://jskarvad.fedorapeople.org/libLunchbox.spec SRPM URL: http://jskarvad.fedorapeople.org/libLunchbox-1.4.0-1.fc18.src.rpm
Description: Lunchbox is C++ library for multi-threaded programming, providing OS abstraction, utility classes and high-performance primitives, such as atomic variables, spin locks and lock-free containers. It is needed by Equalizer.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #1 from Jaroslav Škarvada jskarvad@redhat.com --- I already renamed it to libLunchbox. It would be great if upstream could change the project name. I will ask them about it.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Mario Blättermann mario.blaettermann@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: Lunchbox - |Review Request: libLunchbox |C++ library for |- C++ library for |multi-threaded programming |multi-threaded programming
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #2 from Michael Schwendt mschwendt@gmail.com --- This will be non-trivial to review, and I dunno whether I will find enough time this weekend to do everything. How deeply have you reviewed the package already?
rpmlint tells: libLunchbox.src: E: description-line-too-long C Lunchbox is C++ library for multi-threaded programming, providing OS abstraction,
Name: libLunchbox Group: Development/Libraries
"System Environment/Libraries" is the group for the run-time library base packages.
License: LGPLv2
It's more than that.
Lunchbox-1.4.0/lunchbox/md5/md5.cc and md5.hh : This is a copied MD5 implementation with an own license by RSA Data Security Inc. And it's one not mentioned at https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries already.
Lunchbox-1.4.0/lunchbox/atomic.h : Distributed under the Boost Software License, Version 1.0.
Lunchbox-1.4.0/lunchbox/indexIterator.h : LGPLv3 Lunchbox-1.4.0/lunchbox/lfVector.h : LGPLv3 Lunchbox-1.4.0/lunchbox/lfVectorIterator.h : LGPLv3 Lunchbox-1.4.0/lunchbox/serializable.h : LGPLv3 Lunchbox-1.4.0/tests/lfVector.cpp : LGPLv3 Lunchbox-1.4.0/tests/servus.cpp : LGPLv3
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Clarifi...
BuildRequires: cmake, doxygen, graphviz
build.log contains a few "could not find …" messages. Are any of them [optional] missing BuildRequires?
%package devel Requires: pkgconfig, cmake
Both should be dropped. There's an automatic dependency on /usr/bin/pkg-config already. And this -devel package certainly does not strictly require cmake, because e.g. it contains a pkgconfig file and can be used without cmake. Feel free to include the cmake directories based on this in the guidelines:
https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_b...
%package doc Requires: %{name} = %{version}-%{release}
Separate documentation -doc packages typically don't require the base package. It should be possible to install documentation without having to install a program and all its dependencies.
With a total size of just 76716 bytes, the files in this -doc package could stay in the -devel package.
%files %{_datadir}/%{name}
This tree belongs into the -devel package.
drwxr-xr-x /usr/share/libLunchbox drwxr-xr-x /usr/share/libLunchbox/CMake -rw-r--r-- /usr/share/libLunchbox/CMake/options.cmake
The path is broken afaics after you move the file in the %install section.
$ cd Lunchbox-1.4.0 $ grep options.cmake * -R CMake/FindLunchbox.cmake: include("${LUNCHBOX_LIBRARY_DIRS}/../share/Lunchbox/CMake/options.cmake") CMakeLists.txt:install(FILES ${CMAKE_BINARY_DIR}/options.cmake lunchbox/configure.cmake:set(OPTIONS_CMAKE ${CMAKE_BINARY_DIR}/options.cmake)
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #3 from Jaroslav Škarvada jskarvad@redhat.com --- (In reply to Michael Schwendt from comment #2) Thanks for the comments, I will try to go through them probably during next week and provide new version.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #4 from Jaroslav Škarvada jskarvad@redhat.com --- Reply from Stefan regarding the rename to libLunchbox:
Hi Jaroslav,
I've looked into Lunchbox vs. libLunchbox and came to the conclusion that only the package name should change, the rest should stay:
include/Lunchbox/... libLunchbox.... share/Lunchbox/...
Do you agree? If so, what do I need to do?
Changing the project name to libLunchbox doesn't make sense to me, since it entails:
find_package(libLunchbox) include/libLunchbox/... liblibLunchbox...
and so on for consistency.
Cheers,
Stefan.
I am +1 for the package name only change. Any comments?
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #5 from Michael Schwendt mschwendt@gmail.com --- Rest assured, it wouldn't be the first project to do that. Project name "foo" releases library with SONAME libfoo.so.N in tarball "libfoo" with headers in /usr/include/foo (#include <foo/blubb.h>) and using root directories such as /usr/share/foo.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #6 from Christopher Meng cickumqt@gmail.com --- I'm still confused with pkgconfig explicit requires now.
And can you update this package to 1.6.0 version?
Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |cickumqt@gmail.com
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jskarvad@redhat.com Flags| |fedora-review? | |needinfo?(jskarvad@redhat.c | |om)
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Jaroslav Škarvada jskarvad@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jskarvad@redhat.c | |om) |
--- Comment #7 from Jaroslav Škarvada jskarvad@redhat.com --- (In reply to Christopher Meng from comment #6)
I'm still confused with pkgconfig explicit requires now.
It's installing pkgconfig files and the basedir is provided by pkgconfig.
And can you update this package to 1.6.0 version?
I upgraded it to 1.9.1: Spec URL: http://jskarvad.fedorapeople.org/libLunchbox.spec SRPM URL: http://jskarvad.fedorapeople.org/libLunchbox-1.9.1-1.fc20.src.rpm
But I am afraid it would require re-packing according to MPI guidelines, will look on it later.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Jonathan Wakely jwakely@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jwakely@redhat.com
--- Comment #8 from Jonathan Wakely jwakely@redhat.com --- I wondered why the upstream still has its own thread library, instead of using std::thread / boost::thread etc.
And indeed the upstream API docs say the Lunchbox classes are deprecated.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
James Hogarth james.hogarth@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |james.hogarth@gmail.com Flags| |needinfo?(jskarvad@redhat.c | |om)
--- Comment #9 from James Hogarth james.hogarth@gmail.com --- This bug has not had an update in a long time.
Are you still intending to progress this?
As per policy if there is no update this bug will be closed.
https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
https://bugzilla.redhat.com/show_bug.cgi?id=976793
Jaroslav Škarvada jskarvad@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jskarvad@redhat.c | |om) |
--- Comment #10 from Jaroslav Škarvada jskarvad@redhat.com --- (In reply to James Hogarth from comment #9)
This bug has not had an update in a long time.
Are you still intending to progress this?
As per policy if there is no update this bug will be closed.
https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
Hi,
I still want to get it into Fedora. IMHO it's waiting on the reviewer.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
--- Comment #11 from Jaroslav Škarvada jskarvad@redhat.com --- (In reply to Jonathan Wakely from comment #8)
I wondered why the upstream still has its own thread library, instead of using std::thread / boost::thread etc.
And indeed the upstream API docs say the Lunchbox classes are deprecated.
IIRC it's there mainly for the 'bino' [1] from the rpmfusion [2], which can use 'Equalizer', which IIRC uses 'Lunchbox', from the upstream docs [3]:
Lunchbox is a C++ library for multi-threaded programming, providing OS abstraction, utility classes and high-performance primitives, such as atomic variables, spin locks and lock-free containers. It is used as a base library for Collage and Equalizer.
Thus I think this package is still useful.
[1] http://bino3d.org/ [2] http://download1.rpmfusion.org/free/fedora/releases/22/Everything/x86_64/os/... [3] http://www.equalizergraphics.com/
https://bugzilla.redhat.com/show_bug.cgi?id=976793
James Hogarth james.hogarth@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(i@cicku.me)
--- Comment #12 from James Hogarth james.hogarth@gmail.com --- Christopher do you still want to review this?
If not best to unset the assignee and flag so someone else can take over.
https://bugzilla.redhat.com/show_bug.cgi?id=976793
James Hogarth james.hogarth@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW Assignee|i@cicku.me |nobody@fedoraproject.org Flags|fedora-review? | |needinfo?(i@cicku.me) |
--- Comment #13 from James Hogarth james.hogarth@gmail.com --- It's been over a week with no response from the reviewer to the NeedsInfo flag.
Resetting the review flag an unassigning as per policy.
package-review@lists.fedoraproject.org