Use latexmk if Sphinx > 1.6 (#5656)

Use latexmk if Sphinx > 1.6

Author: humitos

docs/guides/feature-flags.rst

@@ -12,8 +12,6 @@ or disable one or more of these featured flags for a particular project.
 Available Flags
 ---------------
 
-``USE_PDF_LATEXMK``: :featureflags:`USE_PDF_LATEXMK`
-
 ``USE_SPHINX_LATEST``: :featureflags:`USE_SPHINX_LATEST`
 
 ``ALLOW_DEPRECATED_WEBHOOKS``: :featureflags:`ALLOW_DEPRECATED_WEBHOOKS`

docs/guides/pdf-non-ascii-languages.rst

@@ -1,13 +1,6 @@
 Build PDF format for non-ASCII languages
 ========================================
 
-
-.. warning::
-
-   To be able to follow this guide and build PDF with this method,
-   you need to ask the Read the Docs core team to enable ``USE_PDF_LATEXMK`` :doc:`feature flag </guides/feature-flags>` in your project.
-   Please, `open an issue`_ in our repository asking for this, and wait for one of the core team to enable it.
-
 .. _open an issue: https://github.com/rtfd/readthedocs.org/issues/new
 
 Sphinx offers different `LaTeX engines`_ that support Unicode characters and non-ASCII languages,

readthedocs/doc_builder/backends/sphinx.py

@@ -146,9 +146,6 @@ def get_config_params(self):
             'dont_overwrite_sphinx_context': self.project.has_feature(
                 Feature.DONT_OVERWRITE_SPHINX_CONTEXT,
             ),
-            'use_pdf_latexmk': self.project.has_feature(
-                Feature.USE_PDF_LATEXMK,
-            ),
         }
 
         finalize_sphinx_context_data.send(
@@ -227,6 +224,38 @@ def build(self):
         )
         return cmd_ret.successful
 
+    def venv_sphinx_supports_latexmk(self):
+        """
+        Check if ``sphinx`` from the user's venv supports ``latexmk``.
+
+        If the version of ``sphinx`` is greater or equal to 1.6.1 it returns
+        ``True`` and ``False`` otherwise.
+
+        See: https://www.sphinx-doc.org/en/master/changes.html#release-1-6-1-released-may-16-2017
+        """
+
+        command = [
+            self.python_env.venv_bin(filename='python'),
+            '-c',
+            (
+                '"'
+                'import sys; '
+                'import sphinx; '
+                'sys.exit(0 if sphinx.version_info >= (1, 6, 1) else 1)'
+                '"'
+            ),
+        ]
+
+        cmd_ret = self.run(
+            *command,
+            bin_path=self.python_env.venv_bin(),
+            cwd=self.project.checkout_path(self.version.slug),
+            escape_command=False,  # used on DockerBuildCommand
+            shell=True,  # used on BuildCommand
+            record=False,
+        )
+        return cmd_ret.exit_code == 0
+
 
 class HtmlBuilder(BaseSphinx):
     type = 'sphinx'
@@ -390,7 +419,9 @@ def build(self):
             raise BuildEnvironmentError('No TeX files were found')
 
         # Run LaTeX -> PDF conversions
-        if self.project.has_feature(Feature.USE_PDF_LATEXMK):
+        # Build PDF with ``latexmk`` if Sphinx supports it, otherwise fallback
+        # to ``pdflatex`` to support old versions
+        if self.venv_sphinx_supports_latexmk():
             return self._build_latexmk(cwd, latex_cwd)
 
         return self._build_pdflatex(tex_files, latex_cwd)

readthedocs/doc_builder/environments.py

@@ -81,6 +81,7 @@ class BuildCommand(BuildCommandResultMixin):
     :param build_env: build environment to use to execute commands
     :param bin_path: binary path to add to PATH resolution
     :param description: a more grokable description of the command being run
+    :param kwargs: allow to subclass this class and extend it
     """
 
     def __init__(
@@ -95,6 +96,7 @@ def __init__(
             bin_path=None,
             description=None,
             record_as_success=False,
+            **kwargs,
     ):
         self.command = command
         self.shell = shell
@@ -164,8 +166,13 @@ def run(self):
             environment['PATH'] = ':'.join(env_paths)
 
         try:
+            # When using ``shell=True`` the command should be flatten
+            command = self.command
+            if self.shell:
+                command = self.get_command()
+
             proc = subprocess.Popen(
-                self.command,
+                command,
                 shell=self.shell,
                 # This is done here for local builds, but not for docker,
                 # as we want docker to expand inside the container
@@ -294,14 +301,20 @@ class DockerBuildCommand(BuildCommand):
     Build command to execute in docker container
     """
 
-    def run(self):
+    def __init__(self, *args, escape_command=True, **kwargs):
         """
-        Execute command in existing Docker container.
+        Override default to extend behavior.
 
-        :param cmd_input: input to pass to command in STDIN
-        :type cmd_input: str
-        :param combine_output: combine STDERR into STDOUT
+        :param escape_command: whether escape special chars the command before
+            executing it in the container. This should only be disabled on
+            trusted or internal commands.
+        :type escape_command: bool
         """
+        self.escape_command = escape_command
+        super(DockerBuildCommand, self).__init__(*args, **kwargs)
+
+    def run(self):
+        """Execute command in existing Docker container."""
         log.info(
             "Running in container %s: '%s' [%s]",
             self.build_env.container_id,
@@ -350,13 +363,15 @@ def run(self):
 
     def get_wrapped_command(self):
         """
-        Escape special bash characters in command to wrap in shell.
+        Wrap command in a shell and optionally escape special bash characters.
 
         In order to set the current working path inside a docker container, we
-        need to wrap the command in a shell call manually. Some characters will
-        be interpreted as shell characters without escaping, such as: ``pip
-        install requests<0.8``. This escapes a good majority of those
-        characters.
+        need to wrap the command in a shell call manually.
+
+        Some characters will be interpreted as shell characters without
+        escaping, such as: ``pip install requests<0.8``. When passing
+        ``escape_command=True`` in the init method this escapes a good majority
+        of those characters.
         """
         bash_escape_re = re.compile(
             r"([\t\ \!\"\#\$\&\'\(\)\*\:\;\<\>\?\@"
@@ -365,16 +380,18 @@ def get_wrapped_command(self):
         prefix = ''
         if self.bin_path:
             prefix += 'PATH={}:$PATH '.format(self.bin_path)
+
+        command = (
+            ' '.join([
+                bash_escape_re.sub(r'\\\1', part) if self.escape_command else part
+                for part in self.command
+            ])
+        )
         return (
             "/bin/sh -c 'cd {cwd} && {prefix}{cmd}'".format(
                 cwd=self.cwd,
                 prefix=prefix,
-                cmd=(
-                    ' '.join([
-                        bash_escape_re.sub(r'\\\1', part)
-                        for part in self.command
-                    ])
-                ),
+                cmd=command,
             )
         )
 

readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl

@@ -141,7 +141,6 @@ if 'extensions' in globals():
 else:
     extensions = ["readthedocs_ext.readthedocs"]
 
-{% if use_pdf_latexmk %}
 project_language = '{{ project.language }}'
 
 # User's Sphinx configurations
@@ -173,4 +172,3 @@ if chinese:
     latex_elements = latex_elements_user or latex_elements_rtd
 elif japanese:
     latex_engine = latex_engine_user or 'platex'
-{% endif %}

readthedocs/projects/models.py

@@ -1358,12 +1358,10 @@ def add_features(sender, **kwargs):
     DONT_SHALLOW_CLONE = 'dont_shallow_clone'
     USE_TESTING_BUILD_IMAGE = 'use_testing_build_image'
     SHARE_SPHINX_DOCTREE = 'share_sphinx_doctree'
-    USE_PDF_LATEXMK = 'use_pdf_latexmk'
     DEFAULT_TO_MKDOCS_0_17_3 = 'default_to_mkdocs_0_17_3'
 
     FEATURES = (
         (USE_SPHINX_LATEST, _('Use latest version of Sphinx')),
-        (USE_PDF_LATEXMK, _('Use latexmk to build the PDF')),
         (ALLOW_DEPRECATED_WEBHOOKS, _('Allow deprecated webhook views')),
         (PIP_ALWAYS_UPGRADE, _('Always run pip install --upgrade')),
         (SKIP_SUBMODULES, _('Skip git submodule checkout')),

readthedocs/rtd_tests/tests/test_builds.py

@@ -220,6 +220,7 @@ def test_build_pdf_latex_failures(self, load_config):
         returns = [
             ((b'', b''), 0),  # sphinx-build html
             ((b'', b''), 0),  # sphinx-build pdf
+            ((b'', b''), 1),  # sphinx version check
             ((b'', b''), 1),  # latex
             ((b'', b''), 0),  # makeindex
             ((b'', b''), 0),  # latex
@@ -236,7 +237,7 @@ def test_build_pdf_latex_failures(self, load_config):
 
         with build_env:
             task.build_docs()
-        self.assertEqual(self.mocks.popen.call_count, 7)
+        self.assertEqual(self.mocks.popen.call_count, 8)
         self.assertTrue(build_env.failed)
 
     @mock.patch('readthedocs.doc_builder.config.load_config')
@@ -271,6 +272,7 @@ def test_build_pdf_latex_not_failure(self, load_config):
         returns = [
             ((b'', b''), 0),  # sphinx-build html
             ((b'', b''), 0),  # sphinx-build pdf
+            ((b'', b''), 1),  # sphinx version check
             ((b'Output written on foo.pdf', b''), 1),  # latex
             ((b'', b''), 0),  # makeindex
             ((b'', b''), 0),  # latex
@@ -287,7 +289,7 @@ def test_build_pdf_latex_not_failure(self, load_config):
 
         with build_env:
             task.build_docs()
-        self.assertEqual(self.mocks.popen.call_count, 7)
+        self.assertEqual(self.mocks.popen.call_count, 8)
         self.assertTrue(build_env.successful)
 
     @mock.patch('readthedocs.projects.tasks.api_v2')