Fix broken symlink to documentation in the Sage jupyter kernelspec

[mkoeppe]

Follow-up from #30299 (Minimal fix for broken jupyter notebook):

Sage creates a broken symlink

local/share/jupyter/kernels/sagemath/doc -> /doesnotexist/html/en

which blocks jupyter kernelspec install into a system Jupyter using the instructions from #30476:

  File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/shutil.py", line 368, in copytree
    raise Error(errors)
shutil.Error: [('/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/local/share/jupyter/kernels/sagemath/doc', '/usr/local/share/jupyter/kernels/sagemath/doc', "[Errno 2] No such file or directory: '/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/local/share/jupyter/kernels/sagemath/doc'")]

Reports:

• ​Ask Sage question 54581: 9.2 download for Ubuntu 20.04 documentation
• ​Sage-Windows issue 52 -- with workaround
• ​Stack Overflow question 65161395

[mkoeppe]

New commits:

​42eaffa

build/pkgs/sagelib/spkg-install: Fix installation of doc symlink in sage kernel spec

[jhpalmieri]

It is not completely clear to me why we "poison" all of these directory names. The comment "# We also poison all directories below SAGE_LOCAL" seems like an aside, as if this may not be necessary. Any comments?

Also, does this affect bdist production?

[mkoeppe]

We poison them so that it becomes harder to reintroduce bad code that breaks the separation of the sage distribution from the sage library. I introduced this in #21480.

[mkoeppe]

Replying to jhpalmieri:

Also, does this affect bdist production?

I don't know if the bdist does anything special about these symlinks. If it does not, then it is broken currently and will be fixed by this ticket as well.

[embray]

I think this is a bad-enough problem that we should release a 9.2.1 patch release, since this broke using help in the Jupyter kernel for users. Not a good look.

[embray]

Is this really the right approach? It also poisons SAGE_SHARE which is used in sage.env to set SAGE_DOC. I think if we really want this environment variable "poisoning" to be used to catch problems like this, there should be something in sage_setup which imports sage.env and makes sure none of the variables start with /doesnotexist what I wrote before doesn't make sense. But maybe it should somehow actually crash when using any of those variables during setup.py.

[embray]

I see now that it's ok since when running this script in the sage shell SAGE_DOC is already set explicitly, so it doesn't need to be derived from SAGE_SHARE. But I think this is still a bit of a mess. Either the variables in sage.env should work and have sane values when running setup.py, or not. And simply setting them to bogus values does not guarantee that they are not being used somewhere during build/installation. Since SAGE_SRC and SAGE_LOCAL are also used in a number of places, what exactly is the goal here?

[embray]

Instead of "poisoning" these variables, why don't we unset them instead, and then let sage.env derive the correct defaults it would use when installing sagelib independently of the sage distribution?

The one exception would be SAGE_ROOT, but I think instead of setting it to a bogus value it, and another other paths derived from it, should be set to None. Then we'll most likely get a crash if any of those variables are used during build/installation.

[mkoeppe]

Replying to embray:

Instead of "poisoning" these variables, why don't we unset them instead, and then let sage.env derive the correct defaults it would use when installing sagelib independently of the sage distribution?

The point of the poisoning is that defaults are NOT used.

[mkoeppe]

Replying to embray:

I see now that it's ok since when running this script in the sage shell SAGE_DOC is already set explicitly, so it doesn't need to be derived from SAGE_SHARE. But I think this is still a bit of a mess.

That's right. The core of the mess is that the build system (setup.py + sage_setup) (still) uses sage.env at all and therefore has access to various variables (and their defaulting behavior) including those that are intended for runtime use only. This is what the poisoning addresses.

[mkoeppe]

In any case, this ticket here is a simple fix. The real action is happening in Meta-ticket #30306.

[dimpase]

lgtm

[mkoeppe]

Thanks.