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

Build documentation in $SAGE_SHARE/doc/sage #19963

Closed
jdemeyer opened this issue Jan 26, 2016 · 32 comments
Closed

Build documentation in $SAGE_SHARE/doc/sage #19963

jdemeyer opened this issue Jan 26, 2016 · 32 comments

Comments

@jdemeyer
Copy link

Depends on #19127

CC: @hivert @mezzarobba

Component: build

Author: Jeroen Demeyer

Branch: 96685ab

Reviewer: François Bissey

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

@jdemeyer jdemeyer added this to the sage-7.1 milestone Jan 26, 2016
@jdemeyer jdemeyer changed the title Build documentation in $SAGE_LOCAL/share/doc/sage Build documentation in $SAGE_SHARE/doc/sage Jan 26, 2016
@jdemeyer
Copy link
Author

@jdemeyer
Copy link
Author

Commit: 2c56897

@jdemeyer
Copy link
Author

New commits:

7c98158Docbuilder cleanup
537d9d2Minor fixes
1cebbc9Fix citation dir with custom SAGE_DOC_OUTPUT
2c56897Build documentation in $SAGE_SHARE/doc/sage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2016

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

dfe2c06Minor fixes to top-level build system

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2016

Changed commit from 2c56897 to dfe2c06

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2016

Changed commit from dfe2c06 to cb2b63f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 28, 2016

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

cb2b63fRemove old src/doc/ouput and .pyc files

@mezzarobba
Copy link
Member

comment:6

This will break the “Help” links in the (classic) notebook, won't it? More generally, though I entirely agree with the goal of this ticket, I'm not sure people like the change. Wouldn't it be possible to keep a compatibility symlink or something? Also, several places in the documentation (starting with README.txt) explicitly refer the reader to src/doc/output.

@kcrisman
Copy link
Member

kcrisman commented Feb 4, 2016

comment:7

Yeah, I'm not sure exactly what the necessity of this is. At the very least it would be something where "deprecation" would be useful?

@jdemeyer
Copy link
Author

jdemeyer commented Feb 4, 2016

comment:8

Replying to @kcrisman:

I'm not sure exactly what the necessity of this is.

Everything which is compiled/built/installed in the Sage distribution ends up in $SAGE_LOCAL... except the documentation. So it's logical to put the built documentation in $SAGE_LOCAL too.

At the very least it would be something where "deprecation" would be useful?

Well, you cannot really deprecate filesystem paths. But a symbolic link can be done indeed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 4, 2016

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

a601a8cAdd symbolic link SAGE_DOC/output -> SAGE_DOC_OUTPUT
de10a5eUse new documentation paths in documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 4, 2016

Changed commit from cb2b63f to de10a5e

@jdemeyer
Copy link
Author

jdemeyer commented Feb 6, 2016

comment:11

Replying to @mezzarobba:

This will break the “Help” links in the (classic) notebook, won't it?

Fixed in sagemath/sagenb#363

@kiwifb
Copy link
Member

kiwifb commented Feb 8, 2016

comment:12

I am unfortunately taking this train after it left the station. I guess the code re-organisation had to happen in #19127 and all. But what a choice of variable names. Seriously SAGE_DOC_OUTPUT? Yes I can see where it came from. But seriously I expect my doc to be found under SAGE_DOC not SAGE_DOC_OUTPUT, that's the natural name to use. The other one could have been SAGE_DOC_SRC, and if it isn't used after install I am not sure I would bother putting it in env.py.

@jdemeyer
Copy link
Author

jdemeyer commented Feb 8, 2016

comment:13

Replying to @kiwifb:

I am unfortunately taking this train after it left the station. I guess the code re-organisation had to happen in #19127 and all. But what a choice of variable names. Seriously SAGE_DOC_OUTPUT? Yes I can see where it came from. But seriously I expect my doc to be found under SAGE_DOC not SAGE_DOC_OUTPUT, that's the natural name to use. The other one could have been SAGE_DOC_SRC, and if it isn't used after install I am not sure I would bother putting it in env.py.

I will change the variable names on this ticket if somebody commits to reviewing this ticket then.

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

comment:14

I am going through the changes in #19127 first. You made it incredibly difficult to build the documentation before having installed sage at all. Unless I was lucky before (possible but unlikely considering the amount of stuff that can go wrong) that's a regression as far as I am concerned. But it is too late for this in that ticket or this ticket.

The code here looks ok to me, it is mostly house keeping.

The html and the pdf documentations are moved to a new home and only the rst file are left behind. The only mention left of SAGE_DOC in the current form is in sage/misc/sagedoc.py, possibly too hard for this ticket but if it is only for building at least some parts of sagedoc.py could/should be moved to sage_setup/docbuild.
There is also some mention of SAGE_DOC and SAGE_DOC/output in sage/misc/latex_macros.py. Only in documentation and comment blocks. At least two of these should be replaced by SAGE_DOC_OUTPUT, the first instance is to quote a path in sage's source tree. Ultimately this should be shipped too in my opinion but at this stage is the only instance of SAGE_DOC that is not superfluous once that sage is installed - outside of sage_setup/docbuild.

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

comment:15

In sage_setup/docbuild/__init__.py we currently have at line 101 and after

        if 'output/html' in output_dir:
            logger.warning("Build finished. The built documents can be found in %s",
                           output_dir)
        elif 'output/pdf' not in output_dir:
            logger.info("Build finished. The built documents can be found in %s",
                           output_dir)
        else:
            logger.info("LaTeX file written to %s; now making PDF.",
                           output_dir)

Will we ever see a message "Build finished. The built documents can be found in..." again?

@jhpalmieri
Copy link
Member

comment:16

You made it incredibly difficult to build the documentation before having installed sage at all.

When was this ever possible? Certainly Sphinx needs to be installed, and docbuilding also reads parts (or all?) of the Sage library.

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

comment:17

Replying to @jhpalmieri:

You made it incredibly difficult to build the documentation before having installed sage at all.

When was this ever possible? Certainly Sphinx needs to be installed, and docbuilding also reads parts (or all?) of the Sage library.

Of course sphinx was installed. I meant the sage library itself. And yes it was possible, may still be possible with a bit of care. What happens technically is that the docbuilding itself only needs to go through sage's sources, and if you don't call anything from cythonized modules during the building process itself, you don't need anything but the sources. You started to import cythonized stuff (cachefunc for starters) and things just broke. It look like you can do stuff from build/lib but attention to details is required.

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

comment:18

Sorry John, I thought that was from Jeroen.

@jdemeyer
Copy link
Author

jdemeyer commented Feb 9, 2016

comment:19

Replying to @kiwifb:

And yes it was possible

Doesn't autodoc import the modules that it builds the documentation of, to determine __doc__? If so, there is no way to build the Sage documentation without building Sage first.

@jdemeyer
Copy link
Author

jdemeyer commented Feb 9, 2016

comment:20

Replying to @kiwifb:

You started to import cythonized stuff (cachefunc for starters)

No, the Sage docbuilder has always used cachefunc, that is nothing new.

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

comment:22

Replying to @jdemeyer:

Replying to @kiwifb:

You started to import cythonized stuff (cachefunc for starters)

No, the Sage docbuilder has always used cachefunc, that is nothing new.

Let's not get into this any more and finish this ticket. I may be looking for something stupid.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 9, 2016

Changed commit from de10a5e to 96685ab

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 9, 2016

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

96685abFix a few more paths

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

Reviewer: François Bissey

@kiwifb
Copy link
Member

kiwifb commented Feb 9, 2016

comment:25

I am happy with that. Anything further will be for a follow up ticket. And once the PR to sagenb is included in a release we should remove the creation of links to the old location. sagenb is really the only thing that currently depends on it.

@vbraun
Copy link
Member

vbraun commented Feb 10, 2016

@jhpalmieri
Copy link
Member

comment:27

For a followup ticket: should we reinstate the line rm -rf output in src/doc/Makefile? Otherwise the old output will remain and possibly confuse people. Or should the old output be removed somewhere else?

@jhpalmieri
Copy link
Member

Changed commit from 96685ab to none

@kiwifb
Copy link
Member

kiwifb commented Feb 12, 2016

comment:28

Replying to @jhpalmieri:

For a followup ticket: should we reinstate the line rm -rf output in src/doc/Makefile? Otherwise the old output will remain and possibly confuse people. Or should the old output be removed somewhere else?

That would be a good idea I would say. Not quite on the top of my laundry list. But if we want to discuss those matters, it shouldn't be on this ticket.

@jdemeyer
Copy link
Author

comment:29

Old docs are deleted, see src/sage_setup/docbuild/__main__.py:

    if os.path.exists(old_doc):
        from shutil import rmtree
        rmtree(old_doc)

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

6 participants