Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#19963 closed enhancement (fixed)

Build documentation in $SAGE_SHARE/doc/sage

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.1
Component: build Keywords:
Cc: hivert, mmezzarobba Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 96685ab (Commits) Commit:
Dependencies: #19127 Stopgaps:

Description


Change History (29)

comment:1 Changed 5 years ago by jdemeyer

  • Summary changed from Build documentation in $SAGE_LOCAL/share/doc/sage to Build documentation in $SAGE_SHARE/doc/sage

comment:2 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/build_documentation_in__sage_local_share_doc_sage

comment:3 Changed 5 years ago by jdemeyer

  • Commit set to 2c56897ca77ed33909a6c08738d8a363975b750f
  • Status changed from new to needs_review

New commits:

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

comment:4 Changed 5 years ago by git

  • Commit changed from 2c56897ca77ed33909a6c08738d8a363975b750f to dfe2c063dfc51ae8a05022fb5c4d09cb545a8354

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

dfe2c06Minor fixes to top-level build system

comment:5 Changed 5 years ago by git

  • Commit changed from dfe2c063dfc51ae8a05022fb5c4d09cb545a8354 to cb2b63f7e13f010b529411c9645a9b6ef337f713

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

cb2b63fRemove old src/doc/ouput and .pyc files

comment:6 follow-up: Changed 5 years ago by mmezzarobba

  • Status changed from needs_review to needs_work

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.

comment:7 follow-up: Changed 5 years ago by kcrisman

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?

comment:8 in reply to: ↑ 7 Changed 5 years ago by jdemeyer

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.

comment:9 Changed 5 years ago by git

  • Commit changed from cb2b63f7e13f010b529411c9645a9b6ef337f713 to de10a5ed704bac161fa3f43d381cd3aac0776817

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

comment:10 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:11 in reply to: ↑ 6 Changed 5 years ago by jdemeyer

Replying to mmezzarobba:

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

Fixed in https://github.com/sagemath/sagenb/pull/363

comment:12 Changed 5 years ago by fbissey

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.

Last edited 5 years ago by fbissey (previous) (diff)

comment:13 Changed 5 years ago by jdemeyer

Replying to fbissey:

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.

comment:14 Changed 5 years ago by fbissey

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.

comment:15 Changed 5 years ago by fbissey

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?

comment:16 follow-up: Changed 5 years ago by 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.

comment:17 in reply to: ↑ 16 ; follow-ups: Changed 5 years ago by fbissey

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.

comment:18 Changed 5 years ago by fbissey

Sorry John, I thought that was from Jeroen.

comment:19 in reply to: ↑ 17 Changed 5 years ago by jdemeyer

Replying to fbissey:

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.

comment:20 in reply to: ↑ 17 ; follow-up: Changed 5 years ago by jdemeyer

Replying to fbissey:

You started to import cythonized stuff (cachefunc for starters)

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

comment:21 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:22 in reply to: ↑ 20 Changed 5 years ago by fbissey

Replying to jdemeyer:

Replying to fbissey:

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.

comment:23 Changed 5 years ago by git

  • Commit changed from de10a5ed704bac161fa3f43d381cd3aac0776817 to 96685abe84f6c5b0f899ac1190e608d65e69981d

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

96685abFix a few more paths

comment:24 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:25 Changed 5 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

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.

comment:26 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/build_documentation_in__sage_local_share_doc_sage to 96685abe84f6c5b0f899ac1190e608d65e69981d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:27 follow-up: Changed 5 years ago by jhpalmieri

  • Commit 96685abe84f6c5b0f899ac1190e608d65e69981d deleted

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?

comment:28 in reply to: ↑ 27 Changed 5 years ago by fbissey

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.

comment:29 Changed 5 years ago by jdemeyer

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

    if os.path.exists(old_doc):
        from shutil import rmtree
        rmtree(old_doc)
Note: See TracTickets for help on using tickets.