jdillard / sphinx-sitemap

Sphinx extension to generate a multi-lingual, multi-version sitemap for HTML builds
https://sphinx-sitemap.readthedocs.io/en/latest/index.html
MIT License
55 stars 21 forks source link

:bug: FIX: Fix incremental building by preventing multiprocessing queue from being pickled with environment #62

Closed bstrdsmkr closed 1 year ago

bstrdsmkr commented 1 year ago

Incremental builds with this extension are currently broken. #47 added support for parallel building via a multiprocessing.Manager().Queue() proxy object attached to the Environment. Sphinx later pickles the Environment object, including the Queue proxy object. The object pickles successfully, but when the Environment is later unpickled to determine if an incremental build can be used, the proxy object tries to reconnect to the server process that existed at the time of pickling, but no longer exists. This failure is near-silently swallowed with an unintuitive error message and sphinx proceeds with a full rebuild:

$ sphinx-build docs/ _build -b html -T -D html_baseurl=https://example.com
Running Sphinx v6.1.3
loading pickled environment... failed
failed: [Errno 2] No such file or directory
building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 87 source files that are out of date
updating environment: [new config] 87 added, 0 changed, 0 removed

The real error isn't revealed until trying to manually unpickle the environment:

$ python
Python 3.10.6 (main, Nov 14 2022, 16:10:14) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pickle
>>> with open('_build/.doctrees/environment.pickle', 'rb') as f:
...   p=pickle.Unpickler(f).load()
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/usr/lib/python3.10/multiprocessing/managers.py", line 942, in RebuildProxy
    return func(token, serializer, incref=incref, **kwds)
  File "/usr/lib/python3.10/multiprocessing/managers.py", line 990, in AutoProxy
    proxy = ProxyType(token, serializer, manager=manager, authkey=authkey,
  File "/usr/lib/python3.10/multiprocessing/managers.py", line 792, in __init__
    self._incref()
  File "/usr/lib/python3.10/multiprocessing/managers.py", line 846, in _incref
    conn = self._Client(self._token.address, authkey=self._authkey)
  File "/usr/lib/python3.10/multiprocessing/connection.py", line 502, in Client
    c = SocketClient(address)
  File "/usr/lib/python3.10/multiprocessing/connection.py", line 630, in SocketClient
    s.connect(address)
FileNotFoundError: [Errno 2] No such file or directory
>>>

Sphinx specifically excludes the instance of app that's attached to the Environment from pickling via it's __getstate__() method: https://github.com/sphinx-doc/sphinx/blob/ba080286b06cb9e0cadec59a6cf1f96aa11aef5a/sphinx/environment/__init__.py#L262

The PR moves the Queue proxy object to the instance of app provided on the Environment in order to avoid pickling the Queue object while it is connected to it's server process.

As an alternative, It should be possible to create a custom Queue class with it's own __getstate__() method to not attempt to reconnect upon unpickling, but I believe the Most Right Answer is for upstream Sphinx to provide a way to mark custom Environment attributes as unpicklable

jdillard commented 1 year ago

Thanks for the thorough explanation!

clalancette commented 1 year ago

Is there any chance we can get a new release with this fix in it? We are using this (excellent) plugin in https://github.com/ros2/ros2_documentation/ , and the ability to have incremental builds working would really make my life better. Thanks in advance!

jdillard commented 1 year ago

Apologies for the long delay, but 2.5.1 is out now if you want to give it a try: https://github.com/jdillard/sphinx-sitemap/releases/tag/v2.5.1

clalancette commented 1 year ago

Apologies for the long delay, but 2.5.1 is out now if you want to give it a try: https://github.com/jdillard/sphinx-sitemap/releases/tag/v2.5.1

Thank you for the release, it is much appreciated!