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

Fix copying images under parallel execution #11100

Merged
merged 6 commits into from Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 7 additions & 4 deletions sphinx/builders/_epub_base.py
Expand Up @@ -401,9 +401,12 @@ def copy_image_files_pil(self) -> None:
the format and resizing the image if necessary/possible.
"""
ensuredir(path.join(self.outdir, self.imagedir))
for src in status_iterator(self.images, __('copying images... '), "brown",
len(self.images), self.app.verbosity):
dest = self.images[src]
converted_images = {*self.env.original_image_uri.values()}
for src in status_iterator(self.env.images, __('copying images... '), "brown",
len(self.env.images), self.app.verbosity):
if src in converted_images:
continue
_docnames, dest = self.env.images[src]
try:
img = Image.open(path.join(self.srcdir, src))
except OSError:
Expand Down Expand Up @@ -438,7 +441,7 @@ def copy_image_files(self) -> None:
"""Copy image files to destination directory.
This overwritten method can use Pillow to convert image files.
"""
if self.images:
if self.env.images:
if self.config.epub_fix_images or self.config.epub_max_image_width:
if not Image:
logger.warning(__('Pillow not found - copying image files'))
Expand Down
11 changes: 7 additions & 4 deletions sphinx/builders/html/__init__.py
Expand Up @@ -764,13 +764,16 @@ def write_domain_indices(self) -> None:
self.handle_page(indexname, indexcontext, 'domainindex.html')

def copy_image_files(self) -> None:
if self.images:
if self.env.images:
converted_images = {*self.env.original_image_uri.values()}
stringify_func = ImageAdapter(self.app.env).get_original_image_uri
ensuredir(path.join(self.outdir, self.imagedir))
for src in status_iterator(self.images, __('copying images... '), "brown",
len(self.images), self.app.verbosity,
for src in status_iterator(self.env.images, __('copying images... '), "brown",
len(self.env.images), self.app.verbosity,
stringify_func=stringify_func):
dest = self.images[src]
if src in converted_images:
continue
_docnames, dest = self.env.images[src]
try:
copyfile(path.join(self.srcdir, src),
path.join(self.outdir, self.imagedir, dest))
Expand Down
11 changes: 7 additions & 4 deletions sphinx/builders/latex/__init__.py
Expand Up @@ -413,12 +413,15 @@ def copy_latex_additional_files(self) -> None:
copy_asset_file(path.join(self.confdir, filename), self.outdir)

def copy_image_files(self) -> None:
if self.images:
if self.env.images:
converted_images = {*self.env.original_image_uri.values()}
stringify_func = ImageAdapter(self.app.env).get_original_image_uri
for src in status_iterator(self.images, __('copying images... '), "brown",
len(self.images), self.app.verbosity,
for src in status_iterator(self.env.images, __('copying images... '), "brown",
len(self.env.images), self.app.verbosity,
stringify_func=stringify_func):
dest = self.images[src]
if src in converted_images:
continue
_docnames, dest = self.env.images[src]
try:
copy_asset_file(path.join(self.srcdir, src),
path.join(self.outdir, dest))
Expand Down
11 changes: 7 additions & 4 deletions sphinx/builders/texinfo.py
Expand Up @@ -173,12 +173,15 @@ def finish(self) -> None:
self.copy_support_files()

def copy_image_files(self, targetname: str) -> None:
if self.images:
if self.env.images:
converted_images = {*self.env.original_image_uri.values()}
stringify_func = ImageAdapter(self.app.env).get_original_image_uri
for src in status_iterator(self.images, __('copying images... '), "brown",
len(self.images), self.app.verbosity,
for src in status_iterator(self.env.images, __('copying images... '), "brown",
len(self.env.images), self.app.verbosity,
stringify_func=stringify_func):
dest = self.images[src]
if src in converted_images:
continue
_docnames, dest = self.env.images[src]
try:
imagedir = path.join(self.outdir, targetname + '-figures')
ensuredir(imagedir)
Expand Down
20 changes: 20 additions & 0 deletions tests/test_build_epub.py
Expand Up @@ -2,6 +2,7 @@

import os
import subprocess
from pathlib import Path
from subprocess import CalledProcessError
from xml.etree import ElementTree

Expand Down Expand Up @@ -390,3 +391,22 @@ def test_xml_name_pattern_check():
assert _XML_NAME_PATTERN.match('id-pub')
assert _XML_NAME_PATTERN.match('webpage')
assert not _XML_NAME_PATTERN.match('1bfda21')


@pytest.mark.sphinx('epub', testroot='images')
def test_copy_images(app, status, warning):
app.build()

images_dir = Path(app.outdir) / '_images'
images = {image.name for image in images_dir.rglob('*')}
assert images == {
'img.gif',
'img.pdf',
'img.png',
'python-logo.png',
'rimg.png',
'rimg1.png',
'svgimg.pdf',
'svgimg.svg',
'testimäge.png',
}
19 changes: 19 additions & 0 deletions tests/test_build_html.py
Expand Up @@ -3,6 +3,7 @@
import os
import re
from itertools import chain, cycle
from pathlib import Path
from unittest.mock import ANY, call, patch

import pytest
Expand Down Expand Up @@ -1770,3 +1771,21 @@ def test_theme_having_multiple_stylesheets(app):

assert '<link rel="stylesheet" type="text/css" href="_static/mytheme.css" />' in content
assert '<link rel="stylesheet" type="text/css" href="_static/extra.css" />' in content


@pytest.mark.sphinx('html', testroot='images')
def test_copy_images(app, status, warning):
app.build()

images_dir = Path(app.outdir) / '_images'
images = {image.name for image in images_dir.rglob('*')}
assert images == {
'img.gif',
'img.pdf',
'img.png',
'rimg.png',
'rimg1.png',
'svgimg.pdf',
'svgimg.svg',
'testimäge.png',
}
23 changes: 23 additions & 0 deletions tests/test_build_latex.py
Expand Up @@ -4,6 +4,7 @@
import re
import subprocess
from itertools import product
from pathlib import Path
from shutil import copyfile
from subprocess import CalledProcessError

Expand Down Expand Up @@ -1670,3 +1671,25 @@ def test_latex_code_role(app):
common_content + '%\n}} code block') in content
assert (r'\begin{sphinxVerbatim}[commandchars=\\\{\}]' +
'\n' + common_content + '\n' + r'\end{sphinxVerbatim}') in content


@pytest.mark.sphinx('latex', testroot='images')
def test_copy_images(app, status, warning):
app.build()

test_dir = Path(app.outdir)
images = {
image.name for image in test_dir.rglob('*')
if image.suffix in {'.gif', '.pdf', '.png', '.svg'}
}
assert images == {
'img.gif',
'img.pdf',
'img.png',
'python-logo.png',
'rimg.png',
'rimg1.png',
'svgimg.pdf',
'svgimg.svg',
'testimäge.png',
}
20 changes: 20 additions & 0 deletions tests/test_build_texinfo.py
Expand Up @@ -3,6 +3,7 @@
import os
import re
import subprocess
from pathlib import Path
from subprocess import CalledProcessError
from unittest.mock import Mock

Expand Down Expand Up @@ -137,3 +138,22 @@ def test_texinfo_samp_with_variable(app, status, warning):
assert '@code{@var{variable_only}}' in output
assert '@code{@var{variable} and text}' in output
assert '@code{Show @var{variable} in the middle}' in output


@pytest.mark.sphinx('texinfo', testroot='images')
def test_copy_images(app, status, warning):
app.build()

images_dir = Path(app.outdir) / 'python-figures'
images = {image.name for image in images_dir.rglob('*')}
assert images == {
'img.gif',
'img.pdf',
'img.png',
'python-logo.png',
'rimg.png',
'rimg1.png',
'svgimg.pdf',
'svgimg.svg',
'testimäge.png',
}