Merge pull request #50998 from timokau/sage-refactor

Sage refactor

17 files changed, 235 insertions, 372 deletions

diff --git a/pkgs/applications/science/math/sage/README.md b/pkgs/applications/science/math/sage/README.md
index 46496664f28e..26e91fc63133 100644
--- a/pkgs/applications/science/math/sage/README.md
+++ b/pkgs/applications/science/math/sage/README.md
@@ -2,46 +2,7 @@
 
 Sage is a pretty complex package that depends on many other complex packages and patches some of those. As a result, the sage nix package is also quite complex.
 
-Don't feel discouraged to fix, simplify or improve things though. Here's a quick overview over the functions of the individual files:
-
-- `sage-src.nix`  
-  Downloads the source code and applies patches. This makes sure that all the other files work with the same sage source. If you want to apply a patch to sage or update sage to a new version, this is the place to do it.
-
-- `env-locations.nix`  
-  Creates a bash file that sets a bunch of environment variables telling sage where to find various packages and files. The definitions of those environment variables can be found in the sage source in the `src/env.py` file. This bash file needs to be sourced before sage is started (done in `sage-env.nix` and `sagedoc.nix`).
-
-- `sage-env.nix`  
-  Sets all environment variables sage needs to run. This includes the package locations defined in `env-locations.nix` as well as the location of sage itself and its various subdirectories.
-
-- `sagelib.nix`  
-  Defines the main sage package (without setting the necessary environments or running any tests).
-
-- `sage-with-env.nix`  
-  Wraps sage in the necessary environment.
-
-- `sage.nix`  
-  Runs sages doctests.
-
-- `sage-wrapper.nix`  
-  Optionally tells sage where do find the docs.
-
-- `sagedoc.nix`  
-  Builds and tests the sage html documentation. Can be used for offline documentation viewing as well as the sage `browse_sage_doc` and `search_doc` functions.
-
-- `sagenb.nix`  
-  The (semi deprecated) sage notebook.
-
-- `default.nix`  
-  Introduces necessary overrides, defines new packages and ties everything together (returning the `sage` package).
-
-- `flask-oldsessions.nix`, `flask-openid.nix`, `python-openid.nix`
-  These are python packages that were rejected from the main nixpkgs tree because they appear unmaintained. They are needed for the (semi-deprecated) sage notebook. Since that notebook is still needed to run the sage doctests, these packages are included but not exposed to the rest of nixpkgs.
-
-- `pybrial.nix`  
-  pybrial is a dependency of sage. However, pybrial itself also has sage as a dependency. Because of that circular dependency, pybrial is hidden from the rest of nixpkgs (just as the flask packages and python-openid.
-
-- `openblas-pc.nix`  
-  This creates a `.pc` file to be read by `pkg-config` that allows openblas to take on different roles, like `cblas` or `lapack`.
+Don't feel discouraged to fix, simplify or improve things though. The individual files have comments explaining their purpose. The most importent ones are `default.nix` linking everything together, `sage-src.nix` adding patches and `sagelib.nix` building the actual sage package.
 
 ## The sage build is broken
 
diff --git a/pkgs/applications/science/math/sage/default.nix b/pkgs/applications/science/math/sage/default.nix
index cf8515283cd2..fda827696d3d 100644
--- a/pkgs/applications/science/math/sage/default.nix
+++ b/pkgs/applications/science/math/sage/default.nix
@@ -1,68 +1,85 @@
-{ nixpkgs
+{ pkgs
 , withDoc ? false
 }:
 
+# Here sage and its dependencies are put together. Some dependencies may be pinned
+# as a last resort. Patching sage for compatibility with newer dependency versions
+# is always preferred, see `sage-src.nix` for that.
+
 let
-  inherit (nixpkgs) fetchpatch fetchurl symlinkJoin callPackage nodePackages;
+  inherit (pkgs) fetchurl symlinkJoin callPackage nodePackages;
 
   # https://trac.sagemath.org/ticket/15980 for tracking of python3 support
-  python = nixpkgs.python2.override {
+  python = pkgs.python2.override {
     packageOverrides = self: super: {
       # python packages that appear unmaintained and were not accepted into the nixpkgs
       # tree because of that. These packages are only dependencies of the more-or-less
       # deprecated sagenb. However sagenb is still a default dependency and the doctests
       # depend on it.
       # See https://github.com/NixOS/nixpkgs/pull/38787 for a discussion.
+      # The dependency on the sage notebook (and therefore these packages) will be
+      # removed in the future:
+      # https://trac.sagemath.org/ticket/25837
       flask-oldsessions = self.callPackage ./flask-oldsessions.nix {};
       flask-openid = self.callPackage ./flask-openid.nix {};
       python-openid = self.callPackage ./python-openid.nix {};
-
-      pybrial = self.callPackage ./pybrial.nix {};
-
-      sagelib = self.callPackage ./sagelib.nix {
-        inherit flint ecl arb;
-        inherit sage-src openblas-blas-pc openblas-cblas-pc openblas-lapack-pc pynac singular;
-        linbox = nixpkgs.linbox.override { withSage = true; };
-      };
-
       sagenb = self.callPackage ./sagenb.nix {
         mathjax = nodePackages.mathjax;
       };
 
-      sagedoc = self.callPackage ./sagedoc.nix {
-        inherit sage-src;
-      };
+      # Package with a cyclic dependency with sage
+      pybrial = self.callPackage ./pybrial.nix {};
 
-      env-locations = self.callPackage ./env-locations.nix {
-        inherit pari_data ecl;
-        inherit singular;
-        three = nodePackages.three;
-        mathjax = nodePackages.mathjax;
+      # `sagelib`, i.e. all of sage except some wrappers and runtime dependencies
+      sagelib = self.callPackage ./sagelib.nix {
+        inherit flint ecl arb;
+        inherit sage-src pynac singular;
+        linbox = pkgs.linbox.override { withSage = true; };
       };
+    };
+  };
 
-      sage-env = self.callPackage ./sage-env.nix {
-        inherit sage-src python rWrapper openblas-cblas-pc ecl singular palp flint pynac pythonEnv;
-        pkg-config = nixpkgs.pkgconfig; # not to confuse with pythonPackages.pkgconfig
-      };
+  # A bash script setting various environment variables to tell sage where
+  # the files its looking fore are located. Also see `sage-env`.
+  env-locations = callPackage ./env-locations.nix {
+    inherit pari_data ecl;
+    inherit singular;
+    cysignals = python.pkgs.cysignals;
+    three = nodePackages.three;
+    mathjax = nodePackages.mathjax;
+  };
 
-      sage-with-env = self.callPackage ./sage-with-env.nix {
-        inherit pythonEnv;
-        inherit sage-src openblas-blas-pc openblas-cblas-pc openblas-lapack-pc pynac singular;
-        pkg-config = nixpkgs.pkgconfig; # not to confuse with pythonPackages.pkgconfig
-        three = nodePackages.three;
-      };
+  # The shell file that gets sourced on every sage start. Will also source
+  # the env-locations file.
+  sage-env = callPackage ./sage-env.nix {
+    sagelib = python.pkgs.sagelib;
+    inherit env-locations;
+    inherit python rWrapper ecl singular palp flint pynac pythonEnv;
+    pkg-config = pkgs.pkgconfig; # not to confuse with pythonPackages.pkgconfig
+  };
 
-      sage = self.callPackage ./sage.nix { };
+  # The documentation for sage, building it takes a lot of ram.
+  sagedoc = callPackage ./sagedoc.nix {
+    inherit sage-with-env;
+    inherit python;
+  };
 
-      sage-wrapper = self.callPackage ./sage-wrapper.nix {
-        inherit sage-src withDoc;
-      };
-    };
+  # sagelib with added wrappers and a dependency on sage-tests to make sure thet tests were run.
+  sage-with-env = callPackage ./sage-with-env.nix {
+    inherit pythonEnv;
+    inherit sage-env;
+    inherit pynac singular;
+    pkg-config = pkgs.pkgconfig; # not to confuse with pythonPackages.pkgconfig
+    three = nodePackages.three;
   };
 
-  openblas-blas-pc = callPackage ./openblas-pc.nix { name = "blas"; };
-  openblas-cblas-pc = callPackage ./openblas-pc.nix { name = "cblas"; };
-  openblas-lapack-pc = callPackage ./openblas-pc.nix { name = "lapack"; };
+  # Doesn't actually build anything, just runs sages testsuite. This is a
+  # separate derivation to make it possible to re-run the tests without
+  # rebuilding sagelib (which takes ~30 minutes).
+  # Running the tests should take something in the order of 1h.
+  sage-tests = callPackage ./sage-tests.nix {
+    inherit sage-with-env;
+  };
 
   sage-src = callPackage ./sage-src.nix {};
 
@@ -91,9 +108,9 @@ let
   } // { extraLibs = pythonRuntimeDeps; }; # make the libs accessible
 
   # needs to be rWrapper, standard "R" doesn't include default packages
-  rWrapper = nixpkgs.rWrapper.override {
+  rWrapper = pkgs.rWrapper.override {
     # https://trac.sagemath.org/ticket/25674
-    R = nixpkgs.R.overrideAttrs (attrs: rec {
+    R = pkgs.R.overrideAttrs (attrs: rec {
       name = "R-3.4.4";
       src = fetchurl {
         url = "http://cran.r-project.org/src/base/R-3/${name}.tar.gz";
@@ -102,44 +119,45 @@ let
     });
   };
 
-  arb = nixpkgs.arb.override { inherit flint; };
+  arb = pkgs.arb.override { inherit flint; };
 
-  singular = nixpkgs.singular.override { inherit flint; };
+  singular = pkgs.singular.override { inherit flint; };
 
   # *not* to confuse with the python package "pynac"
-  pynac = nixpkgs.pynac.override { inherit singular flint; };
+  pynac = pkgs.pynac.override { inherit singular flint; };
 
   # With openblas (64 bit), the tests fail the same way as when sage is build with
   # openblas instead of openblasCompat. Apparently other packages somehow use flints
   # blas when it is available. Alternative would be to override flint to use
   # openblasCompat.
-  flint = nixpkgs.flint.override { withBlas = false; };
+  flint = pkgs.flint.override { withBlas = false; };
 
   # Multiple palp dimensions need to be available and sage expects them all to be
   # in the same folder.
   palp = symlinkJoin {
-    name = "palp-${nixpkgs.palp.version}";
+    name = "palp-${pkgs.palp.version}";
     paths = [
-      (nixpkgs.palp.override { dimensions = 4; doSymlink = false; })
-      (nixpkgs.palp.override { dimensions = 5; doSymlink = false; })
-      (nixpkgs.palp.override { dimensions = 6; doSymlink = true; })
-      (nixpkgs.palp.override { dimensions = 11; doSymlink = false; })
+      (pkgs.palp.override { dimensions = 4; doSymlink = false; })
+      (pkgs.palp.override { dimensions = 5; doSymlink = false; })
+      (pkgs.palp.override { dimensions = 6; doSymlink = true; })
+      (pkgs.palp.override { dimensions = 11; doSymlink = false; })
     ];
   };
 
   # Sage expects those in the same directory.
   pari_data = symlinkJoin {
     name = "pari_data";
-    paths = with nixpkgs; [
+    paths = with pkgs; [
       pari-galdata
       pari-seadata-small
     ];
   };
 
   # https://trac.sagemath.org/ticket/22191
-  ecl = nixpkgs.ecl_16_1_2;
+  ecl = pkgs.ecl_16_1_2;
 in
-  python.pkgs.sage-wrapper // {
-    doc = python.pkgs.sagedoc;
-    lib = python.pkgs.sagelib;
-  }
+# A wrapper around sage that makes sure sage finds its docs (if they were build).
+callPackage ./sage.nix {
+  inherit sage-tests sage-with-env sagedoc;
+  inherit withDoc;
+}
diff --git a/pkgs/applications/science/math/sage/env-locations.nix b/pkgs/applications/science/math/sage/env-locations.nix
index 098ce3925cf3..9d94e9ca5e34 100644
--- a/pkgs/applications/science/math/sage/env-locations.nix
+++ b/pkgs/applications/science/math/sage/env-locations.nix
@@ -16,6 +16,8 @@
 , cysignals
 }:
 
+# A bash script setting various environment variables to tell sage where
+# the files its looking fore are located. Also see `sage-env`.
 writeTextFile rec {
   name = "sage-env-locations";
   destination = "/${name}";
diff --git a/pkgs/applications/science/math/sage/openblas-pc.nix b/pkgs/applications/science/math/sage/openblas-pc.nix
deleted file mode 100644
index f4669a6557e9..000000000000
--- a/pkgs/applications/science/math/sage/openblas-pc.nix
+++ /dev/null
@@ -1,17 +0,0 @@
-{ openblasCompat
-, writeTextFile
-, name
-}:
-
-writeTextFile {
-  name = "openblas-${name}-pc-${openblasCompat.version}";
-  destination = "/lib/pkgconfig/${name}.pc";
-  text = ''
-    Name: ${name}
-    Version: ${openblasCompat.version}
-
-    Description: ${name} for SageMath, provided by the OpenBLAS package.
-    Cflags: -I${openblasCompat}/include
-    Libs: -L${openblasCompat}/lib -lopenblas
-  '';
-}
diff --git a/pkgs/applications/science/math/sage/patches/eclib-20180710.patch b/pkgs/applications/science/math/sage/patches/eclib-20180710.patch
deleted file mode 100644
index 986ae42aeb62..000000000000
--- a/pkgs/applications/science/math/sage/patches/eclib-20180710.patch
+++ /dev/null
@@ -1,16 +0,0 @@
-diff --git a/src/sage/interfaces/mwrank.py b/src/sage/interfaces/mwrank.py
-index 4417b59276..ae57ca2991 100644
---- a/src/sage/interfaces/mwrank.py
-+++ b/src/sage/interfaces/mwrank.py
-@@ -54,8 +54,9 @@ def Mwrank(options="", server=None, server_tmpdir=None):
-         sage: M = Mwrank('-v 0 -l')
-         sage: print(M('0 0 1 -1 0'))
-         Curve [0,0,1,-1,0] :    Rank = 1
--        Generator 1 is [0:-1:1]; height 0.0511114082399688
--        Regulator = 0.0511114082399688
-+        Generator 1 is [0:-1:1]; height 0.051111408239969
-+        Regulator = 0.051111408239969
-+
-     """
-     global instances
-     try:
diff --git a/pkgs/applications/science/math/sage/patches/eclib-regulator-precision.patch b/pkgs/applications/science/math/sage/patches/eclib-regulator-precision.patch
deleted file mode 100644
index a1464b8fe310..000000000000
--- a/pkgs/applications/science/math/sage/patches/eclib-regulator-precision.patch
+++ /dev/null
@@ -1,98 +0,0 @@
-diff --git a/src/sage/libs/eclib/interface.py b/src/sage/libs/eclib/interface.py
-index f77000c478..9d17d412ae 100644
---- a/src/sage/libs/eclib/interface.py
-+++ b/src/sage/libs/eclib/interface.py
-@@ -1014,7 +1014,7 @@ class mwrank_MordellWeil(SageObject):
-             WARNING: saturation at primes p > 2 will not be done;
-             ...
-             Gained index 2
--            New regulator =  93.857300720636393209
-+            New regulator =  93.85730...
-             (False, 2, '[ ]')
-             sage: EQ.points()
-             [[-2, 3, 1], [2707496766203306, 864581029138191, 2969715140223272], [-13422227300, -49322830557, 12167000000]]
-@@ -1025,7 +1025,7 @@ class mwrank_MordellWeil(SageObject):
-             WARNING: saturation at primes p > 3 will not be done;
-             ...
-             Gained index 3
--            New regulator =  10.4285889689595992455
-+            New regulator =  10.42858...
-             (False, 3, '[ ]')
-             sage: EQ.points()
-             [[-2, 3, 1], [-14, 25, 8], [-13422227300, -49322830557, 12167000000]]
-@@ -1036,7 +1036,7 @@ class mwrank_MordellWeil(SageObject):
-             WARNING: saturation at primes p > 5 will not be done;
-             ...
-             Gained index 5
--            New regulator =  0.417143558758383969818
-+            New regulator =  0.41714...
-             (False, 5, '[ ]')
-             sage: EQ.points()
-             [[-2, 3, 1], [-14, 25, 8], [1, -1, 1]]
-@@ -1221,7 +1221,7 @@ class mwrank_MordellWeil(SageObject):
-             WARNING: saturation at primes p > 2 will not be done;
-             ...
-             Gained index 2
--            New regulator =  93.857300720636393209
-+            New regulator =  93.85730...
-             (False, 2, '[ ]')
-             sage: EQ
-             Subgroup of Mordell-Weil group: [[-2:3:1], [2707496766203306:864581029138191:2969715140223272], [-13422227300:-49322830557:12167000000]]
-@@ -1235,7 +1235,7 @@ class mwrank_MordellWeil(SageObject):
-             WARNING: saturation at primes p > 3 will not be done;
-             ...
-             Gained index 3
--            New regulator =  10.4285889689595992455
-+            New regulator =  10.42858...
-             (False, 3, '[ ]')
-             sage: EQ
-             Subgroup of Mordell-Weil group: [[-2:3:1], [-14:25:8], [-13422227300:-49322830557:12167000000]]
-@@ -1249,7 +1249,7 @@ class mwrank_MordellWeil(SageObject):
-             WARNING: saturation at primes p > 5 will not be done;
-             ...
-             Gained index 5
--            New regulator =  0.417143558758383969818
-+            New regulator =  0.41714...
-             (False, 5, '[ ]')
-             sage: EQ
-             Subgroup of Mordell-Weil group: [[-2:3:1], [-14:25:8], [1:-1:1]]
-diff --git a/src/sage/libs/eclib/mwrank.pyx b/src/sage/libs/eclib/mwrank.pyx
-index a4f89e1ca5..f8a22d2f55 100644
---- a/src/sage/libs/eclib/mwrank.pyx
-+++ b/src/sage/libs/eclib/mwrank.pyx
-@@ -1234,9 +1234,9 @@ cdef class _two_descent:
-             sage: D2.saturate()
-             Searching for points (bound = 8)...done:
-               found points which generate a subgroup of rank 3
--              and regulator 0.417143558758383969817119544618093396749810106098479
-+              and regulator 0.41714...
-             Processing points found during 2-descent...done:
--              now regulator = 0.417143558758383969817119544618093396749810106098479
-+              now regulator = 0.41714...
-             No saturation being done
-             sage: D2.getbasis()
-             '[[1:-1:1], [-2:3:1], [-14:25:8]]'
-@@ -1281,9 +1281,9 @@ cdef class _two_descent:
-             sage: D2.saturate()
-             Searching for points (bound = 8)...done:
-               found points which generate a subgroup of rank 3
--              and regulator 0.417143558758383969817119544618093396749810106098479
-+              and regulator 0.41714...
-             Processing points found during 2-descent...done:
--              now regulator = 0.417143558758383969817119544618093396749810106098479
-+              now regulator = 0.41714...
-             No saturation being done
-             sage: D2.getbasis()
-             '[[1:-1:1], [-2:3:1], [-14:25:8]]'
-@@ -1329,9 +1329,9 @@ cdef class _two_descent:
-             sage: D2.saturate()
-             Searching for points (bound = 8)...done:
-               found points which generate a subgroup of rank 3
--              and regulator 0.417143558758383969817119544618093396749810106098479
-+              and regulator 0.41714...
-             Processing points found during 2-descent...done:
--              now regulator = 0.417143558758383969817119544618093396749810106098479
-+              now regulator = 0.41714...
-             No saturation being done
-             sage: D2.getbasis()
-             '[[1:-1:1], [-2:3:1], [-14:25:8]]'
diff --git a/pkgs/applications/science/math/sage/patches/known-padics-bug.patch b/pkgs/applications/science/math/sage/patches/known-padics-bug.patch
deleted file mode 100644
index bdccd73e0ce1..000000000000
--- a/pkgs/applications/science/math/sage/patches/known-padics-bug.patch
+++ /dev/null
@@ -1,15 +0,0 @@
-diff --git a/build/pkgs/openblas/package-version.txt b/build/pkgs/openblas/package-version.txt
-index 3bc45c25d4..7c7c224887 100644
---- a/src/sage/schemes/elliptic_curves/padics.py
-+++ b/src/sage/schemes/elliptic_curves/padics.py
-@@ -292,8 +292,8 @@ def padic_regulator(self, p, prec=20, height=None, check_hypotheses=True):
- 
-         sage: max_prec = 30    # make sure we get past p^2    # long time
-         sage: full = E.padic_regulator(5, max_prec)           # long time
--        sage: for prec in range(1, max_prec):                 # long time
--        ....:     assert E.padic_regulator(5, prec) == full   # long time
-+        sage: for prec in range(1, max_prec):                 # known bug (#25969) # long time
-+        ....:     assert E.padic_regulator(5, prec) == full   # known bug (#25969) # long time
- 
-     A case where the generator belongs to the formal group already
-     (:trac:`3632`)::
diff --git a/pkgs/applications/science/math/sage/patches/matplotlib-normed-deprecated.patch b/pkgs/applications/science/math/sage/patches/matplotlib-normed-deprecated.patch
deleted file mode 100644
index 41747635cacd..000000000000
--- a/pkgs/applications/science/math/sage/patches/matplotlib-normed-deprecated.patch
+++ /dev/null
@@ -1,12 +0,0 @@
-diff --git a/src/sage/all.py b/src/sage/all.py
-index 14cec431f1..25a35a0522 100644
---- a/src/sage/all.py
-+++ b/src/sage/all.py
-@@ -310,6 +310,7 @@ warnings.filters.remove(('ignore', None, DeprecationWarning, None, 0))
- # Ignore all deprecations from IPython etc.
- warnings.filterwarnings('ignore',
-     module='.*(IPython|ipykernel|jupyter_client|jupyter_core|nbformat|notebook|ipywidgets|storemagic)')
-+warnings.filterwarnings('ignore', "The 'normed' kwarg is deprecated, and has been replaced by the 'density' kwarg.") # matplotlib normed deprecation
- # However, be sure to keep OUR deprecation warnings
- warnings.filterwarnings('default',
-     '[\s\S]*See http://trac.sagemath.org/[0-9]* for details.')
diff --git a/pkgs/applications/science/math/sage/sage-env.nix b/pkgs/applications/science/math/sage/sage-env.nix
index 317eb6e16c49..725ca0438670 100644
--- a/pkgs/applications/science/math/sage/sage-env.nix
+++ b/pkgs/applications/science/math/sage/sage-env.nix
@@ -2,7 +2,6 @@
 , lib
 , writeTextFile
 , python
-, sage-src
 , sagelib
 , env-locations
 , gfortran
@@ -37,7 +36,7 @@
 , lcalc
 , rubiks
 , flintqs
-, openblas-cblas-pc
+, openblasCompat
 , flint
 , gmp
 , mpfr
@@ -47,6 +46,10 @@
 , ntl
 }:
 
+# This generates a `sage-env` shell file that will be sourced by sage on startup.
+# It sets up various environment variables, telling sage where to find its
+# dependencies.
+
 let
   runtimepath = (lib.makeBinPath ([
     "@sage-local@"
@@ -96,26 +99,27 @@ writeTextFile rec {
   destination = "/${name}";
   text = ''
     export PKG_CONFIG_PATH='${lib.concatStringsSep ":" (map (pkg: "${pkg}/lib/pkgconfig") [
-        # This is only needed in the src/sage/misc/cython.py test and I'm not sure if there's really a use-case
-        # for it outside of the tests. However since singular and openblas are runtime dependencies anyways
-        # and openblas-cblas-pc is tiny, it doesn't really hurt to include.
+        # This is only needed in the src/sage/misc/cython.py test and I'm not
+        # sure if there's really a usecase for it outside of the tests. However
+        # since singular and openblas are runtime dependencies anyways, it doesn't
+        # really hurt to include.
         singular
-        openblas-cblas-pc
+        openblasCompat
       ])
     }'
-    export SAGE_ROOT='${sage-src}'
+    export SAGE_ROOT='${sagelib.src}'
     export SAGE_LOCAL='@sage-local@'
     export SAGE_SHARE='${sagelib}/share'
     orig_path="$PATH"
     export PATH='${runtimepath}'
 
     # set dependent vars, like JUPYTER_CONFIG_DIR
-    source "${sage-src}/src/bin/sage-env"
+    source "${sagelib.src}/src/bin/sage-env"
     export PATH="${runtimepath}:$orig_path" # sage-env messes with PATH
 
     export SAGE_LOGS="$TMPDIR/sage-logs"
     export SAGE_DOC="''${SAGE_DOC_OVERRIDE:-doc-placeholder}"
-    export SAGE_DOC_SRC="''${SAGE_DOC_SRC_OVERRIDE:-${sage-src}/src/doc}"
+    export SAGE_DOC_SRC="''${SAGE_DOC_SRC_OVERRIDE:-${sagelib.src}/src/doc}"
 
     # set locations of dependencies
     . ${env-locations}/sage-env-locations
@@ -154,9 +158,11 @@ writeTextFile rec {
 
     export SAGE_LIB='${sagelib}/${python.sitePackages}'
 
-    export SAGE_EXTCODE='${sage-src}/src/ext'
+    export SAGE_EXTCODE='${sagelib.src}/src/ext'
 
-    # for find_library
+  # for find_library
     export DYLD_LIBRARY_PATH="${lib.makeLibraryPath [stdenv.cc.libc singular]}:$DYLD_LIBRARY_PATH"
   '';
+} // {
+  lib = sagelib; # equivalent of `passthru`, which `writeTextFile` doesn't support
 }
diff --git a/pkgs/applications/science/math/sage/sage-src.nix b/pkgs/applications/science/math/sage/sage-src.nix
index 295125eefd1e..5d729d9c5fdd 100644
--- a/pkgs/applications/science/math/sage/sage-src.nix
+++ b/pkgs/applications/science/math/sage/sage-src.nix
@@ -2,6 +2,12 @@
 , fetchFromGitHub
 , fetchpatch
 }:
+
+# This file is responsible for fetching the sage source and adding necessary patches.
+# It does not actually build anything, it just copies the patched sources to $out.
+# This is done because multiple derivations rely on these sources and they should
+# all get the same sources with the same patches applied.
+
 stdenv.mkDerivation rec {
   version = "8.4";
   name = "sage-src-${version}";
@@ -13,6 +19,8 @@ stdenv.mkDerivation rec {
     sha256 = "0gips1hagiz9m7s21bg5as8hrrm2x5k47h1bsq0pc46iplfwmv2d";
   };
 
+  # Patches needed because of particularities of nix or the way this is packaged.
+  # The goal is to upstream all of them and get rid of this list.
   nixPatches = [
     # https://trac.sagemath.org/ticket/25358
     (fetchpatch {
@@ -31,6 +39,7 @@ stdenv.mkDerivation rec {
     # Revert the commit that made the sphinx build fork even in the single thread
     # case. For some yet unknown reason, that breaks the docbuild on nix and archlinux.
     # See https://groups.google.com/forum/#!msg/sage-packaging/VU4h8IWGFLA/mrmCMocYBwAJ.
+    # https://trac.sagemath.org/ticket/26608
     ./patches/revert-sphinx-always-fork.patch
 
     # Make sure py2/py3 tests are only run when their expected context (all "sage"
@@ -39,10 +48,16 @@ stdenv.mkDerivation rec {
     ./patches/Only-test-py2-py3-optional-tests-when-all-of-sage-is.patch
   ];
 
+  # Patches needed because of package updates. We could just pin the versions of
+  # dependencies, but that would lead to rebuilds, confusion and the burdons of
+  # maintaining multiple versions of dependencies. Instead we try to make sage
+  # compatible with never dependency versions when possible. All these changes
+  # should come from or be proposed to upstream. This list will probably never
+  # be empty since dependencies update all the time.
   packageUpgradePatches = let
-    # fetch a diff between base and rev on sage's git server
-    # used to fetch trac tickets by setting the base to the release and the
-    # revision to the last commit that should be included
+    # Fetch a diff between `base` and `rev` on sage's git server.
+    # Used to fetch trac tickets by setting the `base` to the last release and the
+    # `rev` to the last commit of the ticket.
     fetchSageDiff = { base, rev, ...}@args: (
       fetchpatch ({
         url = "https://git.sagemath.org/sage.git/patch?id2=${base}&id=${rev}";
@@ -54,7 +69,7 @@ stdenv.mkDerivation rec {
   in [
     # New glpk version has new warnings, filter those out until upstream sage has found a solution
     # https://trac.sagemath.org/ticket/24824
-    ./patches/pari-stackwarn.patch # not actually necessary since tha pari upgrade, but necessary for the glpk patch to apply
+    ./patches/pari-stackwarn.patch # not actually necessary since the pari upgrade, but necessary for the glpk patch to apply
     (fetchpatch {
       url = "https://salsa.debian.org/science-team/sagemath/raw/58bbba93a807ca2933ca317501d093a1bb4b84db/debian/patches/dt-version-glpk-4.65-ignore-warnings.patch";
       sha256 = "0b9293v73wb4x13wv5zwyjgclc01zn16msccfzzi6znswklgvddp";
@@ -64,8 +79,8 @@ stdenv.mkDerivation rec {
     # https://trac.sagemath.org/ticket/25260
     ./patches/numpy-1.15.1.patch
 
-    # ntl upgrade
-    # https://trac.sagemath.org/ticket/25532#comment:29
+    # needed for ntl update
+    # https://trac.sagemath.org/ticket/25532
     (fetchpatch {
       name = "lcalc-c++11.patch";
       url = "https://git.archlinux.org/svntogit/community.git/plain/trunk/sagemath-lcalc-c++11.patch?h=packages/sagemath&id=0e31ae526ab7c6b5c0bfacb3f8b1c4fd490035aa";
@@ -100,9 +115,7 @@ stdenv.mkDerivation rec {
     })
   ];
 
-  patches = nixPatches ++ packageUpgradePatches ++ [
-    ./patches/known-padics-bug.patch
-  ];
+  patches = nixPatches ++ packageUpgradePatches;
 
   postPatch = ''
     # make sure shebangs etc are fixed, but sage-python23 still works
diff --git a/pkgs/applications/science/math/sage/sage-tests.nix b/pkgs/applications/science/math/sage/sage-tests.nix
new file mode 100644
index 000000000000..feccf75a7acb
--- /dev/null
+++ b/pkgs/applications/science/math/sage/sage-tests.nix
@@ -0,0 +1,51 @@
+{ stdenv
+, lib
+, sage-with-env
+, makeWrapper
+, files ? null # "null" means run all tests
+, longTests ? true # run tests marked as "long time"
+}:
+
+# for a quick test of some source files:
+# nix-build -E 'with (import ./. {}); sage.tests.override { files = [ "src/sage/misc/cython.py" ];}'
+
+let
+  src = sage-with-env.env.lib.src;
+  runAllTests = files == null;
+  testArgs = if runAllTests then "--all" else testFileList;
+  patienceSpecifier = if longTests then "--long" else "";
+  relpathToArg = relpath: lib.escapeShellArg "${src}/${relpath}"; # paths need to be absolute
+  testFileList = lib.concatStringsSep " " (map relpathToArg files);
+in
+stdenv.mkDerivation rec {
+  version = src.version;
+  name = "sage-tests-${version}";
+  inherit src;
+
+  buildInputs = [
+    makeWrapper
+    sage-with-env
+  ];
+
+  unpackPhase = "#do nothing";
+  configurePhase = "#do nothing";
+  buildPhase = "#do nothing";
+
+  installPhase = ''
+    # This output is not actually needed for anything, the package just
+    # exists to decouple the sage build from its t ests.
+
+    mkdir -p "$out/bin"
+    # Like a symlink, but make sure that $0 points to the original.
+    makeWrapper "${sage-with-env}/bin/sage" "$out/bin/sage"
+  '';
+
+  doInstallCheck = true;
+  installCheckPhase = ''
+    export HOME="$TMPDIR/sage-home"
+    mkdir -p "$HOME"
+
+    # "--long" tests are in the order of 1h, without "--long" its 1/2h
+    "sage" -t --nthreads "$NIX_BUILD_CORES" --optional=sage ${patienceSpecifier} ${testArgs}
+  '';
+}
diff --git a/pkgs/applications/science/math/sage/sage-with-env.nix b/pkgs/applications/science/math/sage/sage-with-env.nix
index 63b9772b8231..c5db392f1036 100644
--- a/pkgs/applications/science/math/sage/sage-with-env.nix
+++ b/pkgs/applications/science/math/sage/sage-with-env.nix
@@ -2,11 +2,7 @@
 , lib
 , makeWrapper
 , sage-env
-, sage-src
 , openblasCompat
-, openblas-blas-pc
-, openblas-cblas-pc
-, openblas-lapack-pc
 , pkg-config
 , three
 , singular
@@ -26,15 +22,15 @@
 , pythonEnv
 }:
 
+# Wrapper that combined `sagelib` with `sage-env` to produce an actually
+# executable sage. No tests are run yet and no documentation is built.
+
 let
   buildInputs = [
     pythonEnv # for patchShebangs
     makeWrapper
     pkg-config
     openblasCompat # lots of segfaults with regular (64 bit) openblas
-    openblas-blas-pc
-    openblas-cblas-pc
-    openblas-lapack-pc
     singular
     three
     pynac
@@ -92,13 +88,12 @@ let
   input_names = map (dep: pkg_to_spkg_name dep patch_names) transitiveDeps;
 in
 stdenv.mkDerivation rec {
-  version = sage-src.version;
+  version = src.version;
   name = "sage-with-env-${version}";
+  src = sage-env.lib.src;
 
   inherit buildInputs;
 
-  src = sage-src;
-
   configurePhase = "#do nothing";
 
   buildPhase = ''
@@ -110,17 +105,24 @@ stdenv.mkDerivation rec {
 
   installPhase = ''
     mkdir -p "$out/var/lib/sage"
-    cp -r installed $out/var/lib/sage
+    cp -r installed "$out/var/lib/sage"
 
     mkdir -p "$out/etc"
     # sage tests will try to create this file if it doesn't exist
     touch "$out/etc/sage-started.txt"
 
     mkdir -p "$out/build"
+
+    # the scripts in src/bin will find the actual sage source files using environment variables set in `sage-env`
     cp -r src/bin "$out/bin"
     cp -r build/bin "$out/build/bin"
+
     cp -f '${sage-env}/sage-env' "$out/bin/sage-env"
     substituteInPlace "$out/bin/sage-env" \
       --subst-var-by sage-local "$out"
   '';
+
+  passthru = {
+    env = sage-env;
+  };
 }
diff --git a/pkgs/applications/science/math/sage/sage-wrapper.nix b/pkgs/applications/science/math/sage/sage-wrapper.nix
deleted file mode 100644
index 4b2f9c461c18..000000000000
--- a/pkgs/applications/science/math/sage/sage-wrapper.nix
+++ /dev/null
@@ -1,41 +0,0 @@
-{ stdenv
-, makeWrapper
-, sage
-, sage-src
-, sagedoc
-, withDoc
-}:
-
-stdenv.mkDerivation rec {
-  version = sage.version;
-  name = "sage-${version}";
-
-  buildInputs = [
-    makeWrapper
-  ];
-
-  unpackPhase = "#do nothing";
-  configurePhase = "#do nothing";
-  buildPhase = "#do nothing";
-
-  installPhase = ''
-    mkdir -p "$out/bin"
-    makeWrapper "${sage}/bin/sage" "$out/bin/sage" \
-      --set SAGE_DOC_SRC_OVERRIDE "${sage-src}/src/doc" ${
-      stdenv.lib.optionalString withDoc "--set SAGE_DOC_OVERRIDE ${sagedoc}/share/doc/sage"
-    }
-  '';
-
-  doInstallCheck = withDoc;
-  installCheckPhase = ''
-    export HOME="$TMPDIR/sage-home"
-    mkdir -p "$HOME"
-    "$out/bin/sage" -c 'browse_sage_doc._open("reference", testing=True)'
-  '';
-
-  meta = with stdenv.lib; {
-    description = "Open Source Mathematics Software, free alternative to Magma, Maple, Mathematica, and Matlab";
-    license = licenses.gpl2;
-    maintainers = with maintainers; [ timokau ];
-  };
-}
diff --git a/pkgs/applications/science/math/sage/sage.nix b/pkgs/applications/science/math/sage/sage.nix
index ad9a32e0ca56..ba8fc095a67e 100644
--- a/pkgs/applications/science/math/sage/sage.nix
+++ b/pkgs/applications/science/math/sage/sage.nix
@@ -1,14 +1,24 @@
 { stdenv
-, sage-with-env
 , makeWrapper
+, sage-tests
+, sage-with-env
+, sagedoc
+, withDoc
 }:
 
+# A wrapper that makes sure sage finds its docs (if they were build).
+
 stdenv.mkDerivation rec {
-  version = sage-with-env.version;
-  name = "sage-tests-${version}";
+  version = src.version;
+  name = "sage-${version}";
+  src = sage-with-env.env.lib.src;
 
   buildInputs = [
     makeWrapper
+
+    # This is a hack to make sure sage-tests is evaluated. It doesn't acutally
+    # produce anything of value, it just decouples the tests from the build.
+    sage-tests
   ];
 
   unpackPhase = "#do nothing";
@@ -17,16 +27,28 @@ stdenv.mkDerivation rec {
 
   installPhase = ''
     mkdir -p "$out/bin"
-    # Like a symlink, but make sure that $0 points to the original.
-    makeWrapper "${sage-with-env}/bin/sage" "$out/bin/sage"
+    makeWrapper "${sage-with-env}/bin/sage" "$out/bin/sage" \
+      --set SAGE_DOC_SRC_OVERRIDE "${src}/src/doc" ${
+      stdenv.lib.optionalString withDoc "--set SAGE_DOC_OVERRIDE ${sagedoc}/share/doc/sage"
+    }
   '';
 
-  doInstallCheck = true;
+  doInstallCheck = withDoc;
   installCheckPhase = ''
     export HOME="$TMPDIR/sage-home"
     mkdir -p "$HOME"
-
-    # "--long" tests are in the order of 1h, without "--long" its 1/2h
-    "$out/bin/sage" -t --nthreads "$NIX_BUILD_CORES" --optional=sage --long --all
+    "$out/bin/sage" -c 'browse_sage_doc._open("reference", testing=True)'
   '';
+
+  passthru = {
+    tests = sage-tests;
+    doc = sagedoc;
+    lib = sage-with-env.env.lib;
+  };
+
+  meta = with stdenv.lib; {
+    description = "Open Source Mathematics Software, free alternative to Magma, Maple, Mathematica, and Matlab";
+    license = licenses.gpl2;
+    maintainers = with maintainers; [ timokau ];
+  };
 }
diff --git a/pkgs/applications/science/math/sage/sagedoc.nix b/pkgs/applications/science/math/sage/sagedoc.nix
index a594428a389a..cc1a4fc61e0c 100644
--- a/pkgs/applications/science/math/sage/sagedoc.nix
+++ b/pkgs/applications/science/math/sage/sagedoc.nix
@@ -1,63 +1,49 @@
 { stdenv
-, sage-src
 , sage-with-env
-, sagelib
-, python2
-, psutil
-, future
-, sphinx
-, sagenb
+, python
 , maxima-ecl
-, networkx
-, scipy
-, sympy
-, matplotlib
-, pillow
-, ipykernel
-, jupyter_client
 , tachyon
 , jmol
-, ipywidgets
-, typing
 , cddlib
-, pybrial
 }:
 
 stdenv.mkDerivation rec {
-  version = sage-src.version;
+  version = src.version;
   name = "sagedoc-${version}";
+  src = sage-with-env.env.lib.src;
 
 
   # Building the documentation has many dependencies, because all documented
   # modules are imported and because matplotlib is used to produce plots.
   buildInputs = [
-    sagelib
-    python2
+    sage-with-env.env.lib
+    python
+    maxima-ecl
+    tachyon
+    jmol
+    cddlib
+  ] ++ (with python.pkgs; [
     psutil
     future
     sphinx
     sagenb
-    maxima-ecl
-    networkx
     scipy
     sympy
     matplotlib
     pillow
+    networkx
     ipykernel
-    jupyter_client
-    tachyon
-    jmol
     ipywidgets
+    jupyter_client
     typing
-    cddlib
     pybrial
-  ];
+  ]);
 
   unpackPhase = ''
     export SAGE_DOC_OVERRIDE="$PWD/share/doc/sage"
     export SAGE_DOC_SRC_OVERRIDE="$PWD/docsrc"
 
-    cp -r "${sage-src}/src/doc" "$SAGE_DOC_SRC_OVERRIDE"
+    cp -r "${src}/src/doc" "$SAGE_DOC_SRC_OVERRIDE"
     chmod -R 755 "$SAGE_DOC_SRC_OVERRIDE"
   '';
 
diff --git a/pkgs/applications/science/math/sage/sagelib.nix b/pkgs/applications/science/math/sage/sagelib.nix
index d26f5dad724e..03b1ecd2c0b7 100644
--- a/pkgs/applications/science/math/sage/sagelib.nix
+++ b/pkgs/applications/science/math/sage/sagelib.nix
@@ -3,9 +3,6 @@
 , buildPythonPackage
 , arb
 , openblasCompat
-, openblas-blas-pc
-, openblas-cblas-pc
-, openblas-lapack-pc
 , brial
 , cliquer
 , cypari2
@@ -51,19 +48,20 @@
 , libbraiding
 }:
 
+# This is the core sage python package. Everything else is just wrappers gluing
+# stuff together. It is not very useful on its own though, since it will not
+# find many of its dependencies without `sage-env`, will not be tested without
+# `sage-tests` and will not have html docs without `sagedoc`.
+
 buildPythonPackage rec {
   format = "other";
-  version = sage-src.version;
-  pname = "sagelib";
-
+  version = src.version;
+  name = "sagelib-${version}";
   src = sage-src;
 
   nativeBuildInputs = [
     iml
     perl
-    openblas-blas-pc
-    openblas-cblas-pc
-    openblas-lapack-pc
     jupyter_core
   ];
 
diff --git a/pkgs/applications/science/math/sage/sagenb.nix b/pkgs/applications/science/math/sage/sagenb.nix
index cc883cc24b42..5adfde4388a7 100644
--- a/pkgs/applications/science/math/sage/sagenb.nix
+++ b/pkgs/applications/science/math/sage/sagenb.nix
@@ -1,4 +1,3 @@
-# Has a cyclic dependency with sage (not expressed here) and is not useful outside of sage
 { stdenv
 , fetchpatch
 , python
@@ -13,6 +12,10 @@
 , flask-babel
 }:
 
+# Has a cyclic dependency with sage (not expressed here) and is not useful outside of sage.
+# Deprecated, hopefully soon to be removed. See
+# https://trac.sagemath.org/ticket/25837
+
 buildPythonPackage rec {
   pname = "sagenb";
   version = "2018-06-26"; # not 1.0.1 because of new flask syntax