[Bug 739398] Review Request: openblas - An optimized BLAS library based on GotoBLAS2

[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=739398

--- Comment #6 from Jussi Lehtola <jussi.lehtola at iki.fi> 2011-10-05 18:38:54 EDT ---
(In reply to comment #5)
> The gcc command which the library is linked with lacks -Wl,-soname=,
> hence the no-soname warning from rpmlint. I think this must be fixed.

I don't think the missing soname is a big issue, since the BLAS/LAPACK API has
stabilized a *long* time ago.

I have reported the lack of soversioning upstream before asking for the review.

> Fixing this properly might also require modifying the %files lists
> apart from patching the Makefiles.

Sure. But I don't want to add soversions myself; that's the job of upstream.

> The changes from GotoBLAS2 are mainly added support for the Chinese
> Loongson CPU and some superficial changes like minor build system tweaks,
> renamed files, new name and added copyright/license texts.

Yes, I'd think the major part of the code is straight from GotoBLAS2, which was
non-free for a long time. GotoBLAS is, however, dead nowadays, so packaging
OpenBLAS seems a lot more sane.

> Bundles lapack-3.1.1 sources -> investigate unbundling, if sources are
> necessary to build, contact lapack maintainer to add a -source subpackage
> and BuildRequire it.
> 
> Bundles some files from lapack-3.0 sources, at least lapack/getri/*.f.

Maybe I'll need to ask for an exception. A generic -source package is not
enough, since the build scripts assume a specific version.

Although, I see that this problem is solved in ATLAS by just BR'ing the static
version of the LAPACK libraries. Maybe the same thing could be done with
OpenBLAS as well.

> Bundles http://www.netlib.org/blas/blast-forum/cblas.tgz:CBLAS/testing
> directory as ctest/ and (modified) test/. Also cblas.h is basically
> the same, only reformatted and with parameter names changed.

Since OpenBLAS provides CBLAS functions, the headers have to be duplicated
anyhow.

> Bundles a modified http://www.netlib.org/blas/blas.tgz:BLAS directory
> as reference/.

.. although it's not used anyhow; it's only used when a cross-check against the
reference implementation is requested.

> Minimum requirements for Fedora are still Pentium Pro or newer.
> Will this run on a Pentium Pro?

Judging from GotoBLAS_01Readme.txt, the minimum CPU is Pentium3 or Athlon.
If someone still runs older systems, they can use ATLAS instead.

It would be of course possible to limit this package to only, say, x86_64
architecture, where it will run on every system.

> The comment seems wrong. Also, is that the only way to remove executable stack?

Good catch.

Yes, I think so. Gcc creates executable stacks whenever there's an assembly
section without a GNU-stack note. And these aren't portable, they're not often
used.

> rm -rf %{buildroot} in %install and the %clean section are only necessary for
> EPEL.

.. where this package will also be useful.

> There are no docs included in %files. I'd suggest at least these:

Added.

http://theory.physics.helsinki.fi/~jzlehtol/rpms/openblas.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/openblas-0.1-2.alpha2.4.fc15.src.rpm

-- 
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.