Set JUPYTER_DATA_DIR etc. during docbuild

[mkoeppe]

After #33507, the jupyter kernel sagemath gets invoked during the docbuild.
This can lead to a new class of errors as seen in #33507 comment:75 and #33507 comment:86

We fix it by setting JUPYTER_DATA_DIR and JUPYTER_PATH during the docbuild so that a random broken user configuration of Jupyter in ~/.jupyter does not break the docbuild.

See https://docs.jupyter.org/en/latest/use/jupyter-directories.html#configuration-files

CC: @kwankyu @dimpase @collares

Component: documentation

Author: Matthias Koeppe

Branch: 614822c

Reviewer: Kwankyu Lee

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

[mkoeppe]

comment:2

This could be done, for example, in build/pkgs/sagemath_doc_html/spkg-install

[kwankyu]

comment:3

We fix it by setting JUPYTER_CONFIG_DIR and JUPYTER_CONFIG_PATH during the docbuild so that a random broken user configuration of Jupyter in ~/.jupyter does not break the docbuild.

Dima had the same issue with ./sage -n. So we should not focus on the docbuild.

[mkoeppe]

comment:4

Well, sage -n is for running Jupyter, so I'd think it's correct to use the user's configuration - so it's the user's problem if their personal configuration in ~/.jupyter is overriding our kernel.

But for the build process we should do better.

[mkoeppe]

Branch: u/mkoeppe/set_jupyter_config_dir_during_docbuild

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Commit: b397afc

[mkoeppe]

New commits:

b397afc

build/bin/sage-site (--docbuild): Poison JUPYTER_CONFIG_DIR, JUPYTER_CONFIG_PATH

[mkoeppe]

comment:7

In addition, I suppose in sage -n we could warn the user if jupyter kernelspec list reveals that our kernels are shadowed by user configuration.

[mkoeppe]

comment:8

I've opened #33651 for this.

[kwankyu]

comment:10

Why not unset VAR instead of pointing jupyter to non-existing directory or path?

[mkoeppe]

comment:11

When unset, it uses the default directories, which can contain the bad user configuration.

[kwankyu]

comment:12

Replying to @mkoeppe:

When unset, it uses the default directories, which can contain the bad user configuration.

I see.

Then it looks good to me. Now I am simulating the failing situation to see that the branch works as expected.

Meanwhile let me note that #33320 (which supersedes #33507) circumvents this bug since it does not run jupyter to build the live doc.

[kwankyu]

comment:13

In /Users/kwankyu/GitHub/sage-dev:

After

(sage-sh) kwankyu@Hera:sage-dev$ ipython kernel install --name "sagemath" --user
Installed kernelspec sagemath in /Users/kwankyu/Library/Jupyter/kernels/sagemath
(sage-sh) kwankyu@Hera:sage-dev$ jupyter kernelspec list
Available kernels:
  sagemath        /Users/kwankyu/Library/Jupyter/kernels/sagemath
  python3         /Users/kwankyu/GitHub/sage-dev/local/var/lib/sage/venv-python3.10.3/share/jupyter/kernels/python3

make fails even with the branch.

After

(sage-sh) kwankyu@Hera:sage-dev$ jupyter kernelspec remove sagemath
Kernel specs to remove:
  sagemath            	/Users/kwankyu/Library/Jupyter/kernels/sagemath
Remove 1 kernel specs [y/N]: y
[RemoveKernelSpec] Removed /Users/kwankyu/Library/Jupyter/kernels/sagemath
(sage-sh) kwankyu@Hera:sage-dev$ jupyter kernelspec list
Available kernels:
  python3         /Users/kwankyu/GitHub/sage-dev/local/var/lib/sage/venv-python3.10.3/share/jupyter/kernels/python3
  sagemath        /Users/kwankyu/GitHub/sage-dev/local/var/lib/sage/venv-python3.10.3/share/jupyter/kernels/sagemath

make does not fail.

[kwankyu]

comment:14

This

export JUPYTER_DATA_DIR=/doesnotexist  

worked. But as I see it, this is still not a safe solution. It does prevent from using default directories, but still .sage/local/etc/jupyter shadows our kernel. Fortunately that directory does not exist either, in my case.

A safe solution would be to add the real path to our kernel at the front...

[kwankyu]

comment:15

Replying to @kwankyu:

A safe solution would be to add the real path to our kernel at the front...

Thus

export JUPYTER_DATA_DIR=???os.path.join(SAGE_VENV, "share", "jupyter")???

This is rather cumbersome though...

[dimpase]

comment:16

Perhaps we should do versioning of Jupyter kernels, so that we can specify the right one to use by name only.

[kwankyu]

comment:17

Replying to @dimpase:

Perhaps we should do versioning of Jupyter kernels, so that we can specify the right one to use by name only.

+1

[kwankyu]

Changed commit from b397afc to ecd6c78

[kwankyu]

New commits:

ecd6c78

Use versioned kernel name

[kwankyu]

Changed branch from u/mkoeppe/set_jupyter_config_dir_during_docbuild to public/33650

[kwankyu]

comment:19

Versioned kernel works well.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

5d9e862

Fix a doctest failure

[sagetrac-git]

Changed commit from ecd6c78 to 5d9e862

[mkoeppe]

comment:21

This is not getting better with versioning because it does not distinguish kernels from different installations of Sage.

[sagetrac-git]

Changed commit from 5d9e862 to 614822c

[sagetrac-git]

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

614822c

build/bin/sage-site (--docbuild): Also poison JUPYTER_DATA_DIR, JUPYTER_PATH

[mkoeppe]

comment:23

Replying to @kwankyu:

Replying to @kwankyu:

export JUPYTER_DATA_DIR=???os.path.join(SAGE_VENV, "share", "jupyter")???

I don't think it's necessary to set it to the location of our jupyter installation because that is always active.

[mkoeppe]

comment:24

Replying to @kwankyu:

This

export JUPYTER_DATA_DIR=/doesnotexist  

worked. But as I see it, this is still not a safe solution. It does prevent from using default directories, but still .sage/local/etc/jupyter shadows our kernel.

Where would that directory be coming from?

[kwankyu]

comment:25

Replying to @mkoeppe:

But as I see it, this is still not a safe solution. It does prevent from using default directories, but still .sage/local/etc/jupyter shadows our kernel.

Where would that directory be coming from?

I don't know... I don't have it anymore since I deleted the .sage directory for experiments. It didn't have kernel definitions in it, and didn't cause a problem.

[kwankyu]

comment:26

I confirm that the current branch works well. I am positive.

[kwankyu]

Reviewer: Kwankyu Lee, ...

[kwankyu]

Changed reviewer from Kwankyu Lee, ... to Kwankyu Lee

[mkoeppe]

comment:30

Thank you!

[vbraun]

Changed branch from public/33650 to 614822c

[collares]

comment:32

On NixOS we build documentation as part of the build, and JUPYTER_PATH must be set appropriately so jupyter-sphinx can find the SageMath kernel during docbuilding. Is this supported? The fact that --docbuild is handled by sage-site makes me think I am doing the wrong thing by using it outside Sage-the-distribution, but make doc-html also uses --docbuild internally. I am temporarily hacking around this by calling sage-python -m sage_docbuild directly.

[collares]

Changed commit from 614822c to none

[kiwifb]

comment:33

Replying to @collares:

On NixOS we build documentation as part of the build, and JUPYTER_PATH must be set appropriately so jupyter-sphinx can find the SageMath kernel during docbuilding. Is this supported? The fact that --docbuild is handled by sage-site makes me think I am doing the wrong thing by using it outside Sage-the-distribution, but make doc-html also uses --docbuild internally. I am temporarily hacking around this by calling sage-python -m sage_docbuild directly.

For the record in sage-on-gentoo where I split sage_setup, sage, sage_docbuild and sage-doc in 9.5 and beyond I patch the makefile for build the documentation. But it also relies on sage to be already installed and in place on the system.
https://github.com/cschwan/sage-on-gentoo/blob/master/sci-mathematics/sage-doc/files/sage-doc-9.6-makefile_and_parallel.patch

I think we are at a point where ditching ./sage --docbuild in favor of what I am currently doing in vanilla sage is not only possible but probably inevitable in the short term.

[mkoeppe]

comment:34

Replying to @collares:

The fact that --docbuild is handled by sage-site

I propose to move it back to src/bin/sage in #33795

[mkoeppe]

comment:35

Replying to @kiwifb:

For the record in sage-on-gentoo where I split sage_setup, sage, sage_docbuild and sage-doc in 9.5 and beyond I patch the makefile for build the documentation. But it also relies on sage to be already installed and in place on the system.

+1, that's the way.