Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: m4ri - Linear Algebra over F_2
https://bugzilla.redhat.com/show_bug.cgi?id=470173
Summary: Review Request: m4ri - Linear Algebra over F_2 Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: konrad@tylerc.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://konradm.fedorapeople.org/fedora/SPECS/m4ri.spec SRPM URL: http://konradm.fedorapeople.org/fedora/SRPMS/m4ri-20081028-1.fc9.src.rpm Description: M4RI is a library for fast arithmetic with dense matrices over F_2. The name M4RI comes from the first implemented algorithm: The "Method of the Four Russians" inversion algorithm published by Gregory Bard. M4RI is used by the Sage mathematics software and the PolyBoRi library.
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=470173
--- Comment #1 from Jason Tibbitts tibbs@math.uh.edu 2008-11-06 13:19:17 EDT --- The header files seem to be in the main package. Is this intentional?
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=470173
--- Comment #2 from Conrad Meyer konrad@tylerc.org 2008-11-06 13:50:23 EDT --- Er, nope. Fixing.
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=470173
--- Comment #3 from Conrad Meyer konrad@tylerc.org 2008-11-06 13:57:16 EDT --- Fixed in:
http://konradm.fedorapeople.org/fedora/SPECS/m4ri.spec http://konradm.fedorapeople.org/fedora/SRPMS/m4ri-20081028-2.fc9.src.rpm
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=470173
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Blocks| |182235 AssignedTo|nobody@fedoraproject.org |tibbs@math.uh.edu Flag| |fedora-review?
--- Comment #4 from Jason Tibbitts tibbs@math.uh.edu 2008-11-07 19:14:44 EDT --- I'm seeing several issues with this package.
The COPYING file contains v2 of the GPL, but the code itself does not specify a version. Their web site indicates GPLv2+, but I do not know if the web site is a sufficient statement of intent. According to the licensing FAQ, the web site isn't consulted about this. Blocking FE-Legal for a ruling.
The "testsuite" directory would seem to include a test suite. Is it possible to run it at build time? If so, it needs to be run unless there's a compelling reason not to do so.
A shared library is installed but ldconfig is not called. In addition, this package seems to have a rather odd library versioning convention. The usual method is to have the library version after the ".so" but this package has it before. I'm afraid I don't understand why it would be doing this differently than almost all other libraries.
The static library is not permitted in the -devel package. http://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Librarie... This is the "static libraries and shared libraries" case.
rpmlint says only: m4ri-devel.x86_64: W: no-documentation which is fine.
* source files match upstream: 6e30b50b74c72ceca431461d471e38f682d7a6ad1c2d07db28806fff1d3e30e8 m4ri-20081028.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 has acceptable complaints. * final provides and requires are sane: m4ri-20081028-2.fc10.x86_64.rpm libm4ri-0.0.20081029.so()(64bit) m4ri = 20081028-2.fc10 m4ri(x86-64) = 20081028-2.fc10 =
m4ri-devel-20081028-2.fc10.x86_64.rpm m4ri-static = 20081028-2.fc10 m4ri-devel = 20081028-2.fc10 m4ri-devel(x86-64) = 20081028-2.fc10 = libm4ri-0.0.20081029.so()(64bit) m4ri = 20081028-2.fc10
X %check is not present but a test suite seems to exist. X shared libraries are installed but ldconfig is not called. * 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. * headers are in the -devel package. * no pkgconfig files. X static libraries are in the -devel package. * no libtool .la files.
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=470173
--- Comment #5 from Conrad Meyer konrad@tylerc.org 2008-11-07 19:56:46 EDT --- (In reply to comment #4)
I'm seeing several issues with this package.
The COPYING file contains v2 of the GPL, but the code itself does not specify a version. Their web site indicates GPLv2+, but I do not know if the web site is a sufficient statement of intent. According to the licensing FAQ, the web site isn't consulted about this. Blocking FE-Legal for a ruling.
Right, I believed I marked the package GPLv2+ accordingly, but it'd be good to hear from Legal.
The "testsuite" directory would seem to include a test suite. Is it possible to run it at build time? If so, it needs to be run unless there's a compelling reason not to do so.
No. Or at least when I tried running them, it didn't work. (It tries to build the tests using system libraries, without those in the buildroot. Is there any easy fix for that?)
A shared library is installed but ldconfig is not called. In addition, this package seems to have a rather odd library versioning convention. The usual method is to have the library version after the ".so" but this package has it before. I'm afraid I don't understand why it would be doing this differently than almost all other libraries.
Beats me.
The static library is not permitted in the -devel package. http://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Librarie... This is the "static libraries and shared libraries" case.
Alright.
? license field matches the actual license. X %check is not present but a test suite seems to exist. X shared libraries are installed but ldconfig is not called. X static libraries are in the -devel package.
Added ldconfig, moved static libraries to -static subpackage.
New spec/srpm: http://konradm.fedorapeople.org/fedora/SPECS/m4ri.spec http://konradm.fedorapeople.org/fedora/SRPMS/m4ri-20081028-3.fc9.src.rpm
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=470173
--- Comment #6 from Jason Tibbitts tibbs@math.uh.edu 2008-11-09 23:21:56 EDT --- Note that if we only trust the code and docs in the tarball, the license is GPL+ as far as I can tell. If we additionally consult the web site, its GPLv2+. Unfortunately we have to be precise here. Ultimately, clarification from upstream is the best step. An email from them is sufficient; a fixed tarball is ideal but not necessary. Otherwise we'll wait to see what the legal folks have to say.
The new package builds fine; rpmlint spews a no-documentation complaint about the -static package but that's nothing to worry about.
About the library versioning thing, my concern is that something built against this package will end up needing -devel installed at runtime because the linker won't understand the different versioning convention and will end up with a dependency on libm4ri.so instead of libm4ri-0.0.20081029.so. This should be relatively easy to verify if you have some software which uses this library around to check.
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=470173
--- Comment #7 from Conrad Meyer konrad@tylerc.org 2008-11-10 03:39:03 EDT --- (In reply to comment #6)
Note that if we only trust the code and docs in the tarball, the license is GPL+ as far as I can tell. If we additionally consult the web site, its GPLv2+. Unfortunately we have to be precise here. Ultimately, clarification from upstream is the best step. An email from them is sufficient; a fixed tarball is ideal but not necessary. Otherwise we'll wait to see what the legal folks have to say.
Is an email from them much better than the front page of their website proclaiming GPLv2+? I've sent the maintainer an email about it anyways and await a reply.
The new package builds fine; rpmlint spews a no-documentation complaint about the -static package but that's nothing to worry about.
About the library versioning thing, my concern is that something built against this package will end up needing -devel installed at runtime because the linker won't understand the different versioning convention and will end up with a dependency on libm4ri.so instead of libm4ri-0.0.20081029.so. This should be relatively easy to verify if you have some software which uses this library around to check.
Sorry, I don't have any software around using this library. The goal is to eventually get Sage itself packaged properly, and this is one of the subprojects it encompasses.
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=470173
--- Comment #8 from Conrad Meyer konrad@tylerc.org 2008-11-10 04:51:17 EDT --- Alright, I got a reply from the current maintainer. He has this to say:
""" Hi,
sorry for the mess. The software is definitely GPLv2+. The discrepancy in the source files is due to my inability to copy'n'paste the right legal blah. I can provide a new tarbar with updated legal stuff in the source files if needed.
About:
In addition, this package seems to have a rather odd library versioning convention. The usual method is to have the library version after the ".so" but this package has it before. I'm afraid I don't understand why it would be doing this differently than almost all other libraries.
This difference is again not intentional. I though autotools would just do the right thing (tm) but somewhere on the line I seem to screw up setting that up. I will try to figure out where it went wrong and fix it somewhen in the near future. """
In other words, upstream is alive and well and is willing to work with us to get things done properly.
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=470173
--- Comment #9 from Jason Tibbitts tibbs@math.uh.edu 2008-11-10 09:04:18 EDT --- My understa(In reply to comment #7)
Is an email from them much better than the front page of their website proclaiming GPLv2+?
Yes, unless you have some sort of copyright statement on the web site indicating that its authorship is the same as the software. I won't pretend to speak for the lawyers, but as I understand things the idea is to discern the intent of the copyright holders of the software. An email, while obviously not falsifiable by any stretch of the imagination, has been deemed sufficient for this purpose. Of course, that email must be included with the software as documentation.
Sorry, I don't have any software around using this library.
Well, then your options are:
* Find some way to test it by linking some code against it and seeing what happens.
* Talk to someone who knows enough about the linker to answer the question.
* Wait for upstream to change things.
* Simply refuse to do one of the above and I'll return this ticket to the queue; perhaps someone will approve it over my objections. Let me know if this is what you prefer.
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=470173
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tcallawa@redhat.com Blocks|182235 |
--- Comment #10 from Tom "spot" Callaway tcallawa@redhat.com 2008-11-10 12:47:34 EDT --- When no other information is available, and we have a high degree of confidence that the website is maintained/created by the copyright holder of the licensed, we can use it as licensing intent (or versioning intent).
It's always preferrable to get upstream to clarify this specifically, which you've done. Ideally, you'll want to get upstream to specifically note this in the source tarball, with proper code attribution (see: http://www.fsf.org/licensing/licenses/gpl-howto.html , search for "copying permission statement").
Lifting FE-Legal.
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=470173
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin@tigcc.ticalc.org
--- Comment #11 from Kevin Kofler kevin@tigcc.ticalc.org 2008-11-10 19:20:16 EDT --- What will actually be required at runtime depends on what the DT_SONAME entry for the library is. If it includes the version, it's right, if it doesn't, it needs to be fixed.
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=470173
--- Comment #12 from Kevin Kofler kevin@tigcc.ticalc.org 2008-11-10 19:22:05 EDT --- Oh, and to figure it out: readelf -d yourlibrary.so | grep SONAME or: eu-readelf -d yourlibrary.so | grep SONAME (readelf is part of binutils, eu-readelf of elfutils.)
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=470173
--- Comment #13 from Kevin Kofler kevin@tigcc.ticalc.org 2008-11-10 19:27:32 EDT --- As for some programs to test the library with, try running make in its testsuite folder. Then use readelf -d test_multiplication.c | grep NEEDED (or eu-readelf) to check what it's requiring.
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=470173
Conrad Meyer konrad@tylerc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |182235
--- Comment #14 from Conrad Meyer konrad@tylerc.org 2008-11-10 19:44:07 EDT --- For example:
$ readelf -d test_multiplication | grep NEEDED 0x0000000000000001 (NEEDED) Shared library: [libm4ri-0.0.20081029.so] $ readelf -d /usr/lib64/libm4ri-0.0.20081029.so |grep SONAME 0x000000000000000e (SONAME) Library soname: [libm4ri-0.0.20081029.so]
On my x86_64 system.
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=470173
Kevin Kofler kevin@tigcc.ticalc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|182235 |
--- Comment #15 from Kevin Kofler kevin@tigcc.ticalc.org 2008-11-10 19:49:39 EDT --- FE-Legal was already lifted by spot, why did you readd it?
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=470173
--- Comment #16 from Kevin Kofler kevin@tigcc.ticalc.org 2008-11-10 19:50:40 EDT --- The output from comment #14 looks OK to me, the versioning system is working properly.
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=470173
--- Comment #17 from Conrad Meyer konrad@tylerc.org 2008-11-10 19:52:53 EDT --- Did I re-add it? I didn't intend to.
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=470173
Conrad Meyer konrad@tylerc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(tibbs@math.uh.edu | |)
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=470173
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(tibbs@math.uh.edu | |) |
--- Comment #18 from Jason Tibbitts tibbs@math.uh.edu 2008-11-26 23:15:39 EDT --- Did I miss a comment from you which answered my question? I was going to set needinfo to you.
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=470173
--- Comment #19 from Kevin Kofler kevin@tigcc.ticalc.org 2008-11-26 23:18:50 EDT --- What question? The library versioning issue? Is the evidence provided by Conrad and me not enough to prove that it's working fine?
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=470173
--- Comment #20 from Jason Tibbitts tibbs@math.uh.edu 2008-12-02 10:34:44 EDT --- I don't know; I'm not going to pretend that I've understood what comment #14 means, and I haven't seen an explicit comment from the submitter of this ticket directly answering the issue I raised in the initial review regarding whether or not the -devel package ends up being required at runtime. All I got was "Beats me" and then several comments regarding DT_SONAME which I might research more deeply if I didn't have 30 other review tickets in flight at the moment.
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=470173
--- Comment #21 from Conrad Meyer konrad@tylerc.org 2008-12-02 11:31:05 EDT --- The -devel package does not end up being required at runtime. The .so required at runtime is the one in %{_libdir} and the above comments agree.
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=470173
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #22 from Jason Tibbitts tibbs@math.uh.edu 2008-12-02 18:20:33 EDT --- OK, in that case I think this is fine. It would still be good to include that email from upstream with the license information as documentation, but you can do that at your leisure.
APPROVED
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=470173
Conrad Meyer konrad@tylerc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #23 from Conrad Meyer konrad@tylerc.org 2008-12-02 18:32:18 EDT --- I'll include the email before import. Thanks very much for the review!
New Package CVS Request ======================= Package Name: m4ri Short Description: Linear Algebra over F_2 Owners: konradm Branches: F-10 F-9 InitialCC:
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=470173
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #24 from Kevin Fenzi kevin@tummy.com 2008-12-03 20:00:28 EDT --- cvs done.
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=470173
Conrad Meyer konrad@tylerc.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
--- Comment #25 from Conrad Meyer konrad@tylerc.org 2008-12-04 13:54:23 EDT --- http://koji.fedoraproject.org/koji/taskinfo?taskID=977087
Built for rawhide. Closing.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla