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

spkg-configure.m4 for linbox #29631

Closed
thierry-FreeBSD opened this issue May 1, 2020 · 91 comments
Closed

spkg-configure.m4 for linbox #29631

thierry-FreeBSD opened this issue May 1, 2020 · 91 comments

Comments

@thierry-FreeBSD
Copy link

This ticket adds an spkg-configure.m4 for linbox, in order to use it from a system package if possible (see #27330).

Note for packagers: by default, linbox produces a buggy linbox.pc, and @LINBOXSAGE_LIBS@ must be removed from their linbox.pc.in (see #27205).

No more problem after that.

CC: @mkoeppe @dimpase @kiwifb @isuruf @embray @antonio-rojas @slel

Component: build: configure

Keywords: linbox, system packages

Author: Thierry Thomas, Michael Orlitzky, Samuel Lelièvre

Branch: bc7a6f8

Reviewer: Dima Pasechnik

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

@thierry-FreeBSD
Copy link
Author

To be put under build/pkgs/linbox

@mkoeppe
Copy link
Contributor

mkoeppe commented May 1, 2020

comment:1

Attachment: spkg-configure.m4.gz

@mkoeppe

This comment has been minimized.

@orlitzky
Copy link
Contributor

orlitzky commented May 4, 2020

comment:2

We also need iml, ntl, flint, fplll, and mpfr in the DEPCHECK. Linbox can be built without some of those libraries with, for example, --without-ntl, so we may need to check for the parts that are actually used in sage. The sage-on-gentoo ebuild passes --without-fplll, so that dependency is probably superfluous, and maybe others are as well? In any case, to check which libraries linbox actually supports... after the PKG_CHECK_MODULES call, I guess we could look for things like -lntl in LINBOX_LIBS?

I don't see any distros that have a 1.6.x version other than 1.6.3, so it would probably be pointless to lower the version to 1.6.0. The real question is whether or not we can use the v1.5.2 that e.g. older Debian/Fedora ship.

@thierry-FreeBSD
Copy link
Author

comment:3
pkgconf linbox --libs

sets LINBOX_LIB, and we could grep that the required libs are there?

@orlitzky
Copy link
Contributor

orlitzky commented May 4, 2020

comment:4

Hmm, looking at the spkg-install for linbox, we already pass --without-fplll:

# Disable fplll as version 5.x is not supported by linbox <= 1.5.0.
# This is harmless as no functionality using fplll is exposed in Sage.
# See trac ticket #21221.
LINBOX_CONFIGURE="--without-fplll $LINBOX_CONFIGURE"

So, the existing dependency on fplll (in build/pkgs/linbox/dependencies) is wrong. I'm going to build a system copy of linbox with all of the other stuff disabled and see what goes wrong. Maybe we don't care if the other stuff is enabled, either.

@orlitzky
Copy link
Contributor

orlitzky commented May 4, 2020

comment:5

For posterity, this also appears to be obsolete,

# Need to use 'bash' for configure, see
# https://github.com/sagemath/sage-prod/issues/23451
if [ -z "$CONFIG_SHELL" ]; then
    export CONFIG_SHELL=`command -v bash`
fi

as I've been compiling linbox-1.6.3 this whole time with /bin/sh -> /bin/dash.

@orlitzky
Copy link
Contributor

orlitzky commented May 4, 2020

comment:6

Even if you pass --without-foo for all of the optional linbox dependencies, flint still gets pulled in automagically, so it's going to be required either way. I'm doing a full sage build / ptestlong now with the rest (ntl, fplll, mpfr, iml) disabled to see if any of them are actually important.

@orlitzky
Copy link
Contributor

orlitzky commented May 5, 2020

Branch: u/mjo/ticket/29631

@orlitzky
Copy link
Contributor

orlitzky commented May 5, 2020

comment:7

Everything is fine if we disable ntl, iml, and mpfr in linbox, so I created a branch with those dependencies eliminated. That means that we only need to add "flint" to the DEPCHECK.


New commits:

72291a0Trac #29631: don't force SHELL=bash when installing linbox.
787b5dfTrac #29631: drop fplll from linbox's dependencies.
e4a14c0Trac #29631: disable additional unused features of the linbox SPKG.

@orlitzky
Copy link
Contributor

orlitzky commented May 5, 2020

Changed author from gh-thierry-FreeBSD to gh-thierry-FreeBSD, Michael Orlitzky

@orlitzky
Copy link
Contributor

orlitzky commented May 5, 2020

Commit: e4a14c0

@orlitzky
Copy link
Contributor

orlitzky commented May 5, 2020

comment:8

This page has some information on how to checkout and push new branches to our trac server:

http://doc.sagemath.org/html/en/developer/manual_git.html

You should be able to checkout my branch, add a commit on top of it, and then push a new branch of your own (which you would then stick in the "branch" field of this ticket). That's the best way to collaborate since it will keep your commit info and author information in-tact. If you don't feel like dealing with it, though, I can just add the spkg-configure.m4 onto my branch and re-push.

After adding flint to the DEPCHECK, I think only two questions remains:

  • Do we need to check for flint support in the spkg-configure check? Given that flint support is "automagically" enabled, I don't think it's necessary on source-based distros, who should have flint support enabled unconditionally to avoid problems. If none of the binary distros ship a flint-less linbox, then we can skip it.
  • Should we try to set the version bound to 1.5.2? Someone would need to test that.

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 11, 2020

comment:9

These questions can be resolved by adding system package information to build/pkgs/linbox/distros and then testing...

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 11, 2020

Changed author from gh-thierry-FreeBSD, Michael Orlitzky to Thierry Thomas, Michael Orlitzky

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 9, 2020

comment:11

The duplicate ticket #29484 reminds us to "(attempt to) lower the bound on fflas-ffpack in its spkg-configure.m4. If we can accept an older system linbox that itself uses an older system fflas-ffpack, that would be nice."

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 13, 2021

comment:13

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@slel
Copy link
Member

slel commented Mar 22, 2021

Changed author from Thierry Thomas, Michael Orlitzky to Thierry Thomas, Michael Orlitzky, Samuel Lelièvre

@slel
Copy link
Member

slel commented Mar 22, 2021

comment:14

Replying to @mkoeppe:

These questions can be resolved by adding system package information to build/pkgs/linbox/distros and then testing...

Here we go.

@slel
Copy link
Member

slel commented Mar 22, 2021

Changed branch from u/mjo/ticket/29631 to u/slelievre/29361

@mkoeppe mkoeppe added this to the sage-9.6 milestone Dec 27, 2021
@orlitzky
Copy link
Contributor

comment:55

Replying to @slel:

Is anything else needed here?

I think we need #8450 to be merged to clear up the merge conflict but otherwise the commits for this ticket are good to go.

@orlitzky
Copy link
Contributor

comment:56

I don't think #8450 is going to make it into v9.5, maybe time to rebase this with just the linbox commits?

@slel
Copy link
Member

slel commented Feb 6, 2022

comment:57

#8450 made it into Sage 9.6.beta0.

@orlitzky
Copy link
Contributor

orlitzky commented Feb 6, 2022

comment:58

Replying to @slel:

#8450 made it into Sage 9.6.beta0.

Merge failed though D:

@orlitzky
Copy link
Contributor

orlitzky commented Feb 7, 2022

Changed commit from 56aba2f to fc083c2

@orlitzky
Copy link
Contributor

orlitzky commented Feb 7, 2022

New commits:

ab58ae4Trac #29631: don't force SHELL=bash when installing linbox.
94d7717Trac #29631: drop fplll from linbox's dependencies.
7afd36129361: Add Fedora and Gentoo linbox package info
8128f66Trac #29631: new spkg-configure.m4 for linbox.
48eab64add missing imports
91aee3bcorrect the package name: `-`->`_`
6ee694229631: add fplll and libm4ri debian distro info
fc083c229631: upper bound for linbox version

@orlitzky
Copy link
Contributor

orlitzky commented Feb 7, 2022

Changed branch from u/slelievre/29631 to u/mjo/ticket/29631

@orlitzky
Copy link
Contributor

orlitzky commented Feb 7, 2022

Changed dependencies from #8450, #32576, #32828 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2022

Changed commit from fc083c2 to bc7a6f8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2022

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

9a8bf64Trac #29631: don't force SHELL=bash when installing linbox.
9f788f5Trac #29631: drop fplll from linbox's dependencies.
796b0c329361: Add Fedora and Gentoo linbox package info
9b7d1ecTrac #29631: new spkg-configure.m4 for linbox.
a5d8c72correct the package name: `-`->`_`
4d4135629631: add fplll and libm4ri debian distro info
bc7a6f829631: upper bound for linbox version

@orlitzky
Copy link
Contributor

comment:61

Ping, this one should be easy now:

  • Very few lines changed
  • We require the one exact version of linbox that sage itself ships
  • It already went through Github CI

@dimpase
Copy link
Member

dimpase commented Mar 10, 2022

comment:62

lgtm

@dimpase
Copy link
Member

dimpase commented Mar 10, 2022

Changed reviewer from Dima Pasechnik, https://github.com/sagemath/sagetrac-mirror/actions/runs/1538684506 to Dima Pasechnik

@orlitzky
Copy link
Contributor

comment:63

Thanks! Now I guess I have to test the maxima ticket on github.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 10, 2022

comment:64

This ticket makes no sense because we have no spkg-configure.m4 for the dependency fflas_ffpack

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 10, 2022

comment:65

Sorry for the noise - I was wrong about that (misled by a typo that I made)

@vbraun
Copy link
Member

vbraun commented Mar 20, 2022

Changed branch from u/mjo/ticket/29631 to bc7a6f8

@dimpase
Copy link
Member

dimpase commented Mar 24, 2022

Changed commit from bc7a6f8 to none

@dimpase
Copy link
Member

dimpase commented Mar 24, 2022

comment:67

I've missed that build/pkgs/linbox/distros/fedora.txt also needs linbox-devel

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

7 participants