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

LaTeX builder fails with Unknown node: substitution_definition #8936

Closed

Comments

@kovidgoyal
Copy link

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
Copy link
Member

tk0miya commented Feb 26, 2021

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

@tk0miya tk0miya added this to the 3.5.2 milestone Feb 26, 2021
@tk0miya
Copy link
Member

tk0miya commented Feb 27, 2021

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

tk0miya commented Feb 27, 2021

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
Copy link
Author

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
Copy link
Contributor

brechtm commented Feb 27, 2021

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
Copy link
Author

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

@brechtm
Copy link
Contributor

brechtm commented Feb 28, 2021

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 added a commit to kovidgoyal/calibre that referenced this issue Feb 28, 2021
@kovidgoyal
Copy link
Author

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. kovidgoyal/calibre@0be5604

@kovidgoyal
Copy link
Author

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
Copy link
Author

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

@kovidgoyal
Copy link
Author

kovidgoyal commented Feb 28, 2021

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

@kovidgoyal kovidgoyal reopened this Feb 28, 2021
@tk0miya
Copy link
Member

tk0miya commented Mar 4, 2021

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.

@tk0miya tk0miya modified the milestones: 3.5.2, 4.0.0 Mar 4, 2021
@kovidgoyal
Copy link
Author

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
Copy link
Member

tk0miya commented Mar 4, 2021

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
Copy link
Author

You can build that missing file with

./setup.py translations

It got left out of the src tarball accidentally.

@tk0miya
Copy link
Member

tk0miya commented Mar 4, 2021

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
Copy link
Author

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 added a commit to tk0miya/sphinx that referenced this issue Mar 4, 2021
… node error

At present, post transform components for LaTeX builder does not work
for the custom LaTeX builder that inherits the original LaTeX builder.
This allows them working with the custom LaTeX builders.
kovidgoyal added a commit to kovidgoyal/calibre that referenced this issue Mar 4, 2021
Since we no longer need Cyrillic text, we dont need a custom builder.
And apparently sphinx breaks with a custom latex builder. See sphinx-doc/sphinx#8936
This was referenced Mar 12, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.