sphinxcontrib-svg2pdfconverter fails due to changed function calls

[Gatherer]

Describe the bug

During tests with Sphinx 5.1.x I found that some of our company documentation projects do not work any more, see https://github.com/missinglinkelectronics/sphinxcontrib-svg2pdfconverter/issues/19.

This error only happens if there are multiple project are defined inside latex_documents.

Is the function call order intended as it is or is it a bug inside Sphinx? If the function call order will stay as is I will update the converter.

How to Reproduce

Any Sphinx

• Latex build
• with multiple latex_documents
• where a SVG figure needs to be converted
• with Inkscape

should fail

Expected behavior

The is_available() function is called for every document defined in latex_documents.

Your project

can not share, can create a minimal project if requested

Screenshots

No response

OS

reproduced on Linux Ubuntu 18.04 LTS, Mint 20.3

Python version

3.6, 3.8

Sphinx version

5.1.1

Sphinx extensions

docutils<0.18 https://github.com/brechtm/rinohtype/archive/refs/tags/v0.5.3.tar.gz https://github.com/sphinx-doc/sphinx/archive/refs/tags/v5.1.1.tar.gz sphinx_rtd_theme==1.0.0 sphinx-autobuild==2021.3.14 sphinxcontrib-bibtex==2.4.1 sphinxcontrib-globalsubs==0.1.0 sphinxcontrib-scalebybuilder==0.1.0 sphinxcontrib-svg2pdfconverter==1.2.0 sphinxcontrib-wavedrom==3.0.2 sphinx-multitoc-numbering==0.1.3

Extra tools

pdfTeX 3.14159265-2.6-1.40.20 (TeX Live 2019/Debian)

Additional context

No response

[jfbu]

@tk0miya will tell better but is seems your code exploits is_available() call for after-effect. How about this:

--- inkscapeconverter-old.py	2022-08-15 13:58:14.000000000 +0200
+++ inkscapeconverter.py	2022-08-15 14:29:03.000000000 +0200
@@ -33,6 +33,8 @@
         ('image/svg+xml', 'application/pdf'),
     ]
 
+    inkscape_version: str = None
+
     def is_available(self):
         # type: () -> bool
         """Confirms if Inkscape is available or not."""
@@ -48,8 +50,8 @@
                                   'Check the inkscape_converter_bin setting'),
                                self.config.inkscape_converter_bin, output)
                 return False
-            self._inkscape_version = match.group(1)
-            logger.debug('Inkscape version: %s', self._inkscape_version)
+            InkscapeConverter.inkscape_version = match.group(1)
+            logger.debug('Inkscape version: %s', InkscapeConverter.inkscape_version)
             return True
         except subprocess.CalledProcessError:
             return False
@@ -65,7 +67,7 @@
         try:
             args = ([self.config.inkscape_converter_bin] +
                     self.config.inkscape_converter_args)
-            if self._inkscape_version.startswith('1.'):
+            if InkscapeConverter.inkscape_version.startswith('1.'):
                     args += ['--export-filename=' + _to, _from]
             else:
                     args += ['--export-pdf=' + _to, _from]

This is quite possibly bad coding style (LaTeX guy here), but it seems to work at my locale. Now perhaps the real question is whether ImageConverter class should provide some means to set up extra data once the converter is found to be available, so it would avoid the temptation of abusing is_available() to set up things.

[Gatherer]

quite the same idea I had if the result of this bug is

If the function call order will stay as is I will update the converter.

but would discuss it here before. Maybe there are needs or requests to also take care of.

[jfbu]

let's wait for @tk0miya :smile: I expect the idea is that is_available() was supposed safe to be called only once, but in your extension it is doing extra set-up. It seems natural ImageConverter class should provide structure to hold such extra data if needed.

[Gatherer]

ok .. looks like it's stuck ... so I will probably go with the class variable solution already posted as soon as I back from vacation.

[Gatherer]

New version of [sphinxcontrib-svg2pdfconverter] released to solve this issue which wasn't a sphinx bug.