sphinx_immaterial.custom_admonitions always flagged as "changed"

[kartben]

Unless I explicitly comment out app.setup_extension("sphinx_immaterial.custom_admonitions"), an incremental build always detects a change with custom_admonitions and rebuilds everything. Commenting out the extension restores the expected behavior where only changed files get rebuilt.

Thanks!

[2bndy5]

I noticed this too. https://github.com/jbms/sphinx-immaterial/blob/c61bad99139f17d535b9a287fac920f6d639b416/sphinx_immaterial/custom_admonitions.py#L483 When reviewing the PR, it was suggested to flag the extension for rebuild on env change because the custom admonition icons (and inline icons) used are stored in the builder env before output via jinja CSS template and consolidated with the bundled theme CSS.

So, it is possible that another extension (possibly one of the theme's own) is triggering a rebuild by updating the builder env.

[kartben]

So, it is possible that another extension (possibly one of the theme's own) is triggering a rebuild by updating the builder env.

Ya, this 👇 looks suspicious, no?

https://github.com/jbms/sphinx-immaterial/blob/c61bad99139f17d535b9a287fac920f6d639b416/sphinx_immaterial/custom_admonitions.py#L419

[2bndy5]

Hmm. Possibly, but we need to create that env field when the builder is initialized (after the custom admonitions' config has been parsed/validated). We can't go without it. In fact, that line is the reason the custom_admonitions ext is set to rebuild on env changes.

I have to review the event chain in Sphinx. I'm under the impression that line shouldn't trigger a rebuild ~since~ if it happens after the conf.py is loaded (config_inited).

[2bndy5]

The custom_admonitions ext is tightly coupled with the inline_icons ext because they use the same cache of SVG data in the env. https://github.com/jbms/sphinx-immaterial/blob/c61bad99139f17d535b9a287fac920f6d639b416/sphinx_immaterial/inline_icons.py#L26 This is one reason why both are always enabled: https://github.com/jbms/sphinx-immaterial/blob/c61bad99139f17d535b9a287fac920f6d639b416/sphinx_immaterial/__init__.py#L331-L332

[chrisjsewell]

Yep you can't change the config in builder-inited events, only in config-inited, you should split up the logic of on_builder_inited; to mutate the config in a config-inited event, and then use the config in a seperate builder-inited event.

This is definietly important to fix, to make this theme viable for use thanks 😄

[2bndy5]

I didn't realize we were modifying the original config objects (pydantic data models). I made a switch to using a copy of the config, and now the docs still get triggered for full rebuild by the jinja ext config.

Tried it on a quickstart project, and it doesn't trigger a full rebuild! 🎉

[chrisjsewell]

Thanks @2bndy5 I checked a build with #285 and it does indeed appear to remove the changed config behaviour 👌

[2bndy5]

Nightly for that merge commit is on test-pypi (includes mathjax dist fix).

There are still some other things almost ready for release.

[kartben]

Nightly for that merge commit is on test-pypi (includes mathjax dist fix).

I am getting a weird encoding error using this build?

    ~/zephyrp/zephyr/do/_build    sphinx-immaterial *27 !1  ninja html                  ✔  17s   zephyrproject   14:00:11  
[0/2] Generating Devicetree bindings documentation...
[1/2] Running Sphinx HTML build...
Traceback (most recent call last):
  File "/Users/kartben/zephyrproject/.venv/bin/sphinx-build", line 5, in <module>
    from sphinx.cmd.build import main
  File "/Users/kartben/zephyrproject/.venv/lib/python3.9/site-packages/sphinx/__init__.py", line 43, in <module>
    ret = subprocess.run(
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 507, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1134, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 2021, in _communicate
    stderr = self._translate_newlines(stderr,
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1011, in _translate_newlines
    data = data.decode(encoding, errors)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 5: ordinal not in range(128)
FAILED: CMakeFiles/html /Users/kartben/zephyrproject/zephyr/doc/_build/CMakeFiles/html 
cd /Users/kartben/zephyrproject/zephyr/doc/_build && /opt/homebrew/Cellar/cmake/3.27.4/bin/cmake -E env DOXYGEN_EXECUTABLE=/Volumes/Doxygen/Doxygen.app/Contents/Resources/doxygen DOT_EXECUTABLE=/opt/homebrew/bin/dot /Users/kartben/zephyrproject/.venv/bin/sphinx-build -b html -c /Users/kartben/zephyrproject/zephyr/doc -d /Users/kartben/zephyrproject/zephyr/doc/_build/doctrees -w /Users/kartben/zephyrproject/zephyr/doc/_build/html.log -t development -j auto -W --keep-going -T /Users/kartben/zephyrproject/zephyr/doc/_build/src /Users/kartben/zephyrproject/zephyr/doc/_build/html

I am pulling the nightly build using

--extra-index-url https://test.pypi.org/simple/
sphinx-immaterial==0.11.7.post1.dev2

in my requirements.txt. Revertying to 0.11.7 immediately fixes the problem.

Thanks!

[kartben]

FWIW the same happens if I manually install with pip install -i https://test.pypi.org/simple/ sphinx-immaterial==0.11.7.post1.dev2 --force-reinstall --no-cache-dir. I'm on OSX, will try on my Windows machine...

[2bndy5]

here are the changes since last release: https://github.com/jbms/sphinx-immaterial/compare/v0.11.7...main

I'm thinking that maybe the source encoding of the newly added mathjax assets might being doing something funky.

[kartben]

I will need to investigate some more but please assume PEBKAC for now :)

[2bndy5]

fix available in v0.11.8