diff --git a/docs/guides/feature-flags.rst b/docs/guides/feature-flags.rst index 4e257ab46b5..572cdffdee4 100644 --- a/docs/guides/feature-flags.rst +++ b/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` diff --git a/docs/guides/pdf-non-ascii-languages.rst b/docs/guides/pdf-non-ascii-languages.rst index 6ab46ea58f4..0c18b277a42 100644 --- a/docs/guides/pdf-non-ascii-languages.rst +++ b/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 ` 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, diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index a71ac354b4c..4f66bbc3986 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -10,7 +10,6 @@ import logging import os import shutil -import sys import zipfile from glob import glob from pathlib import Path @@ -149,9 +148,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( @@ -232,6 +228,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' @@ -395,7 +423,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) diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index cda73492dea..e83d5e26adb 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -83,6 +83,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__( @@ -97,6 +98,7 @@ def __init__( bin_path=None, description=None, record_as_success=False, + **kwargs, ): self.command = command self.shell = shell @@ -166,8 +168,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 @@ -296,14 +303,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, @@ -352,13 +365,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\ \!\"\#\$\&\'\(\)\*\:\;\<\>\?\@" @@ -367,16 +382,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, ) ) diff --git a/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl b/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl index b5dac3f8505..fca9d18d1f2 100644 --- a/readthedocs/doc_builder/templates/doc_builder/conf.py.tmpl +++ b/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 %} diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 02127823c84..3f9204f0a3a 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1337,12 +1337,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')), diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index 254d8630713..76c2c09a75b 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/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')