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@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@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 thisout (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 somepreprocessor 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!