Bug 183947 - Review Request: R-waveslim - R package for wavelets
Review Request: R-waveslim - R package for wavelets
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-03-03 19:00 EST by José Matos
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-08 18:34:17 EST
Type: ---
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 José Matos 2006-03-03 19:00:57 EST
Spec Name or Url: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/R-waveslim.spec
SRPM Name or Url: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/R-waveslim-1.5-1.src.rpm
Description:
Basic wavelet routines for time series (1D), image (2D)
and array (3D) analysis.  The code provided here is based on
wavelet methodology developed in Percival and Walden (2000);
Gencay, Selcuk and Whitcher (2001); the dual-tree complex wavelet
transform (CWT) from Kingsbury (1999, 2001) as implemented by
Selesnick; and Hilbert wavelet pairs (Selesnick 2001, 2002).
Comment 1 Rick L Vinyard Jr 2006-03-05 13:28:05 EST
Not a review; just a few comments from a newbie. These are based on the other
comments that I've received and a quick scan...

%build missing an explicit make statement. Something like:
     make %{?_smp_mflags}

Comment 2 José Matos 2006-03-05 17:14:48 EST
And where is it the Makefile? ;-) 
 
Seriously this rule does not apply to programming languages that have their 
own way to install packages. R is a free implementation of the S programming 
language. 
 
If you look to most python packages you will notice that that there is not any 
make there as well, they use distutils. The same should apply to perl packages 
and I would expect to ruby as well. 
 
Thank you for your input but it does not apply in this case. :-) 
Comment 3 Jason Tibbitts 2006-03-07 00:18:23 EST
I like the package; contratulations on working up a specfile generator for R
packages.  Everything looks pretty good.  rpmlint says:

W: R-waveslim invalid-license GPL version 2 or newer
W: R-waveslim no-documentation

Good:
We have no standards for R package naming, but R-packagename is reasonable.
The specfile is clean and understandable.
The source matches upstream.
Package dependencies are sane.

Bad:
rpmlint doesn't like the License: tag.  No package in extras yet specifies the
acceptable GPL version; I would suggest just using GPL.  When GPLv3 is out I
suppose we'll have to be more specific.

rpmlint doesn't find any documentation.  Of course this package includes a pile
of documentation, but it's all incorporated into R's internal documentation
system.  The base R package doesn't list any of the internal documentation as
%doc so I don't think it would be reasonable to do so here.  This shouldn't be a
blocker, but if someone takes issue with it then the base R package needs some
work as well.

The package fails to build in mock:

* Installing *source* package 'waveslim' ...
** libs
gfortran   -fPIC  -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32
 -march=i386 -mtune=pentium4 -fasynchronous-unwind-tables -c bell-p-w.f -o
bell-p-w.o
make: gfortran: Command not found
make: *** [bell-p-w.o] Error 127
ERROR: compilation failed for package 'waveslim'
** Removing
'/var/tmp/R-waveslim-1.5-1.fc5-root-mockbuild/usr/lib/R/library/waveslim'
error: Bad exit status from /var/tmp/rpm-tmp.71138 (%install)

I believe you need a RuildRequires: gcc-gfortran.
Comment 4 José Matos 2006-03-08 04:37:08 EST
According to the discussion had in packagers mailing list I have: 
 
Rename License to GPL 
Added DESCRIPTION in %doc 
Added gcc-gfortran in BR 
 
I have modified cran2rpmspec to follow these guidelines (the last two at 
least) as well. 
 
New package: 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/R-waveslim.spec 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/R-waveslim-1.5-2.src.rpm 
Comment 5 Jason Tibbitts 2006-03-08 16:16:10 EST
Looks good.  The package now builds in mock and rpmlint is now silent.

Approved.

The only question I have is the following from %check:

* checking if this is a source package ... WARNING
Subdirectory 'waveslim/src' contains object files.
[...]
WARNING: There was 1 warning, see
  /builddir/build/BUILD/waveslim.Rcheck/00check.log
for details

No idea what that means, but perhaps it's something for a future cleanup.
Comment 6 José Matos 2006-03-08 17:38:47 EST
(In reply to comment #5)  
> Looks good.  The package now builds in mock and rpmlint is now silent.  
>   
> Approved.  
>   
> The only question I have is the following from %check:  
>   
> * checking if this is a source package ... WARNING  
> Subdirectory 'waveslim/src' contains object files.  
  
I guess that it is because of the file waveslim/src/so_locations that is a  
text file:  
  
waveslim.so \  
                :st = .text 0x5ffe0000, 0x00010000:\  
                :st = .data 0x5fff0000, 0x00010000:  
 
> [...]  
> WARNING: There was 1 warning, see  
>   /builddir/build/BUILD/waveslim.Rcheck/00check.log  
> for details  
>   
> No idea what that means, but perhaps it's something for a future cleanup.  
  
  Probably, or to report it upstream. :-) 
  
Comment 7 José Matos 2006-03-08 18:34:17 EST
Build on target fedora-development-extras succeeded. 

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