Skip to content

6.1.2 breaks the aafigure extension, it worked in 6.1.1 #11117

Closed
Closed
6.1.2 breaks the aafigure extension, it worked in 6.1.1#11117
Labels
builderbuilder:htmltype:bug
Milestone
6.1.3

Description

@viblo
viblo
opened on Jan 9, 2023

Describe the bug

The latest version of sphinx, 6.1.2, breaks the aafigure extension, it makes it try to copy the src folder instead of the actual images I think.

Traceback (most recent call last):
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\builders\html\__init__.py", line 778, in copy_image_files
    copyfile(path.join(self.srcdir, src),
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\util\osutil.py", line 97, in copyfile
    shutil.copyfile(source, dest)
  File "C:\Users\Victor\miniforge3\lib\shutil.py", line 265, in copyfile
    with open(src, 'rb') as fsrc, open(dst, 'wb') as fdst:
PermissionError: [Errno 13] Permission denied: 'C:\\path-to-docs\\src\\.'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\cmd\build.py", line 284, in build_main
    app.build(args.force_all, args.filenames)
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\application.py", line 347, in build
    self.builder.build_update()
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\builders\__init__.py", line 311, in build_update
    self.build(to_build,
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\builders\__init__.py", line 380, in build
    self.finish()
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\builders\html\__init__.py", line 685, in finish
    self.finish_tasks.add_task(self.copy_image_files)
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\util\parallel.py", line 38, in add_task
    res = task_func()
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\builders\html\__init__.py", line 781, in copy_image_files
    logger.warning(__('cannot copy image file %r: %s'),
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 1812, in warning
    self.log(WARNING, msg, *args, **kwargs)
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\util\logging.py", line 127, in log
    super().log(level, msg, *args, **kwargs)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 1844, in log
    self.logger.log(level, msg, *args, **kwargs)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 1512, in log
    self._log(level, msg, args, **kwargs)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 1589, in _log
    self.handle(record)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 1599, in handle
    self.callHandlers(record)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 1661, in callHandlers
    hdlr.handle(record)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 948, in handle
    rv = self.filter(record)
  File "C:\Users\Victor\miniforge3\lib\logging\__init__.py", line 806, in filter
    result = f.filter(record)
  File "C:\Users\Victor\miniforge3\lib\site-packages\sphinx\util\logging.py", line 427, in filter
    raise exc
sphinx.errors.SphinxWarning: cannot copy image file 'C:\\path-to-docs\\src\\.': [Errno 13] Permission denied: 'C:\\path-to-docs\\src\\.'

How to Reproduce

index.rst with empty aafigure directive + conf.py with the extension is enough to trigger the error:

.. aafig::

extensions = ["aafigure.sphinxext"]

Environment Information

Platform:              win32; (Windows-10-10.0.22621-SP0)
Python version:        3.9.7 | packaged by conda-forge | (default, Sep 29 2021, 19:15:42) [MSC v.1916 64 bit (AMD64)])
Python implementation: CPython
Sphinx version:        6.1.2
Docutils version:      0.19
Jinja2 version:        3.0.3
Pygments version:      2.14.0

Sphinx extensions

["aafigure.sphinxext"]

Additional context

I wonder if its this PR that causes it? #11100

Activity

  1. ksunden commented on Jan 9, 2023

    @ksunden
    ksunden
    on Jan 9, 2023
    Contributor

    Matplotlib's sphinx extension is also similarly affected (not sure yet if it is the same cause, we are getting assertion errors in our test suite rather than direct errors raised by sphinx, but it is new with 6.1.2)

    Ours does seem to be affected by #11100, which causes additional .py files to appear in the images folder (in addition to the _downloads directory) and we assert that only one file with that name exists.

  2. ksunden commented on Jan 9, 2023

    @ksunden
    ksunden
    on Jan 9, 2023
    Contributor

    Okay, my digging has shown that #11100 changed from using self.images to self.env.images in the builders (see the html builder for example).

    self.images is commented as "images that need to be copied over", but seems to no longer be used for this purpose as of #11100.

    self.images contains a narrower set of items extracted from self.env.images in Builder.post_process_images, which (among some other things), seems to cut out anything that is not a "supported image type" (such as a py file for matplotlib or a directory for aafigure).

    For aafigure, this means trying to copy a folder by opening it as a file, which is an PermissionError. For matplotlib, this means that a py file gets copied where it hadn't been before and our tests catch that change.

    Arguably, the proper fix is to prevent unsupported image types from entering self.env.images in ImageCollector.

    The logic around guess_mimetype in collect_candidates ends up getting a "mimetype" of "image/x-py" for python files since it is pre-wired to treat everything it sees as an image type.

  3. AA-Turner commented on Jan 9, 2023

    @AA-Turner
    AA-Turner
    on Jan 9, 2023
    Member

    The change to .env.images was done to allow more parallelism. If there are changes Sphinx should make here, would you be able to open a PR? I may have time to review it but due to other commitments I won't be able to look at this myself before May-ish.

    A

  4. ksunden
    added a commit that references this issue on Jan 10, 2023

    Pin sphinx != 6.1.2

    d4e68ca

  5. jfbu commented on Jan 10, 2023

    @jfbu
    jfbu
    on Jan 10, 2023
    Contributor

    The following patch (on 6.1.x) appears to fix the aafigure problem:

    diff --git a/sphinx/environment/collectors/asset.py b/sphinx/environment/collectors/asset.py
index 1ab7c09de..cb7ec9a34 100644
--- a/sphinx/environment/collectors/asset.py
+++ b/sphinx/environment/collectors/asset.py
@@ -45,6 +45,13 @@ class ImageCollector(EnvironmentCollector):
             candidates: dict[str, str] = {}
             node['candidates'] = candidates
             imguri = node['uri']
+            if imguri == "":
+                # image nodes created (via directive from extensions) via
+                # docutils.parsers.directives.images.Image.run()
+                # may create node of the type <image uri=""/>.  The image
+                # files do not exist yet in source, let's skip.
+                candidates['?'] = imguri
+                continue
             if imguri.startswith('data:'):
                 candidates['?'] = imguri
                 continue

    (the comments I added are for my own education and not quite precise; thanks @ksunden for your analysis, I have not checked the matplotlib context, only tested with aafigure)

    PS @ksunden do you happen to have a small project showing the issue with .py files?

  6. AA-Turner commented on Jan 10, 2023

    @AA-Turner
    AA-Turner
    on Jan 10, 2023
    Member

    I have released v6.1.3 which should fix this by reverting the parallel image changes.

    A

  7. ksunden commented on Jan 10, 2023

    @ksunden
    ksunden
    on Jan 10, 2023
    Contributor

    https://gist.github.com/ksunden/1b726225cbc93e65c7e478fda4dd5a1f

    Here is a gist with a fairly minimal conf/Makefile extracted from our tests (still using the mpl extension, so not minimal in that dimension, but outside of that it is fairly minimal) (the makefile was edited to create an empty _static folder because there was a warning without it but it is otherwise unused and gists can't have folders)

    It builds without warning/error, but the .py file generated from the plot directive source ends up in the build/html/_images directory, which is unexpected.

    I will note that mpl's sphinx extension was recently modified in matplotlib/matplotlib#24205 and undoing that change does actually fix our tests, but leaves many files in build/html directly (but does not treat py files as images).

  8. viblo commented on Jan 10, 2023

    @viblo
    viblo
    on Jan 10, 2023
    Author

    I can confirm that v6.1.3 with the reverted changes fixes the aafigure issue for me.
    Thanks for the quick action!

  9. jfbu commented on Jan 10, 2023

    @jfbu
    jfbu
    on Jan 10, 2023
    Contributor

    I will note that mpl's sphinx extension was recently modified in matplotlib/matplotlib#24205 and undoing that change does actually fix our tests, but leaves many files in build/html directly (but does not treat py files as images).

    Thanks @ksunden for very small example indeed (I used the bare sphinx-quickstart Makefile). I had not seen the issue when trying directly earlier because I was using Matplotlib 3.6.2 (see below).

    The :show-source-link: is not in the last release 3.6.2 of matplotlib so I installed from the git clone the current dev. Then I realized I reproduced the issue with Sphinx 6.1.2 and latest Matplotlib-dev also without using the :show-source-link: option so I removed it from then on. (as far as I could tell later this option allows to remove the link to the source code, but by default it is there and was already there with earlier matplotlib).

    With 3.7.0.dev1292+ge0dc18d51e

    • Sphinx 5.3.0:
    $ find _build/html -name '*.py'
_build/html/_downloads/25f419794f3589105a3cedf01038cfc8/index-1.py
    • Sphinx 6.1.2:
    $ find _build/html -name '*.py'
_build/html/_downloads/25f419794f3589105a3cedf01038cfc8/index-1.py
_build/html/_images/index-1.py
    • Sphinx 6.1.3
    $ find _build/html -name '*.py'
_build/html/_downloads/25f419794f3589105a3cedf01038cfc8/index-1.py

    With matplotlib 3.6.2 on the other hand all versions of Sphinx act the same

    • Sphinx 5.3.0 and 6.1.2 and 6.1.3
    $ find _build/html -name '*.py'
_build/html/index-1.py

    The source code link next to the plot rendering appears to work fine but I checked only with Sphinx 6.1.3 and either Matplotlib 3.6.2 or the dev version which puts the .py file in _downloads/ rather than directly under $BUILDDIR/html. The Building editable for matplotlib (pyproject.toml) ... done for install from git clone (with or without editable) is a bit longish and I was too dumb to think of creating one new venv to do it there rather than alternating with pip install matplotlib==3.6.2.

    In summary it seems users of Matplotlib will not be hit by this Sphinx 6.1.2 issue because it is not seen with Matplotlib 3.6.2 but requires the dev version. And Sphinx has already released 6.1.3 today.

    My [patch above on Sphinx 6.1.2](https://github.com//issues/11117#issuecomment-1376936102) which fixed the aafigure problem is irrelevant to stuff with ``.py`` files.
  10. jfbu
    added
    type:bug
    builder:html
    builder
    on Jan 10, 2023
  11. jfbu
    added this to the 6.1.3 milestone on Jan 10, 2023

  12. jfbu commented on Jan 10, 2023

    @jfbu
    jfbu
    on Jan 10, 2023
    Contributor

    Closing this as 6.1.3 release resolved the problem via 2a7c40d.

    Relates #11100.

  13. jfbu
    closed this as completedon Jan 10, 2023
  14. github-actions
    locked as resolved and limited conversation to collaborators on Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    builderbuilder:htmltype:bug

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions