Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move handling of "sage --docbuild" back to src/bin/sage #33795

Closed
mkoeppe opened this issue May 4, 2022 · 20 comments
Closed

Move handling of "sage --docbuild" back to src/bin/sage #33795

mkoeppe opened this issue May 4, 2022 · 20 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented May 4, 2022

(from #33650 comment:32)

It was moved in #29111 to build/bin/sage-site, but it does have some (limited - #30475) functionality for users.

sage-docbuild would be declared as an install-requires of sagemath-standard. #29941 also declares it as an extras-require of sagemath-environment.

CC: @jhpalmieri @collares @kiwifb @kwankyu

Component: scripts

Author: Kwankyu Lee

Branch/Commit: fc9b96f

Reviewer: Matthias Koeppe

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

@mkoeppe mkoeppe added this to the sage-9.7 milestone May 4, 2022
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented May 4, 2022

comment:4

I personally think --docbuild should disappear and be replaced by direct call to python -m sage_docbuild (or possibly sage-python if it still makes sense). The current set up make things overly complex in src/sage/doc/Makefile and I am not sure putting things back into the main sage script makes it any better.

If we have any downstream using the command, I suggest we issue a deprecation message.

@jhpalmieri
Copy link
Member

comment:5

I'm a little concerned about things breaking if we just use python -m sage_docbuild, based on this snippet from sage-site:

if [ "$1" = "-docbuild" -o "$1" = "--docbuild" ]; then
    # Redirect stdin from /dev/null. This helps with running TeX which
    # tends to ask interactive questions if something goes wrong. These
    # cause the build to hang. If stdin is /dev/null, TeX just aborts.
    shift

    # Trac #30002: ensure an English locale so that it is possible to
    # scrape out warnings by pattern matching.
    export LANG=C
    export LANGUAGE=C

    # See #30351: bugs in macOS implementations of openblas/libgopm can cause
    # docbuild to hang if multiple OpenMP threads are allowed.
    if [ `uname` = 'Darwin' ]; then
        export OMP_NUM_THREADS=1
    fi

    # Trac #33650: Make sure that user configuration of Jupyter does not
    # shadow our sagemath kernel when jupyter-sphinx is invoked
    export JUPYTER_CONFIG_DIR=/doesnotexist
    export JUPYTER_CONFIG_PATH=/doesnotexist
    export JUPYTER_DATA_DIR=/doesnotexist
    export JUPYTER_PATH=/doesnotexist

    exec sage-python -m sage_docbuild "$@" </dev/null
fi

@kiwifb
Copy link
Member

kiwifb commented May 4, 2022

comment:6

I'll admit that I don't have some of that baggage in sage-on-gentoo. If I was needing any of that stuff to build the doc, I would set it up before running the make invocation.

The LANG/LANGUAGE is something I expect from my environment for sanity (possibly a mistake :) ). The darwin thing could be handled if I go back to supporting gentoo-prefix on Mac. I build in a sandbox where user settings for jupyter will not affect me (gentoo-prefix may need checking for that scenario).

The /dev/null thing is probably something that need to be added to what I have done so far.

I am mostly seeing sage --docbuild as a relic from the past, but yes that cruft needs attention.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 5, 2022

comment:7

Replying to @kiwifb:

I am mostly seeing sage --docbuild as a relic from the past

In #29941, I am making src/bin/sage part of a small basic distribution sagemath-environment, so that users can invoke, for example, sage --sh even when nothing else of Sage is installed. Options such as sage --docbuild, of course, will not work when nothing else is installed -- so the script could inform users what to install in this case.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 7, 2022

Branch: public/33795

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 7, 2022

Author: Kwankyu Lee

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 7, 2022

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

e266569Move -docbuild from sage-site to sage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 7, 2022

Commit: e266569

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 7, 2022

comment:13

Is this what to be done here?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 7, 2022

comment:15

LGTM, but this comment:

+    # Redirect stdin from /dev/null. This helps with running TeX which
+    # tends to ask interactive questions if something goes wrong. These
+    # cause the build to hang. If stdin is /dev/null, TeX just aborts.

is in a strange place (already in the original).
It would be better placed just before

+    exec sage-python -m sage_docbuild "$@" </dev/null

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2022

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

fc9b96fMove a comment to the right place

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 8, 2022

Changed commit from e266569 to fc9b96f

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 8, 2022

comment:17

Replying to @mkoeppe:

It would be better placed just before

+    exec sage-python -m sage_docbuild "$@" </dev/null

Right. Done.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 8, 2022

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Jul 9, 2022

Changed branch from public/33795 to fc9b96f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants