Spec URL: http://jjames.fedorapeople.org/fflas-ffpack/fflas-ffpack-devel.spec SRPM URL: http://jjames.fedorapeople.org/fflas-ffpack/fflas-ffpack-devel-1.4.1-1.fc14.src.rpm Description: The FFLAS-FFPACK library provides functionality for dense linear algebra over word size prime finite fields. This package is required for the latest version of linbox.
Just a quick note for now: Wouldn't it better to have an empty fflas-ffpack package with a devel subpackage? You never know, what upstream does next and if you need a fflas-ffpack-whatever package... The other hacks seem reasonable. Is the wrong address known for upstream?
Good point. Here are new URLs: http://jjames.fedorapeople.org/fflas-ffpack/fflas-ffpack.spec http://jjames.fedorapeople.org/fflas-ffpack/fflas-ffpack-1.4.1-1.fc14.src.rpm As for the address, no, I have not reported that upstream yet. I will do that today. Thanks!
Review: Good: - %files ok - %install ok - parallel make used - %build maybe ok (see below for givaro problem) - %prep ok - BR ok - no libs installed no postun etc needed - no static libs - no *la - hacks look needed/ok - source match upstrem: ff3755ba79b622e9c37cc14818213eb5 fflas-ffpack-1.4.1.tar.gz - license wrong: /usr/bin/fflasffpack-config is CeCILL-B -> LGPLv2+ and CeCILL-B with a comment, which file is under which license TODO: * Please add a %check section, e.g. with "make check" * I'm seeing: ******************************************************************************* WARNING: GIVARO not found! GIVARO version 3.4.0 or greater (<3.5) is required for some tests in this library. Please make sure GIVARO is installed and specify its location with the option --with-givaro=<prefix> when running configure. ******************************************************************************* -> A givaro update would be needed for this to solve (don't know what exactly is now broken in this package, maybe you?) * a doc subpackage would be really nice: $ du -sh /usr/share/doc/fflas-ffpack-devel-1.4.1/ /usr/include/fflas-ffpack/ 3,7M /usr/share/doc/fflas-ffpack-devel-1.4.1/ 596K /usr/include/fflas-ffpack/ * missing requires: $ rpm -qa --requires fflas-ffpack-devel rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(PayloadIsXz) <= 5.2-1 And this file: /usr/include/fflas-ffpack/utils/args-parser.h:#include "linbox/util/commentator.h" -> R on linbox-devel is missing * There are other files with wrong FSF address, but I wouldn't fix this, just reporting upstream...: licensecheck -r /usr/include/fflas* | grep incorrect /usr/include/fflas-ffpack/utils/Matio.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/utils/debug.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/fflas-ffpack.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/field/modular-balanced-int32.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/field/unparametric.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/field/modular-int32.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/field/modular-balanced-int64.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/field/modular-int64.h: LGPL (v2 or later) (with incorrect FSF address) /usr/include/fflas-ffpack/config-blas.h: LGPL (v2 or later) (with incorrect FSF address)
I fixed *some* of the problems you pointed out. See below for more information. New URLs: http://jjames.fedorapeople.org/fflas-ffpack/fflas-ffpack.spec http://jjames.fedorapeople.org/fflas-ffpack/fflas-ffpack-1.4.1-2.fc14.src.rpm (In reply to comment #3) > - license wrong: /usr/bin/fflasffpack-config is CeCILL-B > -> LGPLv2+ and CeCILL-B with a comment, which file is under which license Fixed. Good catch! > TODO: > * Please add a %check section, e.g. with "make check" > * I'm seeing: > ******************************************************************************* > WARNING: GIVARO not found! > > GIVARO version 3.4.0 or greater (<3.5) is required for some tests in this > library. > Please make sure GIVARO is installed and specify its location with the > option --with-givaro=<prefix> when running configure. > ******************************************************************************* > -> A givaro update would be needed for this to solve (don't know what exactly > is now broken in this package, maybe you?) Yes. I have updates ready for givaro and linbox once fflas-ffpack is ready. So "make check" is not going to work for fflas-ffpack until the givaro update is stable. So I think the sequence has to go like this: - New fflas-ffpack package without %check goes stable - Updated givaro and linbox packages are built and go stable - The fflas-ffpack package is updated with a %check script But that's going to break everytime we upgrade, isn't it? There's an alternative. See below. > * a doc subpackage would be really nice: Done. > * missing requires: > $ rpm -qa --requires fflas-ffpack-devel > rpmlib(CompressedFileNames) <= 3.0.4-1 > rpmlib(FileDigests) <= 4.6.0-1 > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rpmlib(PayloadIsXz) <= 5.2-1 > And this file: > /usr/include/fflas-ffpack/utils/args-parser.h:#include > "linbox/util/commentator.h" > -> R on linbox-devel is missing This is a problem. Since linbox BRs fflas-ffpack, this means that when building linbox we get linbox -> fflas-ffpack-devel -> linbox-devel. That is, the old linbox-devel will have to be installed to build the new linbox. Plus, linbox-devel has to Require fflas-ffpack-devel anyway, due to includes going in the opposite direction. More below. > * There are other files with wrong FSF address, but I wouldn't fix this, just > reporting upstream...: > licensecheck -r /usr/include/fflas* | grep incorrect It's easily fixed. Done, and reported upstream as well. I suspect we should rethink submitting this as a separate package. The problems with %check and requiring linbox-devel arise from the incestuous relationship between fflas-ffpack, givaro, and linbox. Plus, given upstream's penchant for breaking backward compatibility with every release, I see trouble ahead. All of this could be resolved by adding the fflas-ffpack source tarball as a second source file in the linbox spec, and building them together. We'd still have the problem of the circular dependency between linbox-devel and fflas-ffpack-devel; I don't know how to resolve that. Perhaps combine them both into linbox-devel, and make it Provides: fflas-ffpack / fflas-ffpack-devel. The downside to that approach is that it is no longer possible to update linbox and fflas-ffpack independently. But I don't think that is possible anyway. What do you think?
Well, because both are independent projects, they should be two separate packages, isn't it? But I surely understand the trouble with updating both packages independently, because I don't believe upstream will have nicely building packages. How about trying to build the new givaro, then fflas-ffpack and then the new linbox locally, and if it works as expected, add this package separately? I don't quite see, how all in one source package would solve this more easily... Then you would also need to build one after the other and could always run into problems. Maybe building fflas-ffpack without linbox support would work, then build linbox against fflas-ffpack and then add linbox support (if there is any). Right now, linbox is only needed in an utils header file with ifdef's around, so I don't see a problem, when no linbox is around. Where do you think is BR: linbox needed? I only see a R on linbox...
(In reply to comment #5) > Well, because both are independent projects, they should be two separate > packages, isn't it? They should, yes. Let's continue on that path. > How about trying to build the new givaro, then fflas-ffpack and then the new > linbox locally, and if it works as expected, add this package separately? I have done that. I've got a working linbox on my machine, with the fflas-ffpack from comment 4, and an updated givaro. > Maybe building fflas-ffpack without linbox support would work, then build > linbox against fflas-ffpack and then add linbox support (if there is any). > Right now, linbox is only needed in an utils header file with ifdef's around, > so I don't see a problem, when no linbox is around. > > Where do you think is BR: linbox needed? I only see a R on linbox... I agree it's an R. What I meant is that if linbox BRs fflas-ffpack-devel, and fflas-ffpack-devel Rs linbox-devel, then koji will have to install both fflas-ffpack-devel and the previous version of linbox-devel in order to build a new linbox. Because of the ifdefs you noted, I don't think fflas-ffpack-devel should *Require* linbox-devel, anyway. In fact, the Requires will have to go the other way; linbox-devel must R fflas-ffpack-devel. Are you okay with the plan I gave for adding a %check section? It can't be there on the initial import, but once givaro and linbox are updated, I can go back and update fflas-ffpack to add the %check section.
Thomas, what else do I need to do to move this forward? Thanks.
Sorry, I somehow missed comment #6... Thanks for the ping. I'm ok with your "Requires will have to go the other way; linbox-devel must R fflas-ffpack-devel". Please make a note in the spec file about this, so nobody wonders about the decision to leave R on linbox-devel away. Adding the %check section now doesn't hurt. It seems, some tests are not running, right now, but are also not failing: """ GIVARO version 3.4.0 or greater (<3.5) is required for some tests in this library. """ -> """ PASS: test-charpoly PASS: test-compressQ ================== All 2 tests passed ================== """ But if you want to add them later, it's fine too (adding them now, is only a "SHOULD" on my review list :) ) ##################################################################### APPROVED
No problem. Obviously I've been busy myself, given how many days it has taken me to find out that you approved the package. :-) Okay, I'll add that note to the spec file and turn on the checks. Thanks for your help!
New Package SCM Request ======================= Package Name: fflas-ffpack Short Description: Finite field linear algebra subroutines Owners: jjames Branches: f15 InitialCC:
Git done (by process-git-requests).
fflas-ffpack-1.4.1-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/fflas-ffpack-1.4.1-2.fc15
fflas-ffpack-1.4.1-2.fc15 has been pushed to the Fedora 15 testing repository.
fflas-ffpack-1.4.1-2.fc15 has been pushed to the Fedora 15 stable repository.