[Bug 475065] Review Request: givaro - C++ library for arithmetic and algebraic computations

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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





--- Comment #10 from Conrad Meyer <konrad at tylerc.org>  2009-05-30 23:41:12 EDT ---
(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 at GLIBC_2.0
> 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!

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.