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

sagemath: update to 10.1. #45708

Merged
merged 1 commit into from
Aug 31, 2023
Merged

Conversation

tornaria
Copy link
Contributor

Testing the changes

  • I tested the changes in this PR: briefly

I will install and test a little bit. I'm pushing now to get an idea if CI works ok.

CC: @dkwo

@tornaria
Copy link
Contributor Author

I know how to fix musl. This is an issue in giac with musl libc:

$ cat test.cc
#include <giac/giac.h>
$ cc -c test.cc
In file included from /usr/include/giac/giac.h:3,
                 from test.cc:1:
/usr/include/giac/first.h: In function 'float fgamma(float)':
/usr/include/giac/first.h:575:39: error: 'gammaf' was not declared in this scope; did you mean 'tgammaf'?
  575 | inline float fgamma(float f1){ return gammaf(f1); } // or tgammaf(f1) on some versions of emscripten
      |                                       ^~~~~~
      |                                       tgammaf

It's already ack by upstream and it will be fixed, meanwhile I'll include a workaround so it works for us.

@tornaria
Copy link
Contributor Author

The i686 failure seems transient 🤷

I still want to run this locally a few more days, I'll rerun CI before we merge.

@dkwo
Copy link
Contributor

dkwo commented Aug 23, 2023

Looks good to me, thanks. I won't be able to run tests on aarch64 for three more weeks.
How about the old idea of splitting a devel subpkg, which depends on devel stuff?

tornaria added a commit to tornaria/void-packages that referenced this pull request Aug 23, 2023
As reported in void-linux#45708, including <giac/giac.h> fails in musl because the
HAVE_TGAMMAF macro from autotools is not available at this time.

Here we include a patch adapted from
void-linux#45708 (comment)
which is the solution upstream will include in next release of giac.

We need to fix this now so we can build sagemath in musl!
tornaria added a commit to tornaria/void-packages that referenced this pull request Aug 23, 2023
As reported in void-linux#45708, including <giac/giac.h> fails in musl because the
HAVE_TGAMMAF macro from autotools is not available at this time.

Here we include a patch adapted from
geogebra/giac@618a5de
which is the solution upstream will include in next release of giac.

We need to fix this now so we can build sagemath in musl!
@tornaria
Copy link
Contributor Author

Looks good to me, thanks. I won't be able to run tests on aarch64 for three more weeks. How about the old idea of splitting a devel subpkg, which depends on devel stuff?

Sure. How would that work? What would we gain? I imagine:

  • we remove some packages from sagemath depends.
  • we create a meta package sagemath-devel which depends on sagemath and on all the packages we removed from sagemath depends.

As a matter of fact, I don't see what would be the point of sagemath-devel. I mean, if it's possible to use sagemath without those depends, then they shouldn't be depends in the first place.

An important point is: how would we test that the "depends" line is correct (i.e. that we don't remove something that is needed, e.g., to pass the testsuite).

tornaria added a commit to tornaria/void-packages that referenced this pull request Aug 24, 2023
As reported in void-linux#45708, including <giac/giac.h> fails in musl because the
HAVE_TGAMMAF macro from autotools is not available at this time.

Here we include a patch adapted from
geogebra/giac@618a5de
which is the solution upstream will include in next release of giac.

We need to fix this now so we can build sagemath in musl!
@dkwo
Copy link
Contributor

dkwo commented Aug 26, 2023

We could remove

depends+=" eclib-devel flintlib-devel gcc-fortran gd-devel
 gsl-devel libpng-devel linbox-devel m4ri-devel 
 mpfr-devel ntl-devel pari-devel pkg-config python3-devel" 

and then see which tests fail from within installed sage;
within the build chroot, there should be no difference,
as those packages are already in (host)makedepends.
Does that make sense?

@tornaria
Copy link
Contributor Author

We could remove

depends+=" eclib-devel flintlib-devel gcc-fortran gd-devel
 gsl-devel libpng-devel linbox-devel m4ri-devel 
 mpfr-devel ntl-devel pari-devel pkg-config python3-devel" 

and then see which tests fail from within installed sage; within the build chroot, there should be no difference, as those packages are already in (host)makedepends. Does that make sense?

Nothing will fail inside the build chroot, since as you said, these are already installed via (host)makedepends. That's precisely the issue I am worried about.

The only easy way I see is to have a separate meta package to test sagemath, with a checkdepends on sagemath, which runs the sagemath testsuite (on the installed package).

leahneukirchen pushed a commit that referenced this pull request Aug 26, 2023
As reported in #45708, including <giac/giac.h> fails in musl because the
HAVE_TGAMMAF macro from autotools is not available at this time.

Here we include a patch adapted from
geogebra/giac@618a5de
which is the solution upstream will include in next release of giac.

We need to fix this now so we can build sagemath in musl!
manipuladordedados pushed a commit to manipuladordedados/void-packages that referenced this pull request Aug 27, 2023
As reported in void-linux#45708, including <giac/giac.h> fails in musl because the
HAVE_TGAMMAF macro from autotools is not available at this time.

Here we include a patch adapted from
geogebra/giac@618a5de
which is the solution upstream will include in next release of giac.

We need to fix this now so we can build sagemath in musl!
@tornaria
Copy link
Contributor Author

@dkwo all dependencies you singled out are needed to use either cython or fortran in sagemath, which are not optional (at least the testsuite fails without them).

I can imagine both features to be made optional, but I'm wary, something needs to happen so these are made optional and the testsuite works fine (a "feature" is detected, etc).

What is the need / advantage of this? I guess some download or installed size, but how much? Is it really worth it?

@tornaria
Copy link
Contributor Author

@dkwo all dependencies you singled out are needed to use either cython or fortran in sagemath, which are not optional (at least the testsuite fails without them).

I can imagine both features to be made optional, but I'm wary, something needs to happen so these are made optional and the testsuite works fine (a "feature" is detected, etc).

What is the need / advantage of this? I guess some download or installed size, but how much? Is it really worth it?

I think this will become possible soon, but not yet. The sagemath doctests are adding "feature flags" which detect features in the environment, but this is not yet completely polished for cython / external libraries / fortran code.

At this time, I'd rather focus on getting sagemath working on cython 3 so we can update cython (see #45086 and ahesford#2). After 10.2 or 10.3 it may be easier to do this.

In any case, I'd like to see a detail of how much is saved (say how much download/disk it takes now an install of sagemath with all deps not standard in void vs. what would it take if we get this "savings"). If the savings is not really very significant, I'm not sure I see the point...

@tornaria tornaria marked this pull request as ready for review August 31, 2023 00:47
@tornaria
Copy link
Contributor Author

@leahneukirchen @dkwo This is ready to merge IMO.

Note that my last push is after the last big update (boost, python, ssl, etc) so CI here is tested with the up-to-date packages in the repo.

@dkwo
Copy link
Contributor

dkwo commented Aug 31, 2023

I agree, I can do more experiments for one of the next point releases.
Looks good to me.

@leahneukirchen leahneukirchen merged commit 9e68e66 into void-linux:master Aug 31, 2023
8 checks passed
@tornaria tornaria deleted the sagemath branch September 2, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants