Bug 475065

Summary: Review Request: givaro - C++ library for arithmetic and algebraic computations
Product: [Fedora] Fedora Reporter: Conrad Meyer <cse.cem+redhatbugz>
Component: Package ReviewAssignee: Martin Gieseking <martin.gieseking>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: alex, fedora-package-review, martin.gieseking, mycae, notting, rdieter, susi.lehtola
Target Milestone: ---Flags: martin.gieseking: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 3.2.15-0.2.rc1.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-10-03 18:55:16 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:
Bug Depends On:    
Bug Blocks: 476299    

Description Conrad Meyer 2008-12-07 08:27:25 UTC
Spec URL: http://konradm.fedorapeople.org/fedora/SPECS/givaro.spec
SRPM URL: http://konradm.fedorapeople.org/fedora/SRPMS/givaro-3.2.13-1.fc9.src.rpm
Description:
Givaro is a C++ library for arithmetic and algebraic computations.
Its main features are implementations of the basic arithmetic of many
mathematical entities: Primes fields, Extensions Fields, Finite Fields,
Finite Rings, Polynomials, Algebraic numbers, Arbitrary precision
integers and rationals (C++ wrappers over gmp) It also provides
data-structures and templated classes for the manipulation of basic
algebraic objects, such as vectors, matrices (dense, sparse, structured),
univariate polynomials (and therefore recursive multivariate).

Comment 1 Conrad Meyer 2008-12-08 00:14:04 UTC
New URLs:
http://konradm.fedorapeople.org/fedora/SPECS/givaro.spec
http://konradm.fedorapeople.org/fedora/SRPMS/givaro-3.2.13-2.fc9.src.rpm

Fixed a multilibs conflict (%{_includedir}/givaro-config.h).

Comment 2 Rex Dieter 2008-12-11 19:55:10 UTC
Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=993866

Comment 3 Michael Schwendt 2009-02-11 11:44:16 UTC
Just a comment, not a review: In your %files sections, you're advised to mark recursively included paths with a trailing slash. That makes it trivial to distinguish files and directories.

Comment 4 Alex Lancaster 2009-02-19 09:30:08 UTC
Can you update taking into account comment #3? and I'll review it.

Comment 5 Alex Lancaster 2009-03-10 03:13:44 UTC
Reassigning to tibbs, given that he's set the review flag... ;)  

tibbs: feel free to reassign back to me if you don't end up doing the review.

Comment 6 Jason Tibbitts 2009-03-10 04:32:05 UTC
Erm, what?  I'm just going through and setting review flags on the tickets where this seems to have been forgotten.  (Basically, any review ticket that is assigned to someone.)  Doing bugzilla cleanup doesn't mean that I actually want to do the review.

Comment 7 Alex Lancaster 2009-03-10 05:25:37 UTC
(In reply to comment #6)
> Erm, what?  I'm just going through and setting review flags on the tickets
> where this seems to have been forgotten.  (Basically, any review ticket that is
> assigned to someone.)  Doing bugzilla cleanup doesn't mean that I actually want
> to do the review.  

OK, I had assigned it to me because I had intended to do the review at some point in the future, but I don't like to set the flag until I'm actually in the process of doing the review in case somebody else wants to take it if I don't get around to it.

Comment 8 Conrad Meyer 2009-03-13 05:15:45 UTC
Alex: I didn't respond to comment #3 because that is a preference thing I and I prefer it the way I have it. I would welcome a review.

Comment 9 D Haley 2009-05-31 03:05:39 UTC
Here is my shot at this.

General comments:
* givaro-makefile is a makefile and not an executable shell script. Remove the $!/bin/sh from it and move the file into %doc.
*Why are the header files split into two sections (gmp++ and %{name})? This may be related to a later point I make about the configure script.
    
        
*  MUST: rpmlint must be run on every package. The output should be posted in the review.[1]]
    
    $ rpmlint ../SRPMS/givaro-3.2.13-2.fc10.src.rpm ../RPMS/i386/givaro-3.2.13-2.fc10.i386.rpm givaro.spec 
givaro.i386: W: shared-lib-calls-exit /usr/lib/libgivaro.so.0.0.2 exit
2 packages and 1 specfiles checked; 0 errors, 1 warnings.

    Please inform upstream that it is not a good idea to do this, or patch this out (if possible). Offending file is: src/kernel/zpz/givzpz16table1.C: Line 46. Could be replaced with an exception or some kind of return code propagation. No idea what the easiest solution is. If upstream is informed that they shouldn't do this, and replies with something sensible, I don't see this as a block.
    
* MUST: The package must be named according to the Package Naming Guidelines .
    OK

* MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. 
    OK

* MUST: The package must meet the Packaging Guidelines .
    As far as I can see it is OK.

* MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines .
    OK
* MUST: The License field in the package spec file must match the actual license. 


* MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.
    OK: Please consider adding AUTHORS and ChangeLog (not a blocker)
* MUST: The spec file must be written in American English.
    OK
* MUST: The spec file for the package MUST be legible. 
    OK
* MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
	FAIL: URL provided gives 404 error. (The requested URL /CASYS/LOGICIELS/givaro/givaro-3.2.13.tar.gz was not found on this server.) Please update URL or request upstream to not remove old tarballs.
* MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. 
	OK. Was able to compile.
 * MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. 
    N/A
* MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.
    OK.
* MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
    OK
* MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. 
    OK
* MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. 
    N/A
* MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. 
    OK
* MUST: A Fedora package must not list a file more than once in the spec file's %files listings. 
    OK
* MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. 
    OK
* MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
    OK
* MUST: Each package must consistently use macros. 
    OK
* MUST: The package must contain code, or permissable content. 
    OK
* MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). 
    OK, no large -doc files
* MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. 
    OK.
* MUST: Header files must be in a -devel package. 
    OK
* MUST: Static libraries must be in a -static package. 
    OK
* MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). 
    N/A
* MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. 
    OK
* MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} 
    OK
* MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
    OK
* MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. 
    N/A
* MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. [24]
    OK
* MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [25]
    OK
* MUST: All filenames in rpm packages must be valid UTF-8. [26]
    OK


*  SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. 
    N/A
* SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. 
    N/A
* SHOULD: The reviewer should test that the package builds in mock. 
    Koji OK
* SHOULD: The package should compile and build into binary rpms on all supported architectures. 
    OK
* SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example.
    Would it be better to patch the -config file to return /usr/include/givaro/ for --cflags rather than /usr/include ? Also it would be good if `givaro-config --cflags --libs` did not return endlines. (add -n to lines 58 and 62, also add leading space to linker & include flags). Finally this may be because of the gmp++ bit?
    The examples given on the website are a bit bogus -- requires some preprocessor that doesn't exist (404 again). Could you pack a trivial example that compiles into doc directory?
* SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. 
    N/A
* SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. 
    OK
* SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. 
    N/A
* SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. 
    OK

Comment 10 Conrad Meyer 2009-05-31 03:41:12 UTC
(In reply to comment #9)
> Here is my shot at this.
> 
> General comments:
> * givaro-makefile is a makefile and not an executable shell script. Remove the
> $!/bin/sh from it and move the file into %doc.

Why should we put this in %doc? How is it remotely helpful?

> *Why are the header files split into two sections (gmp++ and %{name})? This may
> be related to a later point I make about the configure script.

gmp++ is C++ bindings for gmp, I think.

> *  MUST: rpmlint must be run on every package. The output should be posted in
> the review.[1]]
> 
>     $ rpmlint ../SRPMS/givaro-3.2.13-2.fc10.src.rpm
> ../RPMS/i386/givaro-3.2.13-2.fc10.i386.rpm givaro.spec 
> givaro.i386: W: shared-lib-calls-exit /usr/lib/libgivaro.so.0.0.2
> exit
> 2 packages and 1 specfiles checked; 0 errors, 1 warnings.
> 
>     Please inform upstream that it is not a good idea to do this, or patch this
> out (if possible). Offending file is: src/kernel/zpz/givzpz16table1.C: Line 46.
> Could be replaced with an exception or some kind of return code propagation. No
> idea what the easiest solution is. If upstream is informed that they shouldn't
> do this, and replies with something sensible, I don't see this as a block.

This is poor coding practice by upstream, but it isn't a blocker.

> ...
> Please consider adding AUTHORS and ChangeLog (not a blocker)

Ok (though this is not something I feel strongly about).

> ...
> * MUST: The sources used to build the package must match the upstream source,
> as provided in the spec URL. Reviewers should use md5sum for this task. If no
> upstream URL can be specified for this package, please see the Source URL
> Guidelines for how to deal with this.
>  FAIL: URL provided gives 404 error. (The requested URL
> /CASYS/LOGICIELS/givaro/givaro-3.2.13.tar.gz was not found on this server.)
> Please update URL or request upstream to not remove old tarballs.

Again, upstream's fault...

> ...
> * SHOULD: The reviewer should test that the package functions as described. A
> package should not segfault instead of running, for example.
>     Would it be better to patch the -config file to return /usr/include/givaro/
> for --cflags rather than /usr/include ? Also it would be good if `givaro-config
> --cflags --libs` did not return endlines. (add -n to lines 58 and 62, also add
> leading space to linker & include flags). Finally this may be because of the
> gmp++ bit?

Something like that sounds necessary, yes.

>     The examples given on the website are a bit bogus -- requires some
> preprocessor that doesn't exist (404 again). Could you pack a trivial example
> that compiles into doc directory?

Sorry, I'm not familiar with the use of this library, I'm just interested in getting SAGE (and dependencies) packaged and in Fedora.

> ...

Thanks for the review!

Comment 11 D Haley 2009-05-31 05:00:28 UTC
Some quick replies:

> Why should we put this in %doc? How is it remotely helpful?
Maybe %doc is not quite the right place, but as far as I can see it is only an example makefile, hence documentation, or similar. It certainly should not be executable, nor should it be mislabelled as a shell script and definitely does not belong in /usr/bin.

> This is poor coding practice by upstream, but it isn't a blocker.
Still, please try to contact upstream and if they reply put a copy of the relevant reply here. or state that they are non-responsive. I note that te

> Again, upstream's fault...
This is a blocker. I cannot confirm the md5sum without this, which is required for review - we need to build against an upstream tarball somewhere, and agree on its location. http://www-lmc.imag.fr/CASYS/LOGICIELS/givaro/Downloads/ may have what we need.

>Sorry, I'm not familiar with the use of this library, I'm just interested in
>getting SAGE (and dependencies) packaged and in Fedora.

I need some way to confirm that the system is working properly -- do you have a suggestion? 

>gmp++ is C++ bindings for gmp, I think
So is this two separate projects bundled into one? I am totally confused.

Comment 12 D Haley 2009-05-31 05:31:50 UTC
Additional:

None of the source files have a proper GPL header, as documented in the GPL licence "How to Apply These Terms to Your New Programs". Please notify upstream. I am uncertain as to whether this is a blocker or not.

Comment 13 Conrad Meyer 2009-05-31 05:36:43 UTC
(In reply to comment #11)
> Some quick replies:
> 
> > Why should we put this in %doc? How is it remotely helpful?
> Maybe %doc is not quite the right place, but as far as I can see it is only an
> example makefile, hence documentation, or similar. It certainly should not be
> executable, nor should it be mislabelled as a shell script and definitely does
> not belong in /usr/bin.

Ah, sorry, didn't realize I was putting it in /usr/bin. I would prefer to not install it anywhere, I think.

> > This is poor coding practice by upstream, but it isn't a blocker.
> Still, please try to contact upstream and if they reply put a copy of the
> relevant reply here. or state that they are non-responsive. I note that te

Sorry, I don't see any sort of contact info or bug tracking system on the home page.

> > Again, upstream's fault...
> This is a blocker. I cannot confirm the md5sum without this, which is required
> for review - we need to build against an upstream tarball somewhere, and agree
> on its location. http://www-lmc.imag.fr/CASYS/LOGICIELS/givaro/Downloads/ may
> have what we need.

Eh, it won't be an issue *after* review though. I can update to the most recent tarball and you can verify against that md5sum (or, please, sha1sum).

> >Sorry, I'm not familiar with the use of this library, I'm just interested in
> >getting SAGE (and dependencies) packaged and in Fedora.
> 
> I need some way to confirm that the system is working properly -- do you have a
> suggestion? 

Maybe look at some of their examples on the webpage?
 
> >gmp++ is C++ bindings for gmp, I think
> So is this two separate projects bundled into one? I am totally confused.  

It's developed by the givaro folks, and they distribute it with givaro. I don't believe it's a library that anyone else is supposed to use (at least not yet). But you can try asking them if you want a better explanation.

Comment 14 Conrad Meyer 2009-05-31 05:37:26 UTC
(In reply to comment #12)
> Additional:
> 
> None of the source files have a proper GPL header, as documented in the GPL
> licence "How to Apply These Terms to Your New Programs". Please notify
> upstream. I am uncertain as to whether this is a blocker or not.  

Upstream doesn't have any sort of bugtracking system or email contact I can find (other than directly emailing all six developers).

Comment 15 D Haley 2009-05-31 09:24:14 UTC
I recommend emailing the three lead devs, probably in the one email. Their addresses are on each of their profile sites. I'm sure they would be happy to help, considering the time it would have taken to write their system. Also their input here would clear up a few matters relatively quickly.

Comment 16 D Haley 2009-07-26 09:05:15 UTC
ping?

Comment 17 Conrad Meyer 2009-07-26 09:42:17 UTC
Pong. Haven't had time :(.

Comment 18 D Haley 2009-08-23 03:04:23 UTC
Hi Conrad,


I have notified upstream via email wrt. to the exit call. They now provide versioned tarballs at the following location:

http://www-lmc.imag.fr/CASYS/LOGICIELS/givaro/Downloads/:

====
Dear Prof. Roch and Gautier,

Recently givaro has been submitted for package review[1] for inclusion in the Fedora linux repositories. However small error was noticed during an automated source-code check. As a requirement of packaging, upstream (you) must be notified of this error. 

File src/kernel/zpz/givzpz16table1.C is included in a shared library (libgivaro.so), and Line 46 makes a call to the exit() function.

Calling exit() in a shared library results in incorrect de-allocation of system resources.

If it is possible to update this file in the next version of givaro to not use exit(), this would be most helpful.


[1] https://bugzilla.redhat.com/show_bug.cgi?id=475065

Kind Regards.

D. Haley

===

Comment 19 Conrad Meyer 2009-08-23 03:08:49 UTC
Hi, I also no longer have time for this package. If you would like to take it, you are quite welcome. Otherwise, it should be closed.

Comment 20 D Haley 2009-08-23 04:32:59 UTC
I'm happy to try to push this through

Comment 21 Conrad Meyer 2009-08-23 04:38:54 UTC
Ok, sounds good!

Comment 22 D Haley 2009-08-23 05:34:12 UTC
SPEC URL: http://dhd.selfip.com/427e/givaro.spec
SRPM URL: http://dhd.selfip.com/427e/givaro-3.2.15-0.1.rc1.fc10.src.rpm

F10: http://koji.fedoraproject.org/koji/taskinfo?taskID=1626651
F11: http://koji.fedoraproject.org/koji/taskinfo?taskID=1626660

rpmlint:
$ rpmlint `cat tmp`
givaro.i386: W: shared-lib-calls-exit /usr/lib/libgivaro.so.0.0.4 exit
givaro-devel.i386: W: no-documentation
givaro-devel.i386: E: non-executable-script /usr/share/givaro/givaro-makefile 0644
givaro-static.i386: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 3 warnings.
$ cat tmp
/home/makerpm/rpmbuild/SRPMS/givaro-3.2.15-0.1.rc1.fc10.src.rpm
/home/makerpm/rpmbuild/RPMS/i386/givaro-3.2.15-0.1.rc1.fc10.i386.rpm
/home/makerpm/rpmbuild/RPMS/i386/givaro-devel-3.2.15-0.1.rc1.fc10.i386.rpm
/home/makerpm/rpmbuild/RPMS/i386/givaro-static-3.2.15-0.1.rc1.fc10.i386.rpm
/home/makerpm/rpmbuild/RPMS/i386/givaro-debuginfo-3.2.15-0.1.rc1.fc10.i386.rpm

Notes:
*Upstream has been notified about GPL headers, and as above, exit() usage
*Tarball has been re-downloaded from new download area on givaro website
*makefile has been placed into %{_datadir}/%{name}/
*Trailing slashes have been added
*givaro-config.in has been patched to produce correct output in case of multiple flags

Comment 23 D Haley 2009-08-23 05:39:35 UTC
Whoops. Sorry for the noise, but I have fixed that E, with "sed -i 's|#! /bin/sh||'  $RPM_BUILD_ROOT%{_datadir}/%{name}/givaro-makefile" in my current spec.

Comment 24 Martin Gieseking 2009-09-08 08:20:41 UTC
Since you've already done a good job bringing this package in shape, there isn't much left to do. If nobody else wants to do the re-review, I can do it later today.
However, I'm not quite happy with the call of exit() in a shared library. This is simply bad style and should be avoided by all means. I'm not sure about Fedora's policy about this, though. 
Here is a patch that replaces the call of exit() by throwing an exception: 
http://mgieseki.fedorapeople.org/givaro/givaro-exit.patch

Comment 25 Susi Lehtola 2009-09-08 09:21:23 UTC
(In reply to comment #24)
> Since you've already done a good job bringing this package in shape, there
> isn't much left to do. If nobody else wants to do the re-review, I can do it
> later today.
> However, I'm not quite happy with the call of exit() in a shared library. This
> is simply bad style and should be avoided by all means. I'm not sure about
> Fedora's policy about this, though. 
> Here is a patch that replaces the call of exit() by throwing an exception: 
> http://mgieseki.fedorapeople.org/givaro/givaro-exit.patch  

Calling exit() is not forbidden (in fact it is quite common in scientific packages), so you can omit the rpmlint warning. You should not apply any patches that make functional changes, since that will break compatibility with upstream. You can ask upstream to make the change, though...

Comment 26 Martin Gieseking 2009-09-08 09:52:24 UTC
Actually, throwing an exception doesn't change the code functionality in a significant way. It still stops the application if it's not caught but provides better memory handling. 
To me, using exit() in shared libraries is pretty amateurish style, especially in combination with C++. I also haven't seen it in any of the scientific packages I use. But anyway, if it's OK to leave it in the package, I can live with it.

Comment 27 Martin Gieseking 2009-09-08 18:04:09 UTC
Here comes the review.

$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
givaro.x86_64: W: shared-lib-calls-exit /usr/lib64/libgivaro.so.0.0.4 exit.5
givaro-devel.x86_64: W: no-documentation
givaro-devel.x86_64: E: non-executable-script /usr/share/givaro/givaro-makefile 0644 /bin/sh
givaro-static.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 3 warnings.

- call of exit() can be ignored according to comment #25
- missing docs in -devel and -static is OK
- #!/bin/sh in makefile already removed according to comment #23


---------------------------------
keys used in following checklist:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - GPLv2 boilerplates missing (upstream notified) 

[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: files containing the text of the license(s) must be included in %doc.
    - COPYING added to %doc of base package

[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    - md5 hash is f0b754ee54bcac0e866f2979b57393ba

[+] MUST: The package MUST successfully compile and build into binary rpms.
    - builds in koji
      https://koji.fedoraproject.org/koji/taskinfo?taskID=1662967

[.] MUST: If the package does not successfully compile, ...
[+] MUST: All build dependencies must be listed in BuildRequires
[.] MUST: The spec file MUST handle locales properly.
    - no locales

[+] MUST: Packages containing .so files must call ldconfig in %post and %postun.
    - base package contains libgivaro.so.0.0.4
    - ldconfig called in %post and %postun

[.] MUST: If the package is designed to be relocatable, ...
    - package not relocatable

[+] MUST: A package must own all directories that it creates.
[+] MUST: A package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
    - givaro-makefile contains #!/bin/sh that must be removed
    - already done according to comment #23 

[+] MUST: %clean section must contain rm -rf %{buildroot}.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
    - no large docs

[+] MUST: Files in %doc must not affect the runtime of the application.
[+] MUST: Header files must be in a -devel package.
[+] MUST: Static libraries must be in a -static package.
    - libgivaro.a

[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
    - no .pc files

[+] MUST: .so without suffix must go in a -devel package.
    - libgivaro.so

[+] MUST: devel packages must require  %{name} = %{version}-%{release}
[+] MUST: Packages must NOT contain any .la libtool archives
    - .la file removed in %install

[.] MUST: Packages containing GUI applications must include a %{name}.desktop file
    - no GUI

[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[+] MUST: All filenames in rpm packages must be valid UTF-8.


[+] SHOULD: The reviewer should test that the package builds in mock.
    - builds in mock

[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
    - I tested some of the functions provided by the library and they worked as expected
    - givaro-config prints the correct flags
	 
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
    call of ldconfig in %post and %postun

[+] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
    - subpackage -static requires base and -devel package
    - Requires: %{name} = %{version}-%{release} is redundant

------------------------
The package is APPROVED.
------------------------

Comment 28 Susi Lehtola 2009-09-08 18:21:41 UTC
Umh..

src/kernel/zpz/StaticElement.h states it is under LGPLv2+, the rest of the files just say "see the copyright file" which doesn't exist. There is a COPYING that contains the GPLv3 license, thus the license tag should read GPL+.

https://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F

Comment 29 D Haley 2009-09-12 06:26:34 UTC
SPEC URL : http://dhd.selfip.com/427e/givaro-2.rc1.spec
SRPM URL : http://dhd.selfip.com/427e/givaro-3.2.15-0.2.rc1.fc10.src.rpm 


F10:http://koji.fedoraproject.org/koji/taskinfo?taskID=1672622
F11: http://koji.fedoraproject.org/koji/taskinfo?taskID=1672627

$ rpmlint `cat tmp | awk '{print $2}'`
givaro.i386: W: shared-lib-calls-exit /usr/lib/libgivaro.so.0.0.4 exit
givaro-devel.i386: W: no-documentation
givaro-static.i386: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 3 warnings.
$ cat tmp
Wrote: /home/makerpm/rpmbuild/SRPMS/givaro-3.2.15-0.2.rc1.fc10.src.rpm
Wrote: /home/makerpm/rpmbuild/RPMS/i386/givaro-3.2.15-0.2.rc1.fc10.i386.rpm
Wrote: /home/makerpm/rpmbuild/RPMS/i386/givaro-devel-3.2.15-0.2.rc1.fc10.i386.rpm
Wrote: /home/makerpm/rpmbuild/RPMS/i386/givaro-static-3.2.15-0.2.rc1.fc10.i386.rpm
Wrote: /home/makerpm/rpmbuild/RPMS/i386/givaro-debuginfo-3.2.15-0.2.rc1.fc10.i386.rpm

changelog:
* Sun Sep 12 2009 D Haley <mycae(a!t)yahoo.com> - 3.2.15-0.2.rc1
- Change to GPL+ from GPL2 per bugzilla comment

Comment 30 Martin Gieseking 2009-09-12 07:07:14 UTC
Sorry for the overlooked license ambiguity. Since this has been fixed, everything seems to be fine now.

Comment 31 D Haley 2009-09-12 08:01:54 UTC
New Package CVS Request
=======================
Package Name: givaro
Short Description: C++ library for arithmetic and algebraic computations
Owners: mycae
Branches: F-10 F-11 EL-5
InitialCC: baz

Comment 32 Kevin Fenzi 2009-09-14 04:56:30 UTC
cvs done.

Comment 33 Fedora Update System 2009-09-14 10:30:55 UTC
givaro-3.2.15-0.2.rc1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/givaro-3.2.15-0.2.rc1.fc11

Comment 34 Fedora Update System 2009-09-14 10:31:03 UTC
givaro-3.2.15-0.2.rc1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/givaro-3.2.15-0.2.rc1.fc10

Comment 35 Fedora Update System 2009-09-15 07:43:21 UTC
givaro-3.2.15-0.2.rc1.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 givaro'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-9573

Comment 36 Fedora Update System 2009-09-15 07:44:24 UTC
givaro-3.2.15-0.2.rc1.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 givaro'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-9580

Comment 37 Fedora Update System 2009-10-03 18:55:09 UTC
givaro-3.2.15-0.2.rc1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 38 Fedora Update System 2009-10-03 19:06:06 UTC
givaro-3.2.15-0.2.rc1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.