Bug 515080 - Review Request: R-preprocessCore - A collection of pre-processing functions
Summary: Review Request: R-preprocessCore - A collection of pre-processing functions
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 515081
TreeView+ depends on / blocked
 
Reported: 2009-08-01 17:53 UTC by Pierre-YvesChibon
Modified: 2009-08-11 22:35 UTC (History)
2 users (show)

Fixed In Version: 1.6.0-2.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-08-11 22:32:11 UTC
Type: ---
Embargoed:
j: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Pierre-YvesChibon 2009-08-01 17:53:47 UTC
Spec URL: http://pingou.fedorapeople.org/RPMs/R-preprocessCore.spec
SRPM URL: http://pingou.fedorapeople.org/RPMs/R-preprocessCore-1.6.0-1.fc11.src.rpm
Description: 
A library of core preprocessing routines

Comment 1 Jason Tibbitts 2009-08-01 20:08:52 UTC
R packaging is getting to be as automatic as Perl package, and about as boring to review.  And, as with Perl, the biggest problem is licensing.

Note several of the C files in src are GPLv2+, not LGPLv2+, unless that's a typo.  This would seem to contradict the DESCRIPTION file.  This should be clarified with the upstream developers.

In addition to the usual one-line-command-in-* complaints, rpmlint says:
  R-preprocessCore-devel.x86_64: W: only-non-binary-in-usr-lib
  R-preprocessCore-devel.x86_64: W: no-documentation
which are both OK; R needs to find the headers in its namespace under _libdir.

Your %descriptions (both of them) are missing periods.

* source files match upstream.  sha256sum:                     
   630b5fa4c98492eb4a189dfafb68213b51af88da928fa4fc90aae6e544811a31  
   preprocessCore_1.6.0.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.                                                              
* description is OK (could use periods).
* dist tag is present.
* build root is OK.
X license field does not seem to match the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  R-preprocessCore-1.6.0-1.fc12.x86_64.rpm
   preprocessCore.so()(64bit)
   R-preprocessCore = 1.6.0-1.fc12
   R-preprocessCore(x86-64) = 1.6.0-1.fc12
  =
   /bin/sh
   R
   R-methods
   libR.so()(64bit)
   libRblas.so()(64bit)
   libRlapack.so()(64bit)
   libgfortran.so.3()(64bit)

  R-preprocessCore-devel-1.6.0-1.fc12.x86_64.rpm
   R-preprocessCore-devel = 1.6.0-1.fc12
   R-preprocessCore-devel(x86-64) = 1.6.0-1.fc12
  =
   R-preprocessCore = 1.6.0

* %check is present and all tests pass.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* scriptlets are OK (R package registration).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Unfortunately I cannot approve this due to the licensing issue.

Comment 2 Pierre-YvesChibon 2009-08-01 20:18:55 UTC
(In reply to comment #1)
> R packaging is getting to be as automatic as Perl package, and about as boring
> to review.  And, as with Perl, the biggest problem is licensing.

I really sorry about that :-)

> Note several of the C files in src are GPLv2+, not LGPLv2+, unless that's a
> typo.  This would seem to contradict the DESCRIPTION file.  This should be
> clarified with the upstream developers.

I will contact upstream and let you know

> Your %descriptions (both of them) are missing periods.
I'll take care of that


Thanks for the review

Comment 3 Pierre-YvesChibon 2009-08-01 21:26:38 UTC
I sent an email to upstream there his is (complete !) answer

"The licence in the DESCRIPTION is the intended licence"

Comment 4 Jason Tibbitts 2009-08-03 22:57:47 UTC
In that case what I'd do is to include that email (including the headers and such) in a README.License file and also indicate in that file that the license statements in the various pieces of code do not apply.  Honestly I'd make a note of the license blocks in each source file so that it's easy to see what is being overridden by that statement by upstream.

It might also be a good idea to double check that there's no code there which is obviously taken from some other source, as it would not be possible for a simple statement like the above to override the license on such code.

Comment 5 Pierre-YvesChibon 2009-08-04 06:53:12 UTC
I have ask upstream if he can do the change on the cvs

Comment 6 Jason Tibbitts 2009-08-04 15:06:35 UTC
You can do that if you like, but for Fedora's purposes the email is sufficient.  It just needs to be stuck in a file that's included in the package.

Comment 7 Pierre-YvesChibon 2009-08-04 18:58:22 UTC
I added the mail in the file preprocessCore_license.

Spec URL: http://pingou.fedorapeople.org/RPMs/R-preprocessCore.spec
SRPM URL:
http://pingou.fedorapeople.org/RPMs/R-preprocessCore-1.6.0-2.fc11.src.rpm

Comment 8 Jason Tibbitts 2009-08-05 06:06:37 UTC
Looks good to me.

APPROVED

Comment 9 Pierre-YvesChibon 2009-08-05 06:13:30 UTC
Thanks for the review :)


New Package CVS Request
=======================
Package Name: R-preprocessCore
Short Description:  A collection of pre-processing functions
Owners: pingou
Branches: F-10, F-11
InitialCC:

Comment 10 Jason Tibbitts 2009-08-05 06:16:22 UTC
CVS done.

Comment 11 Fedora Update System 2009-08-05 08:17:10 UTC
R-preprocessCore-1.6.0-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/R-preprocessCore-1.6.0-2.fc11

Comment 12 Fedora Update System 2009-08-05 08:17:16 UTC
R-preprocessCore-1.6.0-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/R-preprocessCore-1.6.0-2.fc10

Comment 13 Fedora Update System 2009-08-07 05:05:37 UTC
R-preprocessCore-1.6.0-2.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update R-preprocessCore'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8353

Comment 14 Fedora Update System 2009-08-07 05:07:09 UTC
R-preprocessCore-1.6.0-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update R-preprocessCore'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8356

Comment 15 Fedora Update System 2009-08-11 22:32:06 UTC
R-preprocessCore-1.6.0-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2009-08-11 22:35:17 UTC
R-preprocessCore-1.6.0-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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