sage: refactor

12 files changed, 211 insertions, 188 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..4eed0d411d9b 100644
--- a/pkgs/applications/science/math/sage/default.nix
+++ b/pkgs/applications/science/math/sage/default.nix
@@ -1,65 +1,87 @@
-{ 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_8_x;
 
   # 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 {};
+      sagenb = self.callPackage ./sagenb.nix {
+        mathjax = nodePackages_8_x.mathjax;
+      };
 
+      # Package with a cyclic dependency with sage
       pybrial = self.callPackage ./pybrial.nix {};
 
+      # `sagelib`, i.e. all of sage except some wrappers and runtime dependencies
       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;
+        linbox = pkgs.linbox.override { withSage = true; };
       };
+    };
+  };
 
-      env-locations = self.callPackage ./env-locations.nix {
-        inherit pari_data ecl;
-        inherit singular;
-        three = nodePackages.three;
-        mathjax = nodePackages.mathjax;
-      };
+  # 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_8_x.three;
+    mathjax = nodePackages_8_x.mathjax;
+  };
 
-      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
-      };
+  # 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 openblas-cblas-pc ecl singular palp flint pynac pythonEnv;
+    pkg-config = pkgs.pkgconfig; # not to confuse with pythonPackages.pkgconfig
+  };
 
-      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 documentation for sage, building it takes a lot of ram.
+  sagedoc = callPackage ./sagedoc.nix {
+    inherit sage-with-env;
+    inherit python;
+  };
 
-      sage = self.callPackage ./sage.nix { };
+  # 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 openblas-blas-pc openblas-cblas-pc openblas-lapack-pc pynac singular;
+    pkg-config = pkgs.pkgconfig; # not to confuse with pythonPackages.pkgconfig
+    three = nodePackages_8_x.three;
+  };
 
-      sage-wrapper = self.callPackage ./sage-wrapper.nix {
-        inherit sage-src withDoc;
-      };
-    };
+  # 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;
   };
 
+  # FIXME
   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"; };
@@ -91,9 +113,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 +124,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/sage-env.nix b/pkgs/applications/science/math/sage/sage-env.nix
index 317eb6e16c49..42050b189ec5 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
@@ -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@"
@@ -103,19 +106,19 @@ writeTextFile rec {
         openblas-cblas-pc
       ])
     }'
-    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 +157,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 7ec58fdb1db3..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 {
@@ -40,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}";
@@ -65,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";
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..fdaf32a88ab3
--- /dev/null
+++ b/pkgs/applications/science/math/sage/sage-tests.nix
@@ -0,0 +1,36 @@
+{ stdenv
+, sage-with-env
+, makeWrapper
+}:
+
+stdenv.mkDerivation rec {
+  version = src.version;
+  name = "sage-tests-${version}";
+  src = sage-with-env.env.lib.src;
+
+  buildInputs = [
+    makeWrapper
+  ];
+
+  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
+    "$out/bin/sage" -t --nthreads "$NIX_BUILD_CORES" --optional=sage --long --all
+  '';
+}
diff --git a/pkgs/applications/science/math/sage/sage-with-env.nix b/pkgs/applications/science/math/sage/sage-with-env.nix
index 63b9772b8231..16e4a8aa03cd 100644
--- a/pkgs/applications/science/math/sage/sage-with-env.nix
+++ b/pkgs/applications/science/math/sage/sage-with-env.nix
@@ -2,7 +2,6 @@
 , lib
 , makeWrapper
 , sage-env
-, sage-src
 , openblasCompat
 , openblas-blas-pc
 , openblas-cblas-pc
@@ -26,6 +25,9 @@
 , 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
@@ -92,13 +94,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 +111,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..60d91ea423af 100644
--- a/pkgs/applications/science/math/sage/sagelib.nix
+++ b/pkgs/applications/science/math/sage/sagelib.nix
@@ -51,11 +51,15 @@
 , 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 = [
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