sage: 8.9 -> 9.2 by collares · Pull Request #105615 · NixOS/nixpkgs

collares · 2020-12-01T20:25:26Z

Motivation for this change

@omasanori did most of the work in this update (see #101447), but I had some free time to finish up the work over the last days. All tests pass, except for the fact that src/sage/interfaces/singular.py is failing intermittently upstream (https://trac.sagemath.org/ticket/30945).

Keep in mind while reviewing is that I changed SAGE_LOCAL in sage-env.nix. There's a comment on the file explaining the motivation, but I am not sure if I made the right call.

Things done

pkgs/applications/science/math/sage/sage-with-env.nix

SuperSandro2000 · 2020-12-01T21:56:27Z

pkgs/development/compilers/ecl/default.nix

Can you please use fetchurl?

Oh derp. You can use fetchurl but fetchpatch is better because it normalizes the patch that changing metadata does not cause a hash mismatch. Sorry about my derp.

Don't worry! I thought about that (it was done like this before), but the patches are plain files committed to Sage's repository, they are not dynamically generated. Does fetchpatch still provide an advantage in this case?

collares · 2020-12-02T01:49:53Z

ecl.pyx is also intermittently segfaulting on the CI build (it passed before I pushed sagedoc changes which have nothing to do with ECL). A trac search returns nothing, and I can't reproduce the failure locally.

Update: This was a GC problem and is now fixed (see below).

jcumming · 2020-12-02T02:18:21Z

I just gave this a whirl, and it works for me. I'll try it in a few more spots and see if I can isolate a failure.

Synthetica9 · 2020-12-02T15:09:16Z

I'm still running tests, but the build seems to work. Kudos!

symphorien · 2020-12-02T15:51:31Z

I reviewed the giac/xcas update and it looks good to me.

collares · 2020-12-02T17:26:07Z

Context for the intermittent failure: ecl.py is just

from sage.libs.ecl import *
from cysignals.tests import interrupt_after_delay
ecl_eval("(setf i 0)")
inf_loop = ecl_eval("(defun infinite() (loop (incf i)))")
interrupt_after_delay(1000)
inf_loop()

Here's the stacktrace for the ecl.pyx failure (running under Python instead of IPython, but that shouldn't make a difference).

#0  0x00007f58ee108c71 in sigsegv_handler ()
   from /nix/store/9wzdzij7qwv27ak6p93shmp22xwvx2ha-ecl-20.4.24/lib/libecl.so.20.4
#1  <signal handler called>
#2  0x00007f58ee11aca6 in ecl_cons ()
   from /nix/store/9wzdzij7qwv27ak6p93shmp22xwvx2ha-ecl-20.4.24/lib/libecl.so.20.4
#3  0x00007f58ee0bffb0 in ecl_interpret ()
   from /nix/store/9wzdzij7qwv27ak6p93shmp22xwvx2ha-ecl-20.4.24/lib/libecl.so.20.4
#4  0x00007f58ee0bd5b5 in ecl_apply_from_stack_frame ()
   from /nix/store/9wzdzij7qwv27ak6p93shmp22xwvx2ha-ecl-20.4.24/lib/libecl.so.20.4
#5  0x00007f58ee0bda55 in cl_apply ()
   from /nix/store/9wzdzij7qwv27ak6p93shmp22xwvx2ha-ecl-20.4.24/lib/libecl.so.20.4
#6  0x00007f58ee3337b4 in safe_cl_apply ()
   from /nix/store/cczgihp4z3f20mvgfw74kymjs0avxvsv-python3-3.8.6-env/lib/python3.8/site-packages/sage/libs/ecl.cpython-38-x86_64-linux-gnu.so
#7  0x00007f58ee33bcca in __pyx_f_4sage_4libs_3ecl_ecl_safe_apply ()
   from /nix/store/cczgihp4z3f20mvgfw74kymjs0avxvsv-python3-3.8.6-env/lib/python3.8/site-packages/sage/libs/ecl.cpython-38-x86_64-linux-gnu.so
#8  0x00007f58ee33c213 in __pyx_pw_4sage_4libs_3ecl_9EclObject_15__call__ ()
   from /nix/store/cczgihp4z3f20mvgfw74kymjs0avxvsv-python3-3.8.6-env/lib/python3.8/site-packages/sage/libs/ecl.cpython-38-x86_64-linux-gnu.so
#9  0x00007f5a89bffd47 in _PyObject_MakeTpCall ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#10 0x00007f5a89c6d839 in _PyEval_EvalFrameDefault ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#11 0x00007f5a89c681f1 in _PyEval_EvalCodeWithName ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#12 0x00007f5a89c67ef3 in PyEval_EvalCodeEx ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#13 0x00007f5a89cdea5b in PyEval_EvalCode ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#14 0x00007f5a89cef718 in run_eval_code_obj ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#15 0x00007f5a89cef6b3 in run_mod ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#16 0x00007f5a89bbb148 in PyRun_FileExFlags ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#17 0x00007f5a89bbaf1b in PyRun_SimpleFileExFlags ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#18 0x00007f5a89cf75e6 in Py_RunMain ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#19 0x00007f5a89cf6f59 in Py_BytesMain ()
   from /nix/store/fzgk230riagc58n8w1fa41ngs723pzpb-python3-3.8.6/lib/libpython3.8.so.1.0
#20 0x00007f5a896fedbd in __libc_start_main ()
   from /nix/store/a3syww9igm49zdzq3ibzw9m8ccvsgxla-glibc-2.32/lib/libc.so.6
#21 0x000000000040107a in _start ()

ecl_cons disables interrupts temporarily. Maybe that interacts badly with cysignals?

collares · 2020-12-02T18:42:51Z

ecl/default.nix does not use boehmgc by default, but ecl/16.1.2.nix does. Is that intentional? The intermittent failures in ecl.pyx are related to that, and I fixed them by overriding useBoehmgc in sage/default.nix. I wonder if we should change the default GC, though.

jcumming · 2020-12-02T18:51:43Z

I've come across a few more warts:

the nauty hash changed (upstream modified the tarball with a bugfix)
the pari 2.11.4 tarball moved
the ppl git repo at https://www.cs.unipr.it/git/gitweb.cgi?p=ppl/ppl.git;a=summary has become inaccessible.

Synthetica9

Compiles and runs for me!

Synthetica9 · 2020-12-14T15:48:22Z

What's holding this up?

jcumming · 2020-12-14T16:15:07Z

My derpy RS code stuff works. Thanks everyone! I support merging this.

collares · 2020-12-14T22:30:39Z

I have rebased and reverted the cypari2 update that landed after this PR was submitted (in #105368), since it was decided in #103810 that we would wait for upstream to update cypari2 first (https://trac.sagemath.org/ticket/31029). Hopefully that helps with the failure.

collares · 2020-12-15T13:21:23Z

Since there are several people on the Sage maintainer team and @omasanori already reviewed the patches, hopefully asking this will help coordinate: Is reviewing this a pending task on someone else's to-do list? If so, I am very happy to wait to get comments, especially on some potentially questionable decisions I made to get the testsuite to pass. I completely understand that time is a scarce resource, and this message is not intended to rush anyone! If you do intend to review this but haven't gotten around to do it yet, please don't re-order your to-do list because of this comment :)

On the other hand, if no one has a pending review, then I propose we merge this PR, since the tests currently pass (modulo the intermittent singular.pyx failure, which also affects upstream and is being investigated there) and there is user interest in getting Sage back into a usable state. There are definitely some parts that need feedback and cleanup, even if just removing "is this OK?" comments I left, but I promise to address any comments or suggestions quickly in a follow-up PR.

cc @timokau @7c6f434c

symphorien · 2020-12-15T19:16:48Z

I ran nix-review successfully on this afternoon (before your last commit). Could you add an explanation in comment to the FIXME you added ?

The old `nauty` tarball is currently accessible at https://distfiles.macports.org/nauty/nauty27r1.tar.gz. The diff is a single line in genbg.c: - SUMMARY(&nout,t2-t1); + SUMMARY(nout,t2-t1);

…auth)

pip install deprecated the --build flag. The standard python installPhase (in pip-install-hook.sh) got updated in commit 76966f8, but we use a custom one so we need to update it separately.

collares · 2020-12-15T21:11:43Z

@symphorien Good catch, thanks! Those lines weren't needed after all, so I just removed them. I also squashed the sage fixups to help with merging.

timokau · 2020-12-19T17:11:26Z

Yes, I had planned to review this. Unfortunately my spare time for open source development is very limited recently and this is quite the diff, so I kept putting it off.

I agree with you: This is a definite improvement over the status quo and shouldn't be held off for too long. Sage and nixpkgs moves on, and things might break again. Given your promise to fix up any issues that come up in an "after the fact" review, I'm happy to merge this now and look at it in some more detail later. I have skimmed the changes and nothing sticks out.

Could you start discussions on the relevant sections of the diff where you need additional feedback?

Thank you @collares and @omasanori for your incredible work. I am very, very glad that we found not only one, but two people who have the motivation and skill to do this after I have neglected the package for so long.

collares

Many thanks @timokau for all your hard work over the years on packaging Sage, and @omasanori for doing most of the work in getting this back to a working state! Sorry for the delay in submitting this, I thought I had done it yesterday but I forgot to click "Submit review" :)

collares · 2020-12-19T21:32:14Z

pkgs/applications/science/math/sage/sagedoc.nix

    chmod -R 755 "$SAGE_DOC_SRC_OVERRIDE"
  '';

+  postPatch = ''


Is postPatch the right place for this?

I saw some similar patch represented as a sequence of commands postPatch instances, so it is fine IMHO.

collares · 2020-12-19T21:36:40Z

pkgs/applications/science/math/sage/sage-env.nix

    export SAGE_ROOT='${sagelib.src}'
-    export SAGE_LOCAL='@sage-local@'
+  '' +
+    # TODO: is using pythonEnv instead of @sage-local@ here a good


This was one of the places where I wanted feedback. This (along with the sed change) makes environment tests pass, but I wasn't sure if using ${pythonEnv} instead of ${sage-with-env} is a good idea conceptually.

Also, now that I look at this, there's an orthogonal issue: I guess I should keep this as @sage-local@ and adapt sage-with-env.nix's substituteInPlace call with whatever value we decide to use.

collares · 2020-12-19T21:39:32Z

pkgs/applications/science/math/sage/sage-src.nix

  '';

-  configurePhase = "# do nothing";
+  # Test src/doc/en/reference/spkg/conf.py will fail if


Upstream sage now has this src/doc/bootstrap file, which generates some files that are used in tests and documentation. I copied the part that is relevant to tests here and the documentation part into sagedoc.nix, but copying such a big chunk of code felt a bit icky. The alternative would be to generate an empty spkg/index.rst file, which is not terrible if there are no tests there anyway.

Also, is configurePhase the right place for this?

yurrriq · 2020-12-21T05:49:18Z

Thanks, everyone! Especially @omasanori!

collares requested review from FRidh and jonringer as code owners December 1, 2020 20:25

ofborg bot added 6.topic: python Python is a high-level, general-purpose programming language. 8.has: documentation This PR adds or changes documentation labels Dec 1, 2020

ofborg bot requested review from 7c6f434c, peti, symphorien and timokau December 1, 2020 20:34

ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Dec 1, 2020

collares force-pushed the sage-update branch 2 times, most recently from b2861ea to 2fbcd37 Compare December 1, 2020 21:08

SuperSandro2000 reviewed Dec 1, 2020

View reviewed changes

omasanori mentioned this pull request Dec 1, 2020

[WIP] sage: 8.9 -> 9.2 #101447

Closed

10 tasks

collares force-pushed the sage-update branch 3 times, most recently from f0ba932 to b17b4e5 Compare December 2, 2020 00:34

collares force-pushed the sage-update branch from b17b4e5 to 29d82d8 Compare December 2, 2020 12:36

collares force-pushed the sage-update branch from 29d82d8 to 019ca9c Compare December 2, 2020 18:42

collares force-pushed the sage-update branch from 5cefb96 to 6fe3f50 Compare December 2, 2020 21:21

Synthetica9 approved these changes Dec 3, 2020

View reviewed changes

omasanori and others added 6 commits December 14, 2020 19:24

sage: 8.9 -> 9.2

5c4136d

giac: 1.5.0-21 -> 1.5.0-87

5df948d

gmpy2: 2.1a4 -> 2.1.0b5

e1998b1

ecl: patches to fix SIGFPEs and for unicode compatibility with maxima

c46dcf4

pynac: import patch to avoid deprecation warnings on python3.8

cf6364b

maxima-ecl: unpin ecl version

fecce09

collares force-pushed the sage-update branch from 6fe3f50 to 90fb6b1 Compare December 14, 2020 22:27

collares force-pushed the sage-update branch 2 times, most recently from 348aeda to a60c239 Compare December 15, 2020 02:06

ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 15, 2020

collares force-pushed the sage-update branch from fde8630 to 82aa21d Compare December 15, 2020 12:21

collares added 4 commits December 15, 2020 16:46

nauty: update tarball sha256

4058e45

The old `nauty` tarball is currently accessible at https://distfiles.macports.org/nauty/nauty27r1.tar.gz. The diff is a single line in genbg.c: - SUMMARY(&nout,t2-t1); + SUMMARY(nout,t2-t1);

pari: update tarball location, since it moved

8b8cae7

ppl: fetch clang patch from alternate location (that doesn't require …

88cfc7f

…auth)

cypari2: revert 2.1.1 -> 2.1.2 update, sync with pip-install-hook.sh

795baaa

pip install deprecated the --build flag. The standard python installPhase (in pip-install-hook.sh) got updated in commit 76966f8, but we use a custom one so we need to update it separately.

collares force-pushed the sage-update branch from 3bb5391 to 1f11137 Compare December 15, 2020 19:47

sage: 8.9 -> 9.2 fixups

8100c5a

collares force-pushed the sage-update branch from 1f11137 to 8100c5a Compare December 15, 2020 19:51

timokau merged commit afead52 into NixOS:master Dec 19, 2020

timokau mentioned this pull request Dec 19, 2020

[WIP] sage: 8.9 -> 9.0, switch to python3 #70474

Closed

10 tasks

collares commented Dec 20, 2020

View reviewed changes

collares deleted the sage-update branch December 28, 2020 14:58

jpas mentioned this pull request Jan 3, 2021

sage: Test failure #92518

Closed

FRidh mentioned this pull request Nov 28, 2021

Convert remaining python2 applications over to python3 #101964

Closed

77 tasks

Uh oh!

Conversation

collares commented Dec 1, 2020

Motivation for this change

Things done

Uh oh!

Uh oh!

SuperSandro2000 Dec 1, 2020

Choose a reason for hiding this comment

Uh oh!

SuperSandro2000 Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

collares Dec 2, 2020

Choose a reason for hiding this comment

Uh oh!

collares commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcumming commented Dec 2, 2020

Uh oh!

Synthetica9 commented Dec 2, 2020

Uh oh!

symphorien commented Dec 2, 2020

Uh oh!

collares commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

collares commented Dec 2, 2020

Uh oh!

jcumming commented Dec 2, 2020

Uh oh!

Synthetica9 left a comment

Choose a reason for hiding this comment

Uh oh!

Synthetica9 commented Dec 14, 2020

Uh oh!

jcumming commented Dec 14, 2020

Uh oh!

collares commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

collares commented Dec 15, 2020

Uh oh!

symphorien commented Dec 15, 2020

Uh oh!

collares commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timokau commented Dec 19, 2020

Uh oh!

collares left a comment

Choose a reason for hiding this comment

Uh oh!

collares Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

omasanori Dec 21, 2020

Choose a reason for hiding this comment

Uh oh!

collares Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

collares Dec 19, 2020

Choose a reason for hiding this comment

Uh oh!

yurrriq commented Dec 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

SuperSandro2000 Dec 2, 2020 •

edited

Loading

collares commented Dec 2, 2020 •

edited

Loading

collares commented Dec 2, 2020 •

edited

Loading

collares commented Dec 14, 2020 •

edited

Loading

collares commented Dec 15, 2020 •

edited

Loading