Bug 470702

Summary: Review Request: L-function - L-function calculator
Product: [Fedora] Fedora Reporter: Conrad Meyer <cse.cem+redhatbugz>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alex, fedora-package-review, notting, rdieter
Target Milestone: ---Flags: j: 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: 2009-03-22 22:00:58 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 Conrad Meyer 2008-11-09 01:49:32 UTC
Spec URL: http://konradm.fedorapeople.org/fedora/SPECS/L-function.spec
SRPM URL: http://konradm.fedorapeople.org/fedora/SRPMS/L-function-1.2-1.fc9.src.rpm
Description:
A library and a command-line executable that do computations on a class
of special functions called L-functions. Among others, this class
includes the Riemann zeta function and the Ramanujan tau function.

Comment 1 Rex Dieter 2008-12-11 19:57:18 UTC
Scratch build:
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=993881

Comment 2 Jason Tibbitts 2009-03-09 22:42:07 UTC
Indeed, this still builds fine on rawhide and rpmlint is silent.

The tarball name is simply "L", which would be a rather poor package name; "L-function" seems a reasonable replacement.

Does the -devel package actually require the main package?  There are no headers or anything, and the main package doesn't seem to have anything that would be necessary for development.  Also, the description for the devel package mentions header files, which don't seem to be there.  I note that there are headers in the source archive but I'm not sure if they're supposed to make it into the final package.

Have you considered enabling the PARI support?

* source files match upstream.  sha256sum:
   7f57eb3bc411caf2a75c9400be0bbaa2566b164386895eb4dd7d4ee9b08f24f8  
   L-1.2.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.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none).
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
? final provides and requires:
   L-function = 1.2-1.fc11
   L-function(x86-64) = 1.2-1.fc11
  =
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libgcc_s.so.1(GCC_4.0.0)(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)
   libstdc++.so.6(GLIBCXX_3.4.11)(64bit)
   libstdc++.so.6(GLIBCXX_3.4.9)(64bit)

  L-function-devel-1.2-1.fc11.x86_64.rpm
   L-function-static = 1.2-1.fc11
   L-function-devel = 1.2-1.fc11
   L-function-devel(x86-64) = 1.2-1.fc11
  =
?  L-function = 1.2-1.fc11

* %check is not present; no test suite upstream.
  I installed and ran the examples in the README file and it seems to work, 
  although I have no idea if the values are actually correct.

* no shared libraries are added to the regular linker search paths.
* 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
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* static libraries present:
  No shared libraries are generate, so static libs are in the -devel package.
  -static is provided.

* no libtool .la files.

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Comment 3 Conrad Meyer 2009-03-14 23:29:15 UTC
(In reply to comment #2)
> Does the -devel package actually require the main package?  There are no
> headers or anything, and the main package doesn't seem to have anything that
> would be necessary for development.

I was Require'ing it for the README and COPYING, but maybe that's not needed. What do you think?

> Also, the description for the devel
> package mentions header files, which don't seem to be there.  I note that there
> are headers in the source archive but I'm not sure if they're supposed to make
> it into the final package.

I have added them to the devel/ package.

> Have you considered enabling the PARI support?

Enabled.

New URLs:
http://konradm.fedorapeople.org/fedora/SPECS/L-function.spec
http://konradm.fedorapeople.org/fedora/SRPMS/L-function-1.2-2.fc10.src.rpm

Comment 4 Jason Tibbitts 2009-03-17 01:10:33 UTC
This failed to build for me:

g++ -Wa,-W -O3  -Wno-deprecated -DINCLUDE_PARI -ffast-math -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -lpari -I../include -I/usr/include/pari  -c Lcommandline_elliptic.cc
In file included from Lcommandline_elliptic.cc:23:
../include/Lcommandline_elliptic.h:35:66: error: pari.h: No such file or directory

I guess there's a missing build dependency.

Comment 5 Conrad Meyer 2009-03-18 00:41:11 UTC
Yep, it's missing pari-devel. Added it and tested in mock:
http://konradm.fedorapeople.org/fedora/SPECS/L-function.spec
http://konradm.fedorapeople.org/fedora/SRPMS/L-function-1.2-3.fc10.src.rpm

Comment 6 Alex Lancaster 2009-03-18 04:28:57 UTC
(In reply to comment #5)
> Yep, it's missing pari-devel. Added it and tested in mock:
> http://konradm.fedorapeople.org/fedora/SPECS/L-function.spec

.spec file isn't updated it's still 1.2-2 not 1.2-3

Comment 7 Conrad Meyer 2009-03-18 04:33:52 UTC
Sorry, updated now, same URLs.

Comment 8 Jason Tibbitts 2009-03-20 21:37:45 UTC
The new package builds fine and includes the header files in the proper place.  Everything looks good to me.

APPROVED

Comment 9 Conrad Meyer 2009-03-20 21:49:23 UTC
Thanks for the review.

New Package CVS Request
=======================
Package Name: L-function
Short Description: L-function calculator
Owners: konradm
Branches: F-10
InitialCC:

Comment 10 Kevin Fenzi 2009-03-22 05:46:50 UTC
cvs done.

Comment 11 Conrad Meyer 2009-03-22 22:00:58 UTC
Build in rawhide, closing.
http://koji.fedoraproject.org/koji/taskinfo?taskID=1253389