Bug 470702 - Review Request: L-function - L-function calculator
Review Request: L-function - L-function calculator
Status: CLOSED RAWHIDE
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:
  Show dependency treegraph
 
Reported: 2008-11-08 20:49 EST by Conrad Meyer
Modified: 2009-03-22 18:00 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-22 18:00:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Conrad Meyer 2008-11-08 20:49:32 EST
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 14:57:18 EST
Scratch build:
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=993881
Comment 2 Jason Tibbitts 2009-03-09 18:42:07 EDT
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 19:29:15 EDT
(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-16 21:10:33 EDT
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-17 20:41:11 EDT
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 00:28:57 EDT
(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 00:33:52 EDT
Sorry, updated now, same URLs.
Comment 8 Jason Tibbitts 2009-03-20 17:37:45 EDT
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 17:49:23 EDT
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 01:46:50 EDT
cvs done.
Comment 11 Conrad Meyer 2009-03-22 18:00:58 EDT
Build in rawhide, closing.
http://koji.fedoraproject.org/koji/taskinfo?taskID=1253389

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