Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verify if cblas.pc and lapack.pc should be replaced by links to openblas.pc #29071

Closed
dimpase opened this issue Jan 23, 2020 · 100 comments
Closed

Comments

@dimpase
Copy link
Member

dimpase commented Jan 23, 2020

currently openblas's spkg-configure.pc unconditionally makes cblas.pc and lapack.pc copies of openblas.pc, which in some cases if incorrect, e.g. Arch Linux has libcblas linked to libopenblas
and containing stuff missing in libopenblas, resulting in errors.

this has been reported on sage-devel
https://groups.google.com/d/msg/sage-devel/pIOnFyFJMtM/_FbzM2OxCQAJ

So we should make these installations conditional.

Depends on #29051

CC: @isuruf @mkoeppe @antonio-rojas

Component: build: configure

Author: Dima Pasechnik

Branch/Commit: 3a4524e

Reviewer: Isuru Fernando, Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/29071

@dimpase dimpase added this to the sage-9.1 milestone Jan 23, 2020
@dimpase
Copy link
Member Author

dimpase commented Jan 23, 2020

comment:1

It depends on the fortran name mangling. For gfortran, one symbol would be dgeqrf_.

we can do an AC_LINK_IFELSE() test with a subroutine dgeqef call,
and try to link it against libopenblas. this would be independent of Fortran compiler.

http://www.netlib.org/lapack/explore-html/df/dc5/group__variants_g_ecomputational_ga3766ea903391b5cf9008132f7440ec7b.html

@dimpase
Copy link
Member Author

dimpase commented Jan 23, 2020

comment:2

And for cblas we can do AC_CHECK_LIB() on cblas_dgemm, as Antonio proposed.

@isuruf
Copy link
Member

isuruf commented Jan 23, 2020

comment:3

You can also use AC_FC_FUNC to get the mangled name.

@dimpase
Copy link
Member Author

dimpase commented Jan 23, 2020

comment:4

Replying to @isuruf:

You can also use AC_FC_FUNC to get the mangled name.

OK, that's better, thanks.

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

Branch: u/dimpase/packages/openblaspcfix

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:5

needs testing on Arch in particular.


New commits:

ccc5d82install cblas.pc and lapack.pc links conditionally

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

Commit: ccc5d82

@videlec
Copy link
Contributor

videlec commented Jan 24, 2020

comment:6

trying now on arch

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

f6714fbuse AC_SEARCH_LIBS

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2020

Changed commit from ccc5d82 to f6714fb

@videlec
Copy link
Contributor

videlec commented Jan 24, 2020

comment:8

With the branch applied, I end up with the same permission issues as with ./configure --without-system-openblas. That is to say

Copying package files from temporary location /opt/sage/local/var/tmp/sage/build/openblas-0.3.6.p0/inst to /opt/sage/local
cp: cannot create regular file '/opt/sage/local/./lib/pkgconfig/blas.pc': Permission denied
cp: cannot create regular file '/opt/sage/local/./lib/pkgconfig/cblas.pc': Permission denied
cp: cannot create regular file '/opt/sage/local/./lib/pkgconfig/lapack.pc': Permission denied

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:9

Is /opt/sage/local your $SAGE_LOCAL?
If so, could you try rm -rf /opt/sage/local/pkgconfig/ ?

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:10

sorry, I meant rm -rf /opt/sage/local/lib/pkgconfig/

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:11

even more precisely, rm -f /opt/sage/local/lib/pkgconfig/*blas.pc && rm -f /opt/sage/local/lib/pkgconfig/lapack.pc

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:12

I presume you get permission problems while running make. This is due to #29003 - where the installation of these is done somewhat carelessly.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2020

Changed commit from f6714fb to 7ac1293

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

7ac1293added missing module names - cblas and lapack

@videlec
Copy link
Contributor

videlec commented Jan 24, 2020

comment:14

When I do make distclean it does remove local entirely. At which step do you want me to try rm XXX?

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:15

the logic of this branch is still not 100% correct - if we end up installing OpenBLAS we should not install any *.pc files, i.e. should not run any AC_CONFIG_LINKS.

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:16

Replying to @videlec:

When I do make distclean it does remove local entirely. At which step do you want me to try rm XXX?

these rm from comment:11 should be run after ./configure and before make, assuming I understand what's going on.

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:17

By the way, please pull the branch again - comment:13 is rather important for Arch.

@videlec
Copy link
Contributor

videlec commented Jan 24, 2020

comment:18

After ./configure there is no directory local/lib/pkgconfig/

$ ls -R local/
local/:
bin  etc  include  lib  lib64  share  var

local/bin:

local/etc:

local/include:

local/lib:

local/share:

local/var:
lib

local/var/lib:
sage

local/var/lib/sage:
installed

local/var/lib/sage/installed:

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:19

local/lib/pkgconfig/ may or may not be present, it depends upon the state of the system.
(e.g. if you did make distclean before ./configure you won't have
local/ at all)

Now, what happens if you run make? Do you still get the same error?

@dimpase
Copy link
Member Author

dimpase commented Jan 24, 2020

comment:20

It seems I got Arch running in an lxc container, so I guess I can polish it on my own.

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

comment:72

I also don't get

+      # We already checked (in the initial portion of configure.ac)
+      # whether the Fortran compiler accepts free-format source code (as

IMHO there is nothing either in configure.ac or build/pkgs/gcc/spkg-configure.m4 that
does any Fortran checks.

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

comment:73

Replying to @dimpase:

+       if test "$SAGE_HAVE_FC_FREEFORM" = yes ; then dnl Have a suitable Fortran compiler

could we please stick to AS_IF ?

actually, I don't see the point of this if at all. If we get there then it's already true, as it's inside the check for gfortran as a pre-req, in the scope of SAGE_SPKG_DEPCHECK([gfortran], ...).
`

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

comment:74

The trouble is in AC_FC_FUNC(), which throws a fit if there is no Fortran compiler around, instead of just suffering in silence.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8cf2a3bconfigure.ac: Check some FC properties earlier
c98c6b2configure.ac, build/pkgs/{openblas,gfortran}: Do not fail if there is no FC

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2020

Changed commit from d2b5e13 to c98c6b2

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:76

Replying to @dimpase:

I also don't get

+      # We already checked (in the initial portion of configure.ac)
+      # whether the Fortran compiler accepts free-format source code (as

IMHO there is nothing either in configure.ac or build/pkgs/gcc/spkg-configure.m4 that
does any Fortran checks.

Sorry, I forgot to include one commit on this branch.
Fixed.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:77

Replying to @dimpase:

The trouble is in AC_FC_FUNC(), which throws a fit if there is no Fortran compiler around, instead of just suffering in silence.

Yes, exactly, and more specifically: Because of AC_REQUIRE and AC_DEFUN_ONCE one cannot easily make all things conditional that one would liike.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:78

Replying to @dimpase:

Replying to @dimpase:

+       if test "$SAGE_HAVE_FC_FREEFORM" = yes ; then dnl Have a suitable Fortran compiler

could we please stick to AS_IF ?

No, they are unfortunately not equivalent because of autoconf's AC_PREREQ/AC_DEFUN_ONCE magic.

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

New commits:

39a317dinstall cblas.pc and lapack.pc links conditionally
7adc712use AC_SEARCH_LIBS
abda83eadded missing module names - cblas and lapack
683a926make installing *.pc files conditional
a0acbcbcorrect quoting of m4 index variable
5ee6844ensure gfortran is available
56541aewrap AC_FC_FUNC so that it does not throw an error without Fortran
3a4524eMerge remote-tracking branch 'trac/public/packages/numpy/no_DEFAULT_and_ALL_numpy_site_cfg' into openblaspcfix

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

Changed commit from c98c6b2 to 3a4524e

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

comment:80

Replying to @mkoeppe:

I don't like the idea of requiring a Fortran compiler to even run ./configure successfully.

Fixed by wrap AC_FC_FUNC so that it does not throw an error without Fortran: 56541ae

@dimpase
Copy link
Member Author

dimpase commented Jan 28, 2020

comment:81

IMHO it's an autoconf bug/feature that AC_FC_FUNC errors out without Fortran, while e.g. AC_FC_FREEFORM does not.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:82

Replying to @dimpase:

IMHO it's an autoconf bug/feature that AC_FC_FUNC errors out without Fortran, while e.g. AC_FC_FREEFORM does not.

Yes, I agree.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:83

Replying to @dimpase:

Replying to @mkoeppe:

I don't like the idea of requiring a Fortran compiler to even run ./configure successfully.

Fixed by wrap AC_FC_FUNC so that it does not throw an error without Fortran: 56541ae

Great, a much easier fix than what I put together in ​8cf2a3b/​c98c6b2.
Testing at https://github.com/mkoeppe/sage/actions/runs/32186055

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:84

Tests finished at https://github.com/mkoeppe/sage/actions/runs/32186055

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 28, 2020

comment:85

Looking good, and certainly fixes what was original reported on this ticket (fflas_ffpack on archlinux). The build error of fflas_ffpack on debian-jessie-standard (https://github.com/mkoeppe/sage/runs/412891766) is unrelated. It is good on all other configurations.

@vbraun
Copy link
Member

vbraun commented Jan 31, 2020

Changed branch from u/dimpase/packages/openblaspcfix to 3a4524e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants