Bug 976793 - Review Request: libLunchbox - C++ library for multi-threaded programming
Review Request: libLunchbox - C++ library for multi-threaded programming
Status: NEW
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 758472
  Show dependency treegraph
 
Reported: 2013-06-21 09:17 EDT by Jaroslav Škarvada
Modified: 2015-12-11 11:20 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jaroslav Škarvada 2013-06-21 09:17:43 EDT
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.
Comment 1 Jaroslav Škarvada 2013-06-21 11:15:29 EDT
I already renamed it to libLunchbox. It would be great if upstream could change the project name. I will ask them about it.
Comment 2 Michael Schwendt 2013-06-22 14:52:23 EDT
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_Clarification


> 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_by_a_package_which_is_not_required_for_your_package_to_function


> %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)
Comment 3 Jaroslav Škarvada 2013-06-25 10:23:49 EDT
(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.
Comment 4 Jaroslav Škarvada 2013-06-25 10:26:06 EDT
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?
Comment 5 Michael Schwendt 2013-06-26 05:27:52 EDT
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.
Comment 6 Christopher Meng 2013-10-25 03:46:07 EDT
I'm still confused with pkgconfig explicit requires now.

And can you update this package to 1.6.0 version?

Thanks.
Comment 7 Jaroslav Škarvada 2014-07-07 12:25:33 EDT
(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.
Comment 8 Jonathan Wakely 2015-03-06 09:00:34 EST
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.
Comment 9 James Hogarth 2015-12-03 22:20:54 EST
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
Comment 10 Jaroslav Škarvada 2015-12-04 04:35:48 EST
(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.
Comment 11 Jaroslav Škarvada 2015-12-04 04:41:38 EST
(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/bino-1.4.4-6.fc22.x86_64.rpm
[3] http://www.equalizergraphics.com/
Comment 12 James Hogarth 2015-12-04 06:07:38 EST
Christopher do you still want to review this?

If not best to unset the assignee and flag so someone else can take over.
Comment 13 James Hogarth 2015-12-11 11:20:46 EST
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.

Note You need to log in before you can comment on or make changes to this bug.