diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 4d6ae67153b..0d0174176d0 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -296,17 +296,8 @@ def build(self): 'mkdocs', self.builder, "--clean", - # ``site_dir`` is relative to where the mkdocs.yaml file is - # https://www.mkdocs.org/user-guide/configuration/#site_dir - # Example: - # - # when ``--config-file=docs/mkdocs.yml``, - # it must be ``--site-dir=../_readthedocs/html`` "--site-dir", - os.path.join( - os.path.relpath(self.project_path, os.path.dirname(self.yaml_file)), - self.build_dir, - ), + os.path.join("$READTHEDOCS_OUTPUT", "html"), "--config-file", os.path.relpath(self.yaml_file, self.project_path), ] diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index 22d1ddb22c3..da3be074982 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -42,16 +42,53 @@ class BaseSphinx(BaseBuilder): # an output file, the parsed source files are cached as "doctree pickles". sphinx_doctrees_dir = "_build/doctrees" - # Output directory relative to where the repository was cloned - # (e.g. "_readthedocs/") + # Output directory relative to $READTHEDOCS_OUTPUT + # (e.g. "html", "htmlzip" or "pdf") relative_output_dir = None def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.config_file = self.config.sphinx.configuration - self.absolute_output_dir = os.path.abspath( - os.path.join(self.project_path, self.relative_output_dir) + + # We cannot use `$READTHEDOCS_OUTPUT` environment variable for + # `absolute_host_output_dir` because it's not defined in the host. So, + # we have to re-calculate its value. We will remove this limitation + # when we execute the whole building from inside the Docker container + # (instead behing a hybrid as it is now) + # + # We need to have two different paths that point to the exact same + # directory. How is that? The directory is mounted into a different + # location inside the container: + # + # 1. path in the host: + # /home/docs/checkouts/readthedocs.org/user_builds// + # 2. path in the container: + # /usr/src/app/checkouts/readthedocs.org/user_builds/b9cbc24c8841/test-builds/ + # + # Besides, the variable `$READTHEDOCS_OUTPUT` is not defined in the + # host, so we have to expand it using the full host's path. This + # variable cannot be used in cwd= due to a limitation of the Docker API + # (I guess) since I received an error when trying that. So, we have to + # fully expand it. + # + # That said, we need: + # + # * use the path in the host, for all the operations that are done via + # Python from the app (e.g. os.path.join, glob.glob, etc) + # + # * use the path in the container, for all the operations that are + # executed inside the container via Docker API using shell commands + self.absolute_host_output_dir = os.path.join( + os.path.join( + self.project.checkout_path(self.version.slug), + "_readthedocs/", + ), + self.relative_output_dir, + ) + self.absolute_container_output_dir = os.path.join( + "$READTHEDOCS_OUTPUT", self.relative_output_dir ) + try: if not self.config_file: self.config_file = self.project.conf_file(self.version.slug) @@ -309,9 +346,7 @@ def build(self): # https://github.com/readthedocs/readthedocs.org/pull/9888#issuecomment-1384649346 ".", # Sphinx's output build directory (OUTPUTDIR) - os.path.relpath( - self.absolute_output_dir, os.path.dirname(self.config_file) - ), + self.absolute_container_output_dir, ] ) cmd_ret = self.run( @@ -338,7 +373,7 @@ def sphinx_parallel_arg(self): class HtmlBuilder(BaseSphinx): - relative_output_dir = "_readthedocs/html" + relative_output_dir = "html" def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -367,18 +402,16 @@ def __init__(self, *args, **kwargs): class LocalMediaBuilder(BaseSphinx): sphinx_builder = 'readthedocssinglehtmllocalmedia' - relative_output_dir = "_readthedocs/htmlzip" + relative_output_dir = "htmlzip" def _post_build(self): """Internal post build to create the ZIP file from the HTML output.""" - target_file = os.path.abspath( - os.path.join( - self.absolute_output_dir, - # TODO: shouldn't this name include the name of the version as well? - # It seems we were using the project's slug previously. - # So, keeping it like that for now until we decide make that adjustment. - f"{self.project.slug}.zip", - ) + target_file = os.path.join( + self.absolute_container_output_dir, + # TODO: shouldn't this name include the name of the version as well? + # It seems we were using the project's slug previously. + # So, keeping it like that for now until we decide make that adjustment. + f"{self.project.slug}.zip", ) # **SECURITY CRITICAL: Advisory GHSA-hqwg-gjqw-h5wg** @@ -390,7 +423,7 @@ def _post_build(self): dirname = f"{self.project.slug}-{self.version.slug}" self.run( "mv", - self.relative_output_dir, + self.absolute_container_output_dir, str(tmp_dir / dirname), cwd=self.project_path, record=False, @@ -398,7 +431,7 @@ def _post_build(self): self.run( "mkdir", "--parents", - self.relative_output_dir, + self.absolute_container_output_dir, cwd=self.project_path, record=False, ) @@ -416,17 +449,19 @@ def _post_build(self): class EpubBuilder(BaseSphinx): sphinx_builder = "epub" - relative_output_dir = "_readthedocs/epub" + relative_output_dir = "epub" def _post_build(self): """Internal post build to cleanup EPUB output directory and leave only one .epub file.""" temp_epub_file = f"/tmp/{self.project.slug}-{self.version.slug}.epub" target_file = os.path.join( - self.absolute_output_dir, + self.absolute_container_output_dir, f"{self.project.slug}.epub", ) - epub_sphinx_filepaths = glob(os.path.join(self.absolute_output_dir, "*.epub")) + epub_sphinx_filepaths = glob( + os.path.join(self.absolute_host_output_dir, "*.epub") + ) if epub_sphinx_filepaths: # NOTE: we currently support only one .epub per version epub_filepath = epub_sphinx_filepaths[0] @@ -437,14 +472,14 @@ def _post_build(self): self.run( "rm", "--recursive", - self.relative_output_dir, + self.absolute_container_output_dir, cwd=self.project_path, record=False, ) self.run( "mkdir", "--parents", - self.relative_output_dir, + self.absolute_container_output_dir, cwd=self.project_path, record=False, ) @@ -481,7 +516,7 @@ class PdfBuilder(BaseSphinx): """Builder to generate PDF documentation.""" - relative_output_dir = "_readthedocs/pdf" + relative_output_dir = "pdf" sphinx_builder = "latex" pdf_file_name = None @@ -504,14 +539,12 @@ def build(self): # https://github.com/readthedocs/readthedocs.org/pull/9888#issuecomment-1384649346 ".", # Sphinx's output build directory (OUTPUTDIR) - os.path.relpath( - self.absolute_output_dir, os.path.dirname(self.config_file) - ), + self.absolute_container_output_dir, cwd=os.path.dirname(self.config_file), bin_path=self.python_env.venv_bin(), ) - tex_files = glob(os.path.join(self.absolute_output_dir, "*.tex")) + tex_files = glob(os.path.join(self.absolute_host_output_dir, "*.tex")) if not tex_files: raise BuildUserError("No TeX files were found.") @@ -526,7 +559,7 @@ def _build_latexmk(self, cwd): # https://github.com/sphinx-doc/sphinx/blob/master/sphinx/texinputs/Makefile_t images = [] for extension in ("png", "gif", "jpg", "jpeg"): - images.extend(Path(self.absolute_output_dir).glob(f"*.{extension}")) + images.extend(Path(self.absolute_host_output_dir).glob(f"*.{extension}")) # FIXME: instead of checking by language here, what we want to check if # ``latex_engine`` is ``platex`` @@ -536,13 +569,13 @@ def _build_latexmk(self, cwd): # step. I don't know exactly why but most of the documentation that # I read differentiate this language from the others. I suppose # it's because it mix kanji (Chinese) with its own symbols. - pdfs = Path(self.absolute_output_dir).glob("*.pdf") + pdfs = Path(self.absolute_host_output_dir).glob("*.pdf") for image in itertools.chain(images, pdfs): self.run( 'extractbb', image.name, - cwd=self.absolute_output_dir, + cwd=self.absolute_host_output_dir, record=False, ) @@ -553,7 +586,7 @@ def _build_latexmk(self, cwd): self.run( 'cat', rcfile, - cwd=self.absolute_output_dir, + cwd=self.absolute_host_output_dir, ) if self.build_env.command_class == DockerBuildCommand: @@ -581,7 +614,7 @@ def _build_latexmk(self, cwd): cls=latex_class, cmd=cmd, warn_only=True, - cwd=self.absolute_output_dir, + cwd=self.absolute_host_output_dir, ) self.pdf_file_name = f'{self.project.slug}.pdf' @@ -597,13 +630,19 @@ def _post_build(self): # TODO: merge this with ePUB since it's pretty much the same temp_pdf_file = f"/tmp/{self.project.slug}-{self.version.slug}.pdf" target_file = os.path.join( - self.absolute_output_dir, + self.absolute_container_output_dir, self.pdf_file_name, ) # NOTE: we currently support only one .pdf per version - pdf_sphinx_filepath = os.path.join(self.absolute_output_dir, self.pdf_file_name) - if os.path.exists(pdf_sphinx_filepath): + pdf_sphinx_filepath = os.path.join( + self.absolute_container_output_dir, self.pdf_file_name + ) + pdf_sphinx_filepath_host = os.path.join( + self.absolute_host_output_dir, + self.pdf_file_name, + ) + if os.path.exists(pdf_sphinx_filepath_host): self.run( "mv", pdf_sphinx_filepath, @@ -614,14 +653,14 @@ def _post_build(self): self.run( "rm", "-r", - self.relative_output_dir, + self.absolute_container_output_dir, cwd=self.project_path, record=False, ) self.run( "mkdir", "-p", - self.relative_output_dir, + self.absolute_container_output_dir, cwd=self.project_path, record=False, ) diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 0485169ba05..44f7afb37b5 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -543,6 +543,9 @@ def get_rtd_env_vars(self): "READTHEDOCS_VERSION_NAME": self.data.version.verbose_name, "READTHEDOCS_PROJECT": self.data.project.slug, "READTHEDOCS_LANGUAGE": self.data.project.language, + "READTHEDOCS_OUTPUT": os.path.join( + self.data.project.checkout_path(self.data.version.slug), "_readthedocs/" + ), } return env diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 63198f9d403..e10447b2b16 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -130,8 +130,6 @@ def __str__(self): # commands, which is not supported anymore def run(self): """Set up subprocess and execute command.""" - log.info("Running build command.", command=self.get_command(), cwd=self.cwd) - self.start_time = datetime.utcnow() environment = self._environment.copy() if 'DJANGO_SETTINGS_MODULE' in environment: @@ -145,6 +143,13 @@ def run(self): env_paths.insert(0, self.bin_path) environment['PATH'] = ':'.join(env_paths) + log.info( + "Running build command.", + command=self.get_command(), + cwd=self.cwd, + environment=environment, + ) + try: # When using ``shell=True`` the command should be flatten command = self.command diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 9245f2876fa..19b1f5c5803 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -120,7 +120,7 @@ def test_build_sphinx_formats(self, load_yaml_config, formats, builders): "-D", "language=en", ".", - "../_readthedocs/html", + "$READTHEDOCS_OUTPUT/html", cwd=mock.ANY, bin_path=mock.ANY, ) @@ -141,7 +141,7 @@ def test_build_sphinx_formats(self, load_yaml_config, formats, builders): "-D", "language=en", ".", - "../_readthedocs/html", + "$READTHEDOCS_OUTPUT/html", cwd=mock.ANY, bin_path=mock.ANY, ) @@ -275,6 +275,9 @@ def test_get_env_vars(self, load_yaml_config, build_environment, config, externa "READTHEDOCS_VERSION_NAME": self.version.verbose_name, "READTHEDOCS_PROJECT": self.project.slug, "READTHEDOCS_LANGUAGE": self.project.language, + "READTHEDOCS_OUTPUT": os.path.join( + self.project.checkout_path(self.version.slug), "_readthedocs/" + ), } self._trigger_update_docs_task() @@ -653,7 +656,7 @@ def test_build_commands_executed( "-D", "language=en", ".", - "../_readthedocs/html", + "$READTHEDOCS_OUTPUT/html", cwd=mock.ANY, bin_path=mock.ANY, ), @@ -670,7 +673,7 @@ def test_build_commands_executed( "-D", "language=en", ".", - "../_readthedocs/htmlzip", + "$READTHEDOCS_OUTPUT/htmlzip", cwd=mock.ANY, bin_path=mock.ANY, ), @@ -715,7 +718,7 @@ def test_build_commands_executed( "-D", "language=en", ".", - "../_readthedocs/pdf", + "$READTHEDOCS_OUTPUT/pdf", cwd=mock.ANY, bin_path=mock.ANY, ), @@ -735,7 +738,7 @@ def test_build_commands_executed( "-D", "language=en", ".", - "../_readthedocs/epub", + "$READTHEDOCS_OUTPUT/epub", cwd=mock.ANY, bin_path=mock.ANY, ), @@ -747,12 +750,16 @@ def test_build_commands_executed( record=False, ), mock.call( - "rm", "--recursive", "_readthedocs/epub", cwd=mock.ANY, record=False + "rm", + "--recursive", + "$READTHEDOCS_OUTPUT/epub", + cwd=mock.ANY, + record=False, ), mock.call( "mkdir", "--parents", - "_readthedocs/epub", + "$READTHEDOCS_OUTPUT/epub", cwd=mock.ANY, record=False, ), @@ -1003,7 +1010,7 @@ def test_build_commands(self, load_yaml_config): }, "commands": [ "pip install pelican[markdown]", - "pelican --settings docs/pelicanconf.py --output _readthedocs/html/ docs/", + "pelican --settings docs/pelicanconf.py --output $READTHEDOCS_OUTPUT/html/ docs/", ], }, }, @@ -1048,7 +1055,7 @@ def test_build_commands(self, load_yaml_config): "--settings", "docs/pelicanconf.py", "--output", - "_readthedocs/html/", + "$READTHEDOCS_OUTPUT/html/", "docs/", escape_command=False, cwd=mock.ANY, @@ -1231,7 +1238,7 @@ def test_sphinx_fail_on_warning(self, load_yaml_config): "-D", "language=en", ".", - "../_readthedocs/html", + "$READTHEDOCS_OUTPUT/html", cwd=mock.ANY, bin_path=mock.ANY, ), @@ -1261,7 +1268,7 @@ def test_mkdocs_fail_on_warning(self, load_yaml_config): "build", "--clean", "--site-dir", - "../_readthedocs/html", + "$READTHEDOCS_OUTPUT/html", "--config-file", "docs/mkdocs.yaml", "--strict", # fail on warning flag @@ -1610,7 +1617,7 @@ def test_sphinx_builder(self, load_yaml_config, value, command): "-D", "language=en", ".", - "../_readthedocs/html", + "$READTHEDOCS_OUTPUT/html", cwd=mock.ANY, bin_path=mock.ANY, ),