Bug 515080 - Review Request: R-preprocessCore - A collection of pre-processing functions
Review Request: R-preprocessCore - A collection of pre-processing functions
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 515081
  Show dependency treegraph
 
Reported: 2009-08-01 13:53 EDT by Pierre-YvesChibon
Modified: 2009-08-11 18:35 EDT (History)
2 users (show)

See Also:
Fixed In Version: 1.6.0-2.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-08-11 18:32:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Pierre-YvesChibon 2009-08-01 13:53:47 EDT
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 16:08:52 EDT
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 16:18:55 EDT
(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 17:26:38 EDT
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 18:57:47 EDT
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 02:53:12 EDT
I have ask upstream if he can do the change on the cvs
Comment 6 Jason Tibbitts 2009-08-04 11:06:35 EDT
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 14:58:22 EDT
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 02:06:37 EDT
Looks good to me.

APPROVED
Comment 9 Pierre-YvesChibon 2009-08-05 02:13:30 EDT
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 02:16:22 EDT
CVS done.
Comment 11 Fedora Update System 2009-08-05 04:17:10 EDT
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 04:17:16 EDT
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 01:05:37 EDT
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 01:07:09 EDT
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 18:32:06 EDT
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 18:35:17 EDT
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.