Opened 20 months ago

Closed 20 months ago

Last modified 19 months ago

#31948 closed enhancement (fixed)

Reimplement parallel docbuild using make

Reported by: mkoeppe Owned by:
Priority: blocker Milestone: sage-9.4
Component: build Keywords:
Cc: dimpase Merged in:
Authors: John Palmieri, Matthias Koeppe Reviewers: Matthias Koeppe, John Palmieri, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 53f260c (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

Based on #31353 (sage --docbuild: Add options to list all documents), we can remove the issues with parallel (#31344) and serial (#31936) docbuild by restructuring the docbuild using make.

Blocker because it solves blocker issue #31936.

Change History (88)

comment:1 Changed 20 months ago by jhpalmieri

I'm not quite sure how to go about this. I tried this:

  • build/make/Makefile.in

    diff --git a/build/make/Makefile.in b/build/make/Makefile.in
    index fb3a6ed5bc..4581dc3226 100644
    a b DOC_DEPENDENCIES = sagelib sage_docbuild $(inst_sphinx) \ 
    335335       $(inst_ipykernel) $(inst_jupyter_client) $(inst_conway_polynomials) \
    336336       $(inst_tachyon) $(inst_jmol) $(inst_thebe) $(inst_ipywidgets)
    337337
    338 doc: doc-html
     338doc: doc-new
     339
     340doc-new: $(DOC_DEPENDENCIES)
     341       for doc in $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents all); do \
     342           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc html $(SAGE_DOCBUILD_OPTS)" logs/dochtml.log; \
     343       done
    339344
    340345doc-html: $(DOC_DEPENDENCIES)
    341346       $(AM_V_at)cd ../.. && sage-logger -p './sage --docbuild --no-pdf-links all html $(SAGE_DOCBUILD_OPTS)' logs/dochtml.log

and it seems to build the documentation, but it doesn't do it in parallel, I think because we've turned off parallel processing for recursive calls to make earlier in the file.

If it did work, this approach would need a little tinkering: we should build the reference manual first, then everything else.

A separate question: why do we use cd ../.. in the recipe for doc-html (and other doc targets), rather than cd $(SAGE_ROOT)?

comment:2 Changed 20 months ago by jhpalmieri

And if we really want to pull all of the parallel building out of the current system, then we should build the pieces of the reference manual separately in the Makefile. I think this is what you intended.

comment:3 Changed 20 months ago by jhpalmieri

Here is my current attempt, which doesn't quite work:

  • build/make/Makefile.in

    diff --git a/build/make/Makefile.in b/build/make/Makefile.in
    index fb3a6ed5bc..c199de440d 100644
    a b DOC_DEPENDENCIES = sagelib sage_docbuild $(inst_sphinx) \ 
    335335       $(inst_ipykernel) $(inst_jupyter_client) $(inst_conway_polynomials) \
    336336       $(inst_tachyon) $(inst_jmol) $(inst_thebe) $(inst_ipywidgets)
    337337
    338 doc: doc-html
     338doc: doc-references doc-other
     339
     340doc-references: $(DOC_DEPENDENCIES)
     341       $(eval DOCS = $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents reference))
     342       $(eval biblio = $(firstword $(DOCS)))
     343       $(eval other_docs = $(wordlist 2, 100, $(DOCS)))
     344       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $(biblio) inventory $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
     345       for doc in $(other_docs); do \
     346           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc inventory $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
     347       done
     348       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links reference inventory $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
     349       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $(biblio) html $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
     350       for doc in $(other_docs); do \
     351           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc html $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
     352       $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links reference html $(SAGE_DOCBUILD_OPTS)" logs/docs-reference-html.log; \
     353       done
     354
     355doc-other: $(DOC_DEPENDENCIES) doc-references
     356       for doc in $(wordlist 2, 100, $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents all)); do \
     357           $(AM_V_at)cd $(SAGE_ROOT) && sage-logger -p "./sage --docbuild --no-pdf-links $$doc html $(SAGE_DOCBUILD_OPTS)" logs/docs-other-html.log; \
     358       done
    339359
    340360doc-html: $(DOC_DEPENDENCIES)
    341361       $(AM_V_at)cd ../.. && sage-logger -p './sage --docbuild --no-pdf-links all html $(SAGE_DOCBUILD_OPTS)' logs/dochtml.log
  • src/sage_docbuild/__init__.py

    diff --git a/src/sage_docbuild/__init__.py b/src/sage_docbuild/__init__.py
    index 79005b903a..a3caa8a92d 100644
    a b def builder_helper(type): 
    116116            # WEBSITESPHINXOPTS is either empty or " -A hide_pdf_links=1 "
    117117            options += WEBSITESPHINXOPTS
    118118
    119         if kwds.get('use_multidoc_inventory', True):
     119        if kwds.get('use_multidoc_inventory', True) and type != 'inventory':
    120120            options += ' -D multidoc_first_pass=0'
    121121        else:
    122122            options += ' -D multidoc_first_pass=1'

Several problems:

  • lack of parallel building
  • line 348 should be replaced by something that just builds the top-level reference manual stuff, assembling the parts into one set of indices, etc. Probably the same for line 352.
  • I'm getting some warning messages that I don't know what to make of: "[schemes ] Handler <function on_build_finished at 0x33872ef70> for event 'build-finished' threw an exception", for example.

comment:4 Changed 20 months ago by mkoeppe

To get make to parallelize it, the individual documents need to become make targets. A combination of $(eval...) and a macro call can do this -- similar to https://www.gnu.org/software/make/manual/html_node/Eval-Function.html

comment:5 Changed 20 months ago by jhpalmieri

I don't see how to do anything like that unless we hardcode the list of documents: we can't run sage --docbuild ... without building some other targets, so it seems to me that we have to construct the list of documents inside a recipe. But I'm very far from a Makefile expert, so I could be missing something.

comment:6 Changed 20 months ago by jhpalmieri

Or write a standalone script to construct the document list.

comment:7 Changed 20 months ago by mkoeppe

Could you push your current version to the ticket please?

comment:8 Changed 20 months ago by jhpalmieri

Branch: u/jhpalmieri/parallel-docbuild-with-make

comment:9 Changed 20 months ago by jhpalmieri

Commit: c2c9b3c735c9b651045c471360f3a1314e883a8b

Here it is.


New commits:

c2c9b3ctrac 31948: very rough draft

comment:10 Changed 20 months ago by mkoeppe

Branch: u/jhpalmieri/parallel-docbuild-with-makeu/mkoeppe/parallel-docbuild-with-make

comment:11 Changed 20 months ago by mkoeppe

Commit: c2c9b3c735c9b651045c471360f3a1314e883a8b393ab650dd0bd38599e4716092b5217b8ecae9ed

Here's a solution using recursive make invocation


New commits:

393ab65build/make/Makefile.in (doc-reference): Invoke make recursively to build documents in parallel

comment:12 Changed 20 months ago by jhpalmieri

This doesn't work for me, unfortunately. If I do make doc-clean && make doc, I get this error, which I don't understand.

[reference] building [inventory]: targets for 1 source files that are out of date
[reference] updating environment: [new config] 1 added, 0 changed, 0 removed
[reference] The inventory files are in local/share/doc/sage/inventory/en/reference/references.
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/share/doc/sage/inventory/en/reference/references
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage
Error building the documentation.
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__main__.py", line 2, in <module>
    main()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1762, in main
    builder()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 769, in _wrapper
    self.write_auto_rest_file(module_name)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1070, in write_auto_rest_file
    with open(filename, 'w') as outfile:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds/utilities.rst'

With the following change, the documentation builds:

  • build/make/Makefile.in

    diff --git a/build/make/Makefile.in b/build/make/Makefile.in
    index 339497fd17..12b4e56b20 100644
    a b doc-references: $(DOC_DEPENDENCIES) 
    349349       $(eval DOCS = $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents reference))
    350350       $(eval biblio = $(firstword $(DOCS)))
    351351       $(eval other_docs = $(wordlist 2, 100, $(DOCS)))
    352        +$(MAKE_REC) doc-inventory-$(subst /,-,$(biblio))
    353        +$(MAKE_REC) $(foreach doc, $(other_docs), doc-inventory-$(subst /,-,$(doc)))
    354352       +$(MAKE_REC) doc-inventory-reference
    355        +$(MAKE_REC) doc-html-$(subst /,-,$(biblio))
    356        +$(MAKE_REC) $(foreach doc, $(other_docs), doc-html-$(subst /,-,$(doc)))
    357353       +$(MAKE_REC) doc-html-reference
    358354
    359355doc-other: $(DOC_DEPENDENCIES) doc-references

This is a hybrid model, using the old parallel building methods for the reference manual. The inventory build is fine, but I get some strange warnings during the html build:

[repl     ] building [html]: targets for 35 source files that are out of date
[polynomia] building [html]: targets for 62 source files that are out of date
[spkg     ] building [html]: targets for 316 source files that are out of date
[tensor_fr] building [html]: targets for 19 source files that are out of date
[algebras ] building [html]: targets for 97 source files that are out of date
[manifolds] building [html]: targets for 81 source files that are out of date
[repl     ] writing additional pages...  searchdone
[repl     ] dumping search index in English (code: en)... done
[repl     ] The HTML pages are in local/share/doc/sage/html/en/reference/repl.
[repl     ] Extension error:
[repl     ] Handler <function on_build_finished at 0x33c247af0> for event 'build-finished' threw an exception
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/share/doc/sage/html/en/reference/repl
[tensor_fr] writing additional pages...  searchdone
[tensor_fr] dumping search index in English (code: en)... done
[tensor_fr] The HTML pages are in local/share/doc/sage/html/en/reference/tensor_free_modules.
[tensor_fr] Extension error:
[tensor_fr] Handler <function on_build_finished at 0x33d1c3040> for event 'build-finished' threw an exception

I don't know what Handler <function on_build_finished at 0x33d1c3040> for event 'build-finished' threw an exception means or where it comes from.

comment:13 in reply to:  12 Changed 20 months ago by mkoeppe

Replying to jhpalmieri:

This doesn't work for me, unfortunately. If I do make doc-clean && make doc, I get this error, which I don't understand.

[reference] building [inventory]: targets for 1 source files that are out of date
[reference] updating environment: [new config] 1 added, 0 changed, 0 removed
[reference] The inventory files are in local/share/doc/sage/inventory/en/reference/references.
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/share/doc/sage/inventory/en/reference/references
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds
Deleting empty directory /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage
Error building the documentation.
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__main__.py", line 2, in <module>
    main()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1762, in main
    builder()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 769, in _wrapper
    self.write_auto_rest_file(module_name)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1070, in write_auto_rest_file
    with open(filename, 'w') as outfile:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.beta1/src/doc/en/reference/manifolds/sage/manifolds/utilities.rst'

Looks like something forgot to create the directory in which this .rst file is to be created

comment:14 Changed 20 months ago by mkoeppe

I was able to reproduce an error of this form as well.

comment:15 Changed 20 months ago by mkoeppe

If you do make doc-clean && make SAGE_DOCBUILD_OPTS="--verbose=3" doc, you can see that this is a race between creating directories and "deleting empty directories"

comment:16 Changed 20 months ago by mkoeppe

I'll add an option to turn it off

comment:17 Changed 20 months ago by git

Commit: 393ab650dd0bd38599e4716092b5217b8ecae9edd08633f65901a3accd709e496527fa8ac5bc4f2e

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

d08633fbuild/make/Makefile.in, src/sage_docbuild/__init__.py: New option --no-prune-empty-dirs, use it to remove races

comment:18 Changed 20 months ago by mkoeppe

Authors: John Palmieri, Matthias Koeppe

comment:19 Changed 20 months ago by jhpalmieri

I still see

[valuation] Extension error:
[valuation] Handler <function on_build_finished at 0x3375808b0> for event 'build-finished' threw an exception

for (I think) every piece of the reference manual. Any ideas about this?

Edit: the manual seems to build correctly, regardless.

Last edited 20 months ago by jhpalmieri (previous) (diff)

comment:20 Changed 20 months ago by mkoeppe

No idea about this error. I don't see it here. I suppose you could try to run the docbuild for a document in pdb to debug this.

comment:21 Changed 20 months ago by mkoeppe

Cc: dimpase added

comment:22 Changed 20 months ago by jhpalmieri

I haven't tried pdb, but I get it when doing make doc-clean && make — running make again at that point doesn't produce the error. I also get it on two different machines, one Big Sur, one Catalina.

One more task: the reference manual is built twice: the line

+$(MAKE_REC) SAGE_DOCBUILD_OPTS="$(SAGE_DOCBUILD_OPTS) --no-prune-empty-dirs" doc-inventory--reference

needs to be replaced with something that just builds the top-level ref manual indices, and similarly for the html build. I may be able to work on this later, but please go ahead if you want to take a stab at it.

comment:23 Changed 20 months ago by mkoeppe

If it shows up again, perhaps #31005 (Add traceback information to exceptions during docbuild) can help to diagnose it

comment:24 Changed 20 months ago by mkoeppe

OK, I see Handler <function on_build_finished at 0x3b9411310> for event 'build-finished' threw an exception now too

comment:25 Changed 20 months ago by git

Commit: d08633f65901a3accd709e496527fa8ac5bc4f2e06c6365c0dba9b4ce85feac19fb1f97cf7bea3e3

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

06c6365build/make/Makefile.in: Build doc-pdf in parallel using make

comment:26 Changed 20 months ago by mkoeppe

Here's an attempt to do the same for PDF.

comment:27 in reply to:  23 Changed 20 months ago by jhpalmieri

When I merged with #31005, I didn't see any extra information.

comment:28 Changed 20 months ago by dimpase

I tried the html build here, and the Japanese is gone. I see that Handler <function on_build_finished... messages too. Something to configure for sphinx, I gather. The default on_build_finished() needs to be replaced by something we should do (maybe just pass)

comment:29 Changed 20 months ago by dimpase

src/docs/conf.py:              'sphinx.ext.graphviz'

and in Sphinx in sphinx/ext/graphviz.py there is

def on_build_finished(app: Sphinx, exc: Exception) -> None:
    if exc is None:
        src = path.join(sphinx.package_dir, 'templates', 'graphviz', 'graphviz.css')
        dst = path.join(app.outdir, '_static')
        copy_asset(src, dst)
Last edited 20 months ago by dimpase (previous) (diff)

comment:30 Changed 20 months ago by jhpalmieri

Okay, I'm annoyed. First, we've had problems with the Sphinx graphviz extension before. Why do we even need it? As far as I can tell, it only provides Sphinx directives .. graphviz::, .. graph, and .. digraph::, and I don't see any of those in our code. Second, it's labeled an optional Sage package, so why load the extension all of the time? Third, what sort of ridiculous package is it? Its spkg-install.in script just says

#! /usr/bin/env bash
echo Error: graphviz is not installed as a system package.
echo Please install the system package recommended by ./configure
exit 1

Can we delete it?

The documentation builds without error if we remove sphinx.ext.graphviz and the related sphinx.ext.inheritance_diagram from the list of Sphinx extensions. Can we remove these extensions?

comment:31 Changed 20 months ago by mkoeppe

This type of packages is there to make system package information available -- this is what will show at the end of ./configure.

comment:32 Changed 20 months ago by mkoeppe

I'm pretty sure that this sphinx extension is not used anywhere in our documentation. It's probably a leftover from antiquity

comment:33 in reply to:  31 Changed 20 months ago by jhpalmieri

Replying to mkoeppe:

This type of packages is there to make system package information available -- this is what will show at the end of ./configure.

I think it would make more sense for graphviz to be part of _recommended rather than its own package.

comment:34 Changed 20 months ago by mkoeppe

No, that would not be an improvement. Our configure script detects the presence of system features on the granularity of packages. A catch-all "_recommended" package can't do that. See #30930.

In any case, all of this is unrelated to the errors in the present ticket.

comment:35 Changed 20 months ago by jhpalmieri

Do we want to deprecate the use of ./sage --docbuild all ..., while keeping ./sage --docbuild DOCUMENT ... for specific documents?

comment:36 Changed 20 months ago by dimpase

graphviz sphinx extension came in #7549 (in 2009)

comment:37 in reply to:  34 ; Changed 20 months ago by jhpalmieri

Replying to mkoeppe:

No, that would not be an improvement. Our configure script detects the presence of system features on the granularity of packages. A catch-all "_recommended" package can't do that. See #30930.

In any case, all of this is unrelated to the errors in the present ticket.

I agree with all of this, but I don't understand why graphviz is treated differently than ffmpeg, for example.

comment:38 in reply to:  35 ; Changed 20 months ago by mkoeppe

Replying to jhpalmieri:

Do we want to deprecate the use of ./sage --docbuild all ..., while keeping ./sage --docbuild DOCUMENT ... for specific documents?

I think that's a good idea.

comment:39 in reply to:  36 ; Changed 20 months ago by jhpalmieri

Replying to dimpase:

graphviz sphinx extension came in #7549 (in 2009)

Thank you, Dima. Let's delete those extensions.

comment:40 in reply to:  37 Changed 20 months ago by mkoeppe

Replying to jhpalmieri:

I don't understand why graphviz is treated differently than ffmpeg, for example.

ffmpeg should also be broken out of _recommended and transformed to its separate script package. When I created _recommended, I did not anticipate that users would be confused when the configure script recommends to install things that are already installed.

comment:41 Changed 20 months ago by jhpalmieri

Branch: u/mkoeppe/parallel-docbuild-with-makeu/jhpalmieri/parallel-docbuild-with-make

comment:42 Changed 20 months ago by jhpalmieri

Commit: 06c6365c0dba9b4ce85feac19fb1f97cf7bea3e3501d3fa85311bdaf07f5f77a776d6869ce789d98

I think that we could also delete the cleanup method in src/sage_docbuild/ext/inventory_builder.py: its docstring says

        Remove the '_static' directory.

        This directory is unnecessary for the inventory build, but it
        may be created by the graphviz extension. Its presence will
        break the docbuild later on, so remove it.

But let's keep it for now. It doesn't cause any harm.


New commits:

501d3fatrac 31948: no longer use sphinx.ext.graphviz or

comment:43 in reply to:  39 Changed 20 months ago by dimpase

Replying to jhpalmieri:

Replying to dimpase:

graphviz sphinx extension came in #7549 (in 2009)

Thank you, Dima. Let's delete those extensions.

Sure. Reading #7549 tells that it was meant for https://www.sphinx-doc.org/en/master/usage/extensions/inheritance.html

comment:44 in reply to:  38 ; Changed 20 months ago by jhpalmieri

Replying to mkoeppe:

Replying to jhpalmieri:

Do we want to deprecate the use of ./sage --docbuild all ..., while keeping ./sage --docbuild DOCUMENT ... for specific documents?

I think that's a good idea.

Actually, can we do this? I don't know enough about distro use: can we really recommend "make doc" and "make doc-pdf" as replacements?

comment:45 in reply to:  44 Changed 20 months ago by mkoeppe

Replying to jhpalmieri:

Replying to mkoeppe:

Replying to jhpalmieri:

Do we want to deprecate the use of ./sage --docbuild all ..., while keeping ./sage --docbuild DOCUMENT ... for specific documents?

I think that's a good idea.

Actually, can we do this? I don't know enough about distro use: can we really recommend "make doc" and "make doc-pdf" as replacements?

I'd say just adding a deprecation warning won't break anything.

Having these make recipes in our main Makefile probably shouldn't be the final form of this. In #31356, I hope that we can restructure the docbuild as script packages (sagemath_doc_html, sagemath_doc_pdf), and in the next step as pip-installable packages (#29868).

comment:46 Changed 20 months ago by git

Commit: 501d3fa85311bdaf07f5f77a776d6869ce789d984057d34ba4a3af97680d46f3b5b9186bbe22f7c5

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

6f60fd9trac 31948: deprecate "sage --docbuild all ..."
4057d34trac 31948: implement "sage --docbuild reference_top FORMAT"

comment:47 Changed 20 months ago by git

Commit: 4057d34ba4a3af97680d46f3b5b9186bbe22f7c50fee9392e1ba167b26dc8b10525f6e582cd5b310

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

0fee939trac 31948: implement "sage --docbuild reference_top FORMAT"

comment:48 Changed 20 months ago by jhpalmieri

Okay, here is a deprecation warning, plus my attempt to add a builder for just the top-level reference manual document(s). It looks like it works, but it needs more testing.

comment:49 Changed 20 months ago by jhpalmieri

(In general I feel like the docbuilding code should be torn down and rebuilt from scratch; I find it very confusing and hard to work with.)

comment:50 Changed 20 months ago by mkoeppe

Reviewers: Matthias Koeppe, ...

The HTML build seems to work well for me, even with high parallelization. PDF stops somewhere with LaTeX complaining about an undefined reference and some unicode characters, but that may be coming from some tickets that I have merged into my development branch.

I'd say let's get this in and see how this works for other people

comment:51 Changed 20 months ago by mkoeppe

[docpdf] Latexmk: Log file says output to 'combinat.pdf'
[docpdf] Latexmk: List of undefined refs and citations:
[docpdf]   Reference `index:module-sage.combinat' on page 3639 undefined on input line 348243
[docpdf] Latexmk: Summary of warnings from last run of *latex:
[docpdf]   Latex failed to resolve 1 reference(s)
[docpdf]   =====Latex reported missing or unavailable character(s).
[docpdf] =====See log file for details.

comment:52 Changed 20 months ago by git

Commit: 0fee9392e1ba167b26dc8b10525f6e582cd5b3101c412844148e4429918bc4035cb8c755a270ecc1

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

1c41284trac 31948: implement "sage --docbuild reference_top FORMAT"

comment:53 Changed 20 months ago by git

Commit: 1c412844148e4429918bc4035cb8c755a270ecc1b17b67463aaa3c0fa4d1e8d54711aeccabda3327

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

b17b674trac 31948: implement "sage --docbuild reference_top FORMAT"

comment:54 Changed 20 months ago by jhpalmieri

I'm struggling to get this to work. The current branch works for me when building the html documentation with make doc, but ./sage --docbuild all html fails with

Error building the documentation.
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.9/3.9.5/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/palmieri/Desktop/Sage/git/sage/local/lib/python3.9/site-packages/sage_docbuild/__main__.py", line 2, in <module>
    main()
  File "/Users/palmieri/Desktop/Sage/git/sage/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 1811, in main
    builder()
  File "/Users/palmieri/Desktop/Sage/git/sage/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 345, in _wrapper
    getattr(get_builder(document), 'inventory')(*args, **kwds)
  File "/Users/palmieri/Desktop/Sage/git/sage/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 589, in _wrapper
    self._build_top_level(format, *args, **kwds)
  File "/Users/palmieri/Desktop/Sage/git/sage/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 571, in _build_top_level
    getattr(ReferenceTopBuilder(), format)(*args, **kwds)
  File "/Users/palmieri/Desktop/Sage/git/sage/local/lib/python3.9/site-packages/sage_docbuild/__init__.py", line 627, in __init__
    DocBuilder.__init__(self, *args, **kwds)
TypeError: __init__() missing 1 required positional argument: 'name'

If I add an argument, for example "reference", to the call

        DocBuilder.__init__(self, *args, **kwds)

in ReferenceTopBuilder.__init__, then make doc fails.

comment:55 Changed 20 months ago by git

Commit: b17b67463aaa3c0fa4d1e8d54711aeccabda332706dfb9097e82dd6d1c95a0f7412f53967d21c217

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

06dfb90trac 31948: fix "help_documents"

comment:56 Changed 20 months ago by git

Commit: 06dfb9097e82dd6d1c95a0f7412f53967d21c2178930551946875bc8eefd855c2b8cb88828f93a24

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

8930551trac 31948: repair "sage --docbuild all html"

comment:57 Changed 20 months ago by jhpalmieri

I think that fixes my problem. I've seen the combinat.pdf failure once, but I can't reproduce it. I'll keep trying.

comment:58 Changed 20 months ago by dimpase

on Lunix I am seeing

[dochtml] [combinat ] updating environment: [new config] 382 added, 0 changed, 0 removed
[dochtml] [manifolds] building [inventory]: targets for 81 source files that are out of date
[dochtml] [manifolds] updating environment: [new config] 81 added, 0 changed, 0 removed
[dochtml] Error, reached the pre-set memory limit
[dochtml] (change it with the -o command line option)
[dochtml] Error, was not in any namespace

not sure what limit is involved here and why.

comment:59 Changed 20 months ago by dimpase

otherwise, all works on Linux, both html and pdf builds.

comment:60 in reply to:  58 Changed 20 months ago by jhpalmieri

Replying to dimpase:

on Lunix I am seeing

[dochtml] [combinat ] updating environment: [new config] 382 added, 0 changed, 0 removed
[dochtml] [manifolds] building [inventory]: targets for 81 source files that are out of date
[dochtml] [manifolds] updating environment: [new config] 81 added, 0 changed, 0 removed
[dochtml] Error, reached the pre-set memory limit
[dochtml] (change it with the -o command line option)
[dochtml] Error, was not in any namespace

not sure what limit is involved here and why.

Is that error new? Is it repeatable?

comment:61 Changed 20 months ago by dimpase

it might be due to the new GAP 4.11.1 I have in my branch (this message comes from GAP, that's certain). Let's ignore it now. I'll try to reproduce it on another system.

comment:62 Changed 20 months ago by jhpalmieri

This also works for me with the Sphinx upgrade at #31696.

comment:63 Changed 20 months ago by dimpase

I wonder if it is easy to get rid of build_many(), and replace it by a make-based parallelisation.

comment:64 in reply to:  63 ; Changed 20 months ago by jhpalmieri

Replying to dimpase:

I wonder if it is easy to get rid of build_many(), and replace it by a make-based parallelisation.

Probably, but on another ticket. Should we start using src/doc/Makefile for more than just cleaning? As I said earlier, I am unclear on the distro situation: I don't know what packages are going to be available in general, and in particular in this case: if we have the documentation source files, can we be guaranteed to have sage_docbuild?

In any case, this may all fit somehow into #31356 and related tickets.

comment:65 in reply to:  64 ; Changed 20 months ago by mkoeppe

Replying to jhpalmieri:

Replying to dimpase:

I wonder if it is easy to get rid of build_many(), and replace it by a make-based parallelisation.

Probably, but on another ticket.

I agree.

Should we start using src/doc/Makefile for more than just cleaning?

I think that's a good idea. The things that we added in build/make/Makefile.in could just be moved there.

comment:66 in reply to:  65 ; Changed 20 months ago by jhpalmieri

Replying to mkoeppe:

Replying to jhpalmieri:

Should we start using src/doc/Makefile for more than just cleaning?

I think that's a good idea. The things that we added in build/make/Makefile.in could just be moved there.

I'm experimenting with this and it seems to be working. A few questions about our Makefiles:

  • Where is SAGE_ROOT defined? It appears to work if I just use it in src/doc/Makefile, but I don't know why.
  • Do I need to use +$(MAKE_REC) or can I just use $(MAKE)? The latter seems to work.
  • Do I need to define AM_V_at in this Makefile, or does it inherit the definition from build/make/Makefile?

comment:67 in reply to:  66 Changed 20 months ago by mkoeppe

Replying to jhpalmieri:

Replying to mkoeppe:

Replying to jhpalmieri:

Should we start using src/doc/Makefile for more than just cleaning?

I think that's a good idea. The things that we added in build/make/Makefile.in could just be moved there.

I'm experimenting with this and it seems to be working. A few questions about our Makefiles:

  • Where is SAGE_ROOT defined? It appears to work if I just use it in src/doc/Makefile, but I don't know why.

It comes from the environment and is set in build/make/install.

  • Do I need to use +$(MAKE_REC) or can I just use $(MAKE)? The latter seems to work.

$(MAKE) is fine here - https://www.gnu.org/software/make/manual/make.html#MAKE-Variable

  • Do I need to define AM_V_at in this Makefile, or does it inherit the definition from build/make/Makefile?

You'd need to copy this definition from Makefile.in; it's not exported

comment:68 Changed 20 months ago by git

Commit: 8930551946875bc8eefd855c2b8cb88828f93a24522b4508e2686a118e4c5dfb0d0bd09f89b4aa00

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

522b450trac 31948: use src/doc/Makefile for docbuilding commands

comment:69 Changed 20 months ago by jhpalmieri

That's helpful, thank you. Here's an attempt. I'm not sure how to handle SAGE_ROOT in src/doc/Makefile: if it's not set, exit with an error? That's my current approach.

The new branch keeps the logging in build/make/Makefile: if someone wants to use src/doc/Makefile to build the documentation, maybe they don't want it logged. This seems consistent with how we do logging in general. It does mean that make doc-pdf logs all docbuilding, including inventory building, in docpdf.log, whereas it used to put the inventory logs into dochtml.log.

comment:70 Changed 20 months ago by git

Commit: 522b4508e2686a118e4c5dfb0d0bd09f89b4aa003d90c59461f5d1c656e6b972809c89468c92a334

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

3d90c59trac 31948: use src/doc/Makefile for docbuilding commands

comment:71 Changed 20 months ago by git

Commit: 3d90c59461f5d1c656e6b972809c89468c92a3342c3a9ae60a5f101f958ca711a413c2d27a61aa17

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

2c3a9aetrac 31948: move some "$(AM_V_at)"s around

comment:72 Changed 20 months ago by git

Commit: 2c3a9ae60a5f101f958ca711a413c2d27a61aa1754e68e068e64d7a8650c474cf5b24af91c0baad5

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

54e68e0trac 31948: move some "$(AM_V_at)"s around

comment:73 Changed 20 months ago by jhpalmieri

Okay, I think I'm done tinkering now.

comment:74 Changed 20 months ago by mkoeppe

Status: newneeds_review

comment:75 Changed 20 months ago by mkoeppe

Looks good and seems to work well. Positive review from my side

comment:76 Changed 20 months ago by jhpalmieri

Dima, any comments? I'm happy with Matthias' contributions.

comment:77 Changed 20 months ago by jhpalmieri

Reviewers: Matthias Koeppe, ...Matthias Koeppe, John Palmieri, ...

comment:78 Changed 20 months ago by git

Commit: 54e68e068e64d7a8650c474cf5b24af91c0baad553f260c1b355fc623cf8feed858614a87133028d

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

53f260ctrac 31948: fix doctests

comment:79 Changed 20 months ago by dimpase

Reviewers: Matthias Koeppe, John Palmieri, ...Matthias Koeppe, John Palmieri, Dima Pasechnik
Status: needs_reviewpositive_review

looks good, thanks!

comment:80 Changed 20 months ago by jhpalmieri

Great, thank you!

comment:81 Changed 20 months ago by mkoeppe

Description: modified (diff)
Priority: majorblocker

comment:82 Changed 20 months ago by vbraun

Branch: u/jhpalmieri/parallel-docbuild-with-make53f260c1b355fc623cf8feed858614a87133028d
Resolution: fixed
Status: positive_reviewclosed

comment:83 Changed 20 months ago by klee

Commit: 53f260c1b355fc623cf8feed858614a87133028d

Previously sage --docbuild all pdf built a main website listing all documentations with icons linking to pdf files. Now it seems that make doc-pdf does not build such an website. Is there a way to build the website?

comment:84 Changed 20 months ago by mkoeppe

I've opened #32043 for this

comment:85 Changed 19 months ago by dimpase

I am seeing a race condition, sphinx may race FileExistsError, cf. e.g.

[dochtml] [reference] preparing documents... skipping loading of indexes... done
[dochtml] [reference] Merging js index files...
[dochtml] [reference]     algebras: 4424 js index entries
[dochtml] [reference]     arithgroup: 1204 js index entries
...

[dochtml] [reference]     topology: 1937 js index entries
[dochtml] [reference]     valuations: 973 js index entries
[dochtml] [reference] ... done (64387 js index entries)
[dochtml] [reference] copying static files... failed
[dochtml] [reference] WARNING: cannot copy static file
FileExistsError(17, 'File exists')
[dochtml] [reference] dumping search index in English (code: en)... done
[dochtml] [reference] The HTML pages are in
local/share/doc/sage/html/en/reference.
[dochtml] Error building the documentation.

make -j1 allows the docs to build.

See e.g. https://github.com/sagemath/sagetrac-mirror/runs/3104375540

It's of course not totally surprising that several processes may try to, say, create the same directory. Perhaps the relevant part of the builder should be launched with this specific error being ignored?

comment:86 Changed 19 months ago by jhpalmieri

It's strange because that doesn't look like part of the docbuild process that's done in parallel with anything else: it looks like the main reference page, built after the other parts of the reference manual. How reliably can you reproduce this?

I think this part of the process (copying static files) is controlled by Sphinx, not by Sage or its modifications to Sphinx, for what that's worth.

comment:87 in reply to:  85 ; Changed 19 months ago by mkoeppe

Replying to dimpase:

I am seeing a race condition, sphinx may race FileExistsError

new ticket please?

comment:88 in reply to:  87 Changed 19 months ago by klee

Replying to mkoeppe:

Replying to dimpase:

I am seeing a race condition, sphinx may race FileExistsError

new ticket please?

This is now #32262

Note: See TracTickets for help on using tickets.