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

Detect and use the system fflas-ffpack #29453

Closed
orlitzky opened this issue Apr 2, 2020 · 27 comments
Closed

Detect and use the system fflas-ffpack #29453

orlitzky opened this issue Apr 2, 2020 · 27 comments

Comments

@orlitzky
Copy link
Contributor

orlitzky commented Apr 2, 2020

This package already has an spkg-configure.m4 file and some distro information, but we can't yet detect and use the system copy. Here's hoping everyone ships the pkg-config file.

CC: @embray @dimpase @mkoeppe @kiwifb

Component: build: configure

Author: Michael Orlitzky

Branch/Commit: bdac4ba

Reviewer: Matthias Koeppe

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

@orlitzky orlitzky added this to the sage-9.1 milestone Apr 2, 2020
@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 2, 2020

Commit: 5ce8498

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 2, 2020

Branch: u/mjo/ticket/29453

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 2, 2020

comment:1

Here's a go at this. I picked the lower version bound based on what Debian has on repology.


New commits:

4cada3fTrac #29453: use PKG_CHECK_MODULES to find a system fflas-ffpack.
5ce8498Trac #29453: add Gentoo package information for fflas-ffpack.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:2

Eh, I see what's going on with the MP library stuff. Sage is installing MPIR but with everything named "gmp", so having the mpir SPKG installed does satisfy a "gmp" dependency. I'll have to add mpir to the DEPCHECK.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:3

Oh, I accidentally got this one right.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2020

comment:4

"Warning: this doesn't get executed with --without-system-fflas-ffpack"
... can this be fixed?

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:5

Replying to @mkoeppe:

"Warning: this doesn't get executed with --without-system-fflas-ffpack"
... can this be fixed?

The issue is already addressed upstream (but not in a release) by detecting support for -march=native and using that instead of a million individual flags. So reporting it upstream isn't going to do any good.

Option 1: I could move that code to the top-level configure.ac. I think it will serve its purpose there, although it will be a bit out of place.

Option 2: we could just disable those flags in spkg-install, and use #27122 to add back -march=native.

Option 3: leave it where it is, and count on the fact that nobody will want to build an extra copy of fflas-ffpack.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2020

comment:6

Maybe just move to the PRE or POST argument of
SAGE_SPKG_CONFIGURE(PACKAGE-NAME,[CHECK],[REQUIRED-CHECK],[PRE],[POST])

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2020

Changed commit from 5ce8498 to 88c1717

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2020

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

88c1717Trac #29453: detect compiler flags in spkg-configure's POST phase.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:8

Replying to @mkoeppe:

Maybe just move to the PRE or POST argument of
SAGE_SPKG_CONFIGURE(PACKAGE-NAME,[CHECK],[REQUIRED-CHECK],[PRE],[POST])

Good idea, done.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 8, 2020

comment:9

Tests run at https://github.com/mkoeppe/sage/actions/runs/73584944

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 8, 2020

comment:10

debian, fedora seem to work

Should add arch: fflas-ffpack
and conda: fflas-ffpack

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 8, 2020

comment:11

Replying to @mkoeppe:

Should add arch: fflas-ffpack
and conda: fflas-ffpack

Can you just add them? I really don't want to sit through another rebuild after git checkout to add two lines of text =)

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 8, 2020

comment:12

Sure, will do

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 8, 2020

Changed branch from u/mjo/ticket/29453 to u/mkoeppe/ticket/29453

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 8, 2020

Changed commit from 88c1717 to a917baa

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 8, 2020

comment:14

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


New commits:

a917baabuild/pkgs/fflas_ffpack/distros: Add more distributions

@dimpase
Copy link
Member

dimpase commented Apr 9, 2020

comment:15

This does not work on Debian buster, as linbox needs a fresher fflas-ffpack:

checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for FFLAS_FFPACK... no
configure: error: Package requirements (fflas-ffpack >= 2.4.0) were not met:

Requested 'fflas-ffpack >= 2.4.0' but version of fflas-ffpack is 2.3.2
You may find new versions of fflas-ffpack at http://github.com/linbox-team/fflas-ffpack

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

Alternatively, you may set the environment variables FFLAS_FFPACK_CFLAGS
and FFLAS_FFPACK_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.
****************************************************************************************************************************************************
Error configuring linbox-1.6.3

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 9, 2020

comment:16

Replying to @dimpase:

This does not work on Debian buster, as linbox needs a fresher fflas-ffpack:

I guess I'll have to update the minimum version for now. When we're able to use the system linbox, we might be able to accept an older fflas-ffpack again. I'll open the linbox ticket now so we don't forget.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 9, 2020

Changed commit from a917baa to bdac4ba

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 9, 2020

comment:17

I opened #29484 for linbox, and set the bound on fflas-ffpack to 2.4.0 for now.


New commits:

56567acTrac #29453: require at least fflas-ffpack-2.4.0.
bdac4babuild/pkgs/fflas_ffpack/distros: Add more distributions

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 9, 2020

Changed branch from u/mkoeppe/ticket/29453 to u/mjo/ticket/29453

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2020

comment:18

This seems to work well (https://github.com/mkoeppe/sage/actions/runs/75739694). Only very new distributions have a sufficiently new version, of course.

By the way, on debian-buster-standard system fflas_ffpack is already not used because its dependency givaro is too old.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2020

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 11, 2020

Author: Michael Orlitzky

@vbraun
Copy link
Member

vbraun commented Apr 16, 2020

Changed branch from u/mjo/ticket/29453 to bdac4ba

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

4 participants