sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.45k stars 2.1k forks source link

LaTeX builder fails with Unknown node: substitution_definition #8936

Closed kovidgoyal closed 3 years ago

kovidgoyal commented 3 years ago

Describe the bug Exception occurred:

  File "/usr/lib/python3.9/site-packages/sphinx/writers/latex.py", line 2072, in unknown_visit
    raise NotImplementedError('Unknown node: ' + node.__class__.__name__)
NotImplementedError: Unknown node: substitution_definition
sphinx-build --version                                                               
sphinx-build 3.5.1

To Reproduce Sadly I dont have a simple reproducer as this occurred while building the PDF documentation for calibre. However, it is almost certainly caused by #7953

tk0miya commented 3 years ago

@brechtm Could you check this if you have time? I'll also take a look later.

tk0miya commented 3 years ago

I can't reproduce the error with the following document with v3.5.1:

blah
----

.. |foo| replace:: FOO

|foo| is |foo|

@kovidgoyal Can you make a reproducible example?

kovidgoyal commented 3 years ago

Sadly, no, it happens in a very large and complex project. However, I was able to "fix" it by simply adding the following to latex.py

    def visit_substitution_definition(self, node):
        pass

    def depart_substitution_definition(self, node):
        pass

Since substitution definition shouldnt be producing any output, this should be safe to do, I think.

kovidgoyal commented 3 years ago

And this is the full log:

# Sphinx version: 3.5.1
# Python version: 3.9.2 (CPython)
# Docutils version: 0.16 release
# Jinja2 version: 2.11.3
# Last messages:
#   develop
#   db_api
#   polish
#   drm
#   glossary
#   
#   resolving references...
#   done
#   writing...
#   failed
# Loaded extensions:
#   sphinx.ext.mathjax (3.5.1) from /usr/lib/python3.9/site-packages/sphinx/ext/mathjax.py
#   sphinxcontrib.applehelp (1.0.2) from /usr/lib/python3.9/site-packages/sphinxcontrib/applehelp/__init__.py
#   sphinxcontrib.devhelp (1.0.2) from /usr/lib/python3.9/site-packages/sphinxcontrib/devhelp/__init__.py
#   sphinxcontrib.htmlhelp (1.0.3) from /usr/lib/python3.9/site-packages/sphinxcontrib/htmlhelp/__init__.py
#   sphinxcontrib.serializinghtml (1.1.4) from /usr/lib/python3.9/site-packages/sphinxcontrib/serializinghtml/__init__.py
#   sphinxcontrib.qthelp (1.0.3) from /usr/lib/python3.9/site-packages/sphinxcontrib/qthelp/__init__.py
#   alabaster (0.7.12) from /usr/lib/python3.9/site-packages/alabaster/__init__.py
#   sphinx.ext.autodoc.type_comment (3.5.1) from /usr/lib/python3.9/site-packages/sphinx/ext/autodoc/type_comment.py
#   sphinx.ext.autodoc (3.5.1) from /usr/lib/python3.9/site-packages/sphinx/ext/autodoc/__init__.py
#   custom (unknown version) from /home/kovid/work/calibre/manual/custom.py
#   sidebar_toc (unknown version) from /home/kovid/work/calibre/manual/sidebar_toc.py
#   sphinx.ext.viewcode (3.5.1) from /usr/lib/python3.9/site-packages/sphinx/ext/viewcode.py
Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app.build(args.force_all, filenames)
  File "/usr/lib/python3.9/site-packages/sphinx/application.py", line 352, in build
    self.builder.build_update()
  File "/usr/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 293, in build_update
    self.build(['__all__'], to_build)
  File "/usr/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 360, in build
    self.write(docnames, list(updated_docnames), method)
  File "/usr/lib/python3.9/site-packages/sphinx/builders/latex/__init__.py", line 304, in write
    docwriter.write(doctree, destination)
  File "/usr/lib/python3.9/site-packages/docutils/writers/__init__.py", line 78, in write
    self.translate()
  File "/usr/lib/python3.9/site-packages/sphinx/writers/latex.py", line 101, in translate
    self.document.walkabout(visitor)
  File "/usr/lib/python3.9/site-packages/docutils/nodes.py", line 214, in walkabout
    if child.walkabout(visitor):
  File "/usr/lib/python3.9/site-packages/docutils/nodes.py", line 214, in walkabout
    if child.walkabout(visitor):
  File "/usr/lib/python3.9/site-packages/docutils/nodes.py", line 214, in walkabout
    if child.walkabout(visitor):
  [Previous line repeated 4 more times]
  File "/usr/lib/python3.9/site-packages/docutils/nodes.py", line 206, in walkabout
    visitor.dispatch_visit(self)
  File "/usr/lib/python3.9/site-packages/sphinx/util/docutils.py", line 471, in dispatch_visit
    super().dispatch_visit(node)
  File "/usr/lib/python3.9/site-packages/docutils/nodes.py", line 1995, in dispatch_visit
    return method(node)
  File "/usr/lib/python3.9/site-packages/sphinx/writers/latex.py", line 2072, in unknown_visit
    raise NotImplementedError('Unknown node: ' + node.__class__.__name__)
NotImplementedError: Unknown node: substitution_definition
tk0miya commented 3 years ago

My concern is who inserted the substitution_definition nodes (mark-ups, extensions, and so on.). As you pointed, Sphinx & LaTeX builder removes them all. So nothing will be reached to the LaTeX writer. So I feel it's strange to handle it on the writer.

kovidgoyal commented 3 years ago

Well, obviously its getting inserted by something somewhere, since tracing that down appears too difficult why not just ignore the node in the one writer that does not handle it?

brechtm commented 3 years ago

Well, obviously its getting inserted by something somewhere, since tracing that down appears too difficult why not just ignore the node in the one writer that does not handle it?

I think the answer to that is #4827.

kovidgoyal commented 3 years ago

I'd still prefer non-optimal footnote numbers to complete failure.

brechtm commented 3 years ago

I'd still prefer non-optimal footnote numbers to complete failure.

No one is saying this shouldn't be fixed.

Does the crash also occur with a Sphinx version that doesn't include PR #4827 (1.7.2, I think)? A minimal project triggering this bug would be helpful.

kovidgoyal commented 3 years ago

As I said before, I have no idea how to go about creating a minimal reproducer. This occurs in a very large and complex build. The best I can think of would be to remove parts of the build and try to narrow down where it occurs, but that is a pretty extended and painful exercise.

And I dont follow your logic. Adding the do nothing methods to the latex writer will prevent the build failing, they wont change anything about generated footnote numbers. If for your project the numbers were correct before adding the dummy methods, then they will be correct after adding them. If the requirement for the dummy methods indicates that footnote numbers are going to be incorrect, then they would be incorrect in either case.

As I'm afraid I dont have the time/ability to try to create a minimal reproducer, I have just fixed it in my codebase by subclassing the latex builder and adding the dummy methods. https://github.com/kovidgoyal/calibre/commit/0be56041fb4ab4548a90164d64871ee5fa279e28

kovidgoyal commented 3 years ago

Oh one thought does occur to me. In my build, some rst files are dynamically generated on the builder-inited event. They use substitutions. This is probably the source of the issue. I use it to make sure app.config.language is available. If there is a better event to do it on, do let me know.

kovidgoyal commented 3 years ago

Changing it to config-inited seems to do the trick.

kovidgoyal commented 3 years ago

Sadly I spoke too soon that did not work, was fooled by the caching.

tk0miya commented 3 years ago

As I'm afraid I dont have the time/ability to try to create a minimal reproducer, I have just fixed it in my codebase by subclassing the latex builder and adding the dummy methods. kovidgoyal/calibre@0be5604

Could you let me know how to set up the build environment for calibre? I'd like to build the document. I tried to make it, but failed now.

FROM python:3.8-slim

RUN apt update;  apt install -y build-essential curl git make unzip vim
RUN git clone https://github.com/kovidgoyal/calibre
WORKDIR /calibre
RUN git checkout 0be56041fb4ab4548a90164d64871ee5fa279e28~1
RUN apt install -y qt5-qmake
RUN apt install -y qt5-default
RUN apt install -y pkg-config
RUN apt install -y libpodofo-dev
RUN apt install -y libhunspell-dev
RUN apt install -y libfreetype6-dev
RUN apt install -y libhyphen-dev
RUN apt install -y libicu-dev
RUN apt install -y libsqlite3-dev
RUN apt install -y libusb-1.0.0-dev
RUN apt install -y libmtp-dev
RUN apt install -y libmtp-dev
RUN pip install sip
RUN apt install -y qtbase5-private-dev
RUN apt install -y libfontconfig1-dev
RUN pip install pyqt-builder
RUN pip install pyqt5
RUN pip install python-dateutil
RUN pip install lxml
RUN pip install zeroconf
RUN pip install msgpack
RUN pip install PyQtWebEngine
RUN apt install -y libnss3-dev
RUN apt install -y libasound-dev
ENV QT_SELECT 5
RUN python setup.py build
WORKDIR /calibre/manual
RUN pip install Sphinx
RUN pip install css-parser
RUN sphinx-build -b latex . _build/latex

Oh one thought does occur to me. In my build, some rst files are dynamically generated on the builder-inited event. They use substitutions. This is probably the source of the issue. I use it to make sure app.config.language is available. If there is a better event to do it on, do let me know.

I think it's not a reason. All substition_definition nodes are removed just before generating a LaTeX file. So your extension is not related to this error.

kovidgoyal commented 3 years ago

Not a Ubuntu user so not sure if their distro package are up-to-date enough to build current calibre. However, on arch, you can simply run

pacman -S --noconfirm --needed base-devel sudo git sip pyqt-builder chmlib icu jxrlib hunspell libmtp libusb libwmf optipng podofo python-apsw python-beautifulsoup4 python-cssselect python-css-parser python-dateutil python-dbus python-dnspython python-dukpy python-feedparser python-html2text python-html5-parser python-lxml python-markdown python-mechanize python-msgpack python-netifaces python-unrardll python-pillow python-psutil python-pygments python-pyqt5 python-regex python-zeroconf python-pyqtwebengine qt5-x11extras qt5-svg qt5-imageformats udisks2 hyphen python-pychm python-pycryptodome speech-dispatcher python-sphinx python-urllib3 python-py7zr python-pip python-cchardet

to install all needed deps, and then run

curl -L https://calibre-ebook.com/dist/src | tar xvJ; cd calibre*
python setup.py build

the manual is built most easily with

cd manual && ./build.py
tk0miya commented 3 years ago

Thank you for the info. I succeeded to build calibre. But I still failed to build document

# Dockerfile
FROM archlinux

# WORKAROUND for glibc 2.33 and old Docker
# See https://github.com/actions/virtual-environments/issues/2658
# Thanks to https://github.com/lxqt/lxqt-panel/pull/1562
RUN patched_glibc=glibc-linux4-2.33-4-x86_64.pkg.tar.zst && \
    curl -LO "https://repo.archlinuxcn.org/x86_64/$patched_glibc" && \
    bsdtar -C / -xvf "$patched_glibc"

RUN pacman -Sy --noconfirm --needed base-devel sudo git sip pyqt-builder chmlib icu jxrlib hunspell libmtp libusb libwmf optipng podofo python-apsw python-beautifulsoup4 python-cssselect python-css-parser python-dateutil python-dbus python-dnspython python-dukpy python-feedparser python-html2text python-html5-parser python-lxml python-markdown python-mechanize python-msgpack python-netifaces python-unrardll python-pillow python-psutil python-pygments python-pyqt5 python-regex python-zeroconf python-pyqtwebengine qt5-x11extras qt5-svg qt5-imageformats udisks2 hyphen python-pychm python-pycryptodome speech-dispatcher python-sphinx python-urllib3 python-py7zr python-pip python-cchardet
RUN curl -L https://calibre-ebook.com/dist/src | tar xvJ
WORKDIR /calibre-5.12.0
RUN python setup.py build
WORKDIR /calibre-5.12.0/manual
RUN ./build.py

The error I saw is here:

Step 8/8 : RUN ./build.py
 ---> Running in af9b616058c0
Traceback (most recent call last):
  File "/calibre-5.12.0/manual/./build.py", line 101, in <module>
    os.environ['ALL_USER_MANUAL_LANGUAGES'] = ' '.join(json.load(open('locale/completed.json', 'rb')))
FileNotFoundError: [Errno 2] No such file or directory: 'locale/completed.json'
kovidgoyal commented 3 years ago

You can build that missing file with

./setup.py translations

It got left out of the src tarball accidentally.

tk0miya commented 3 years ago

Thanks, confirmed the error now:

FROM archlinux

# WORKAROUND for glibc 2.33 and old Docker
# See https://github.com/actions/virtual-environments/issues/2658
# Thanks to https://github.com/lxqt/lxqt-panel/pull/1562
RUN patched_glibc=glibc-linux4-2.33-4-x86_64.pkg.tar.zst && \
    curl -LO "https://repo.archlinuxcn.org/x86_64/$patched_glibc" && \
    bsdtar -C / -xvf "$patched_glibc"

RUN pacman -Sy --noconfirm --needed base-devel sudo git sip pyqt-builder chmlib icu jxrlib hunspell libmtp libusb libwmf optipng podofo python-apsw python-beautifulsoup4 python-cssselect python-css-parser python-dateutil python-dbus python-dnspython python-dukpy python-feedparser python-html2text python-html5-parser python-lxml python-markdown python-mechanize python-msgpack python-netifaces python-unrardll python-pillow python-psutil python-pygments python-pyqt5 python-regex python-zeroconf python-pyqtwebengine qt5-x11extras qt5-svg qt5-imageformats udisks2 hyphen python-pychm python-pycryptodome speech-dispatcher python-sphinx python-urllib3 python-py7zr python-pip python-cchardet
RUN curl -L https://calibre-ebook.com/dist/src | tar xvJ
WORKDIR /calibre-5.12.0
RUN python setup.py build
RUN python setup.py translations
WORKDIR /calibre-5.12.0/manual
RUN ./build.py en build

And I found the reason of the error. It seems your project uses a custom LaTeX builder to build LaTeX document. But our substitution_definition remover is only available for the original LaTeX builder. So substitution_definition nodes are not removed.

kovidgoyal commented 3 years ago

Cool, thanks. I dont currently need the custom builder anymore so I can remove it, however, I am guessing that this should be fixed in sphinx? My custom builder simply subclasses the builtin ones, so it is unexpected that doing so would break the build.

tk0miya commented 3 years ago

I just posted #8958 to fix this. It will be released as 3.5.2 in this weekend. Please wait a moment. Thank you for your patience.

kovidgoyal commented 3 years ago

Thanks, much obliged.