Bug 529861

Summary: Review Request: userspace-rcu - RCU (read-copy-update) in user space
Product: [Fedora] Fedora Reporter: Jan "Yenya" Kasprzak <kas>
Component: Package ReviewAssignee: Dan Horák <dan>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: dan, fedora-package-review, greenscientist, mcepl, notting, pahan
Target Milestone: ---Flags: dan: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-11-17 10:26:11 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jan "Yenya" Kasprzak 2009-10-20 13:51:52 UTC
Spec URL: http://www.fi.muni.cz/~kas/tmp/userspace-rcu.spec
SRPM URL: http://www.fi.muni.cz/~kas/tmp/userspace-rcu-0.2.4-1.fc11.src.rpm
Description:
This data synchronization library provides read-side access which scales
linearly with the number of cores. It does so by allowing multiples copies
of a given data structure to live at the same time, and by monitoring
the data structure accesses to detect grace periods after which memory
reclamation is possible.

Note: will need a sponsor as this is my attempt to become Fedora package maintainer.

Comment 1 Dan Horák 2009-10-21 10:48:54 UTC
formal review is here, see the notes below:

OK	source files match upstream:
	    a2f7d4813515d8c510163b84cbbf8cd5d8e177ca  userspace-rcu-0.2.4.tar.bz2
OK	package meets naming and versioning guidelines.
OK	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
BAD	license field matches the actual license.
OK	license is open source-compatible (LGPLv2+). License text is included upstream.
OK	latest version is being packaged.
OK	BuildRequires are proper.
OK	compiler flags are appropriate.
OK	%clean is present.
OK	package builds in mock (Rawhide/x86_64).
OK	debuginfo package looks complete.
BAD	rpmlint is silent.
OK	final provides and requires look sane.
BAD	%check is present and all tests pass.
OK	shared libraries are added to the regular linker search paths.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
OK	correct scriptlets present.
OK	code, not content.
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	headers in -devel.
OK	no pkgconfig files.
OK	no libtool .la droppings.
OK	not a GUI app.

- the license is right, but the value of the tag should be LGPLv2+, see https://fedoraproject.org/wiki/Licensing for more details
- the license texts (*.txt) and the licensing "manual" (LICENSE) are included in the source archive and they must be included in the package as %doc
- rpmlint complains a bit:
userspace-rcu-debuginfo.x86_64: W: invalid-license LGPL-2.1
userspace-rcu-devel.x86_64: W: invalid-license LGPL-2.1
userspace-rcu.src: W: invalid-license LGPL-2.1
userspace-rcu.x86_64: W: invalid-license LGPL-2.1
    => see above
userspace-rcu-devel.x86_64: W: no-documentation
    => can be ignored
userspace-rcu.x86_64: W: shared-lib-calls-exit /usr/lib64/liburcu-qsbr.so.0.0.0 exit.5
userspace-rcu.x86_64: W: shared-lib-calls-exit /usr/lib64/liburcu-defer.so.0.0.0 exit.5
userspace-rcu.x86_64: W: shared-lib-calls-exit /usr/lib64/liburcu-mb.so.0.0.0 exit.5
userspace-rcu.x86_64: W: shared-lib-calls-exit /usr/lib64/liburcu.so.0.0.0 exit.5
userspace-rcu.x86_64: W: shared-lib-calls-exit /usr/lib64/liburcu-bp.so.0.0.0 exit.5
    => should be discussed with upstream, run rpmlint -i on the binary rpm for reasons of this warning

- a test suite is included in the source archive, but it's not run. If possible the test suite should be run in the %check section

Comment 2 Jan "Yenya" Kasprzak 2009-10-26 16:10:23 UTC
Thanks for the review. I did the following:
- changed the license to LGPLv2+ (my rpmlint-0.91-1.fc11.noarch did not object
  to the previous value at all)
- added %doc LICENSE to the %files of the main package
  (I am considering moving the %doc README to the -devel package, as the
  information written there is mostly developer oriented)
- added the %check section to run the tests (altough the package does not
  contain a test _suite_ per se, just a bunch of tests without any notion
  of how they should be run).
- as for the shared-lib-calls-exit warning - apparently this is not
  an error, but a design decision of the upstream - they call exit(-1) for
  every fatal error. I am really not sure that trying to change it is the
  task for the packager.

Anyway, the new .spec and .src.rpm files are at the same location (i have kept the release tag at 1, as the previous attempt was not published anywhere in Fedora).

Comment 3 Dan Horák 2009-10-27 10:01:03 UTC
(In reply to comment #2)
> Thanks for the review. I did the following:
> - changed the license to LGPLv2+ (my rpmlint-0.91-1.fc11.noarch did not object
>   to the previous value at all)

I used rpmlint 0.90 on the built rpms. rpmlint should work with the authoritative list of allowed values from the "Licensing" wiki page ...

> - added %doc LICENSE to the %files of the main package

but the actual license texts (gpl-2.0.txt, lgpl-2.1.txt) + lgpl-relicensing.txt are still missing, please add them

>   (I am considering moving the %doc README to the -devel package, as the
>   information written there is mostly developer oriented)

yes, moving the README to -devel makes sense to me

> - added the %check section to run the tests (altough the package does not
>   contain a test _suite_ per se, just a bunch of tests without any notion
>   of how they should be run).

Thanks for adding them. Did you check whether they work even in the chrooted build environment?

> - as for the shared-lib-calls-exit warning - apparently this is not
>   an error, but a design decision of the upstream - they call exit(-1) for
>   every fatal error. I am really not sure that trying to change it is the
>   task for the packager.

The packager should be just aware of the issue and eventually discuss it with the upstream.
 
> Anyway, the new .spec and .src.rpm files are at the same location (i have kept
> the release tag at 1, as the previous attempt was not published anywhere in
> Fedora).  

I am still receiving the old srpm, while the spec is updated. Can you check it?
Every published iteration of the package (even published here in bugzilla) must have an updated version/release plus a changelog entry with summary of the changes. The reviewer (and the community) can easily check for the changes and it also prevents the issue with the srpm.

Comment 4 Dan Horák 2010-02-23 13:56:56 UTC
ping?

Comment 5 Jan "Yenya" Kasprzak 2010-03-01 11:02:33 UTC
http://www.fi.muni.cz/~kas/tmp/userspace-rcu.spec
http://www.fi.muni.cz/~kas/tmp/userspace-rcu-0.4.1-1.fc12.src.rpm

> the actual license texts (gpl-2.0.txt, lgpl-2.1.txt)
> + lgpl-relicensing.txt are still missing, please add them

added.

> moving the README to -devel makes sense to me

done.

> Did you check whether they work even in the chrooted build environment?

Do you mean chrooted or %buildrooted? As for the first option, it depends whether the chrooted environment contains the necessary shared libraries (ld.so, libc, libpthread, and linux-vdso). Which I presume it does.

I have updated the package to the new upstream version, added more tests to the %check section, and built the package. Rpmlint now has no complaints except the "W: shared-lib-calls-exit" issue we have already discussed.

Anyway, sorry for the delay.

Comment 6 Dan Horák 2010-03-18 09:42:37 UTC
(In reply to comment #5)
> > Did you check whether they work even in the chrooted build environment?
> 
> Do you mean chrooted or %buildrooted? As for the first option, it depends
> whether the chrooted environment contains the necessary shared libraries
> (ld.so, libc, libpthread, and linux-vdso). Which I presume it does.

I mean the chrooted environment created by mock to build the package. But as I've checked it works and that's good.
 
> I have updated the package to the new upstream version, added more tests to the
> %check section, and built the package. Rpmlint now has no complaints except the
> "W: shared-lib-calls-exit" issue we have already discussed.
> 
> Anyway, sorry for the delay.    

No problem, there has been also a delay on my side.

There are no issues now and this package is APPROVED and I will sponsor you. If you have any questions regarding the processes or guidelines, don't hesitate to ask me.

Comment 7 Dan Horák 2010-03-18 09:49:35 UTC
Also add yourself to the fedorabugs group in FAS, so you have all the needed rights there.

Comment 8 Jan "Yenya" Kasprzak 2010-03-19 14:33:01 UTC
New Package CVS Request
=======================
Package Name: userspace-rcu
Short Description: RCU (read-copy-update) implementation in user space
Owners: yenya
Branches: F-11 F-12 F-13 EL-5
InitialCC:

Comment 9 Kevin Fenzi 2010-03-19 19:38:54 UTC
CVS done (by process-cvs-requests.py).

Comment 10 Jan "Yenya" Kasprzak 2010-03-31 06:27:36 UTC
I am sorry to ask questions here, but I have still not managed to build the package in Koji. I have CVS set up, checked out, and hopefully tagged as described in https://fedoraproject.org/wiki/PackageMaintainers/Join (except that cvs-import seems to create the tag already - "make tag" fails with "ERROR: Tag userspace-rcu-0_4_1-1_fc14 has been already created.". However, the "make build" fails as well with a rather unhelpful error message:

devel$ make build
/usr/bin/koji  build  dist-rawhide 'cvs://cvs.fedoraproject.org/cvs/pkgs?rpms/userspace-rcu/devel#userspace-rcu-0_4_1-1_fc14'
Error: []
make: *** [build] Error 1

What am I doing wrong?

Comment 11 Matěj Cepl 2010-03-31 09:21:22 UTC
(In reply to comment #10)
> What am I doing wrong?    

You are just doing too much. With just cvs co of your module it builds pretty well (http://koji.fedoraproject.org/koji/taskinfo?taskID=2086338 ... of course, this is just scratch build).

If you want to change anything, then probably your best option is just to bump release (integers are cheap ;)) and then make tag will be happy.

Comment 12 Matěj Cepl 2010-03-31 09:27:30 UTC
BTW, hint ... #fedora-devel on Freenode is pretty friendly place, where are always some people willing to help you with similar issues.

Comment 13 Yannick Brosseau 2012-06-13 22:48:35 UTC
*** Bug 717337 has been marked as a duplicate of this bug. ***

Comment 14 Dan Horák 2013-11-17 10:26:11 UTC
went into the distro via mass-rebuilds long time ago, closing