From 67b01e29f0cac56d694e85c59af30d1bbc4e2543 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 23 Dec 2024 18:09:26 -0500 Subject: [PATCH] Config file: make sphinx or mkdocs configuration required for projects using Sphinx or MkDocs (#11852) - This introduces a breaking change for users overriding the new (undocumented) jobs (create_environment, install, build.html and friends). This is, if they have overridden any of those jobs without explicitly declaring a Sphinx or MkDocs key, we will no longer run the sphinx/mkdocs commands, not their setup. - This hardcodes the dates from https://github.com/readthedocs/website/pull/342 to through an error to users using the configuration file without an explicit sphinx/mkdocs configuration. This allows for users to keep using the new overrides without worrying about sphinx/mkdocs, while giving enough time to old users to migrate their projects to give an explicit path. ### Some notes - We are allowing to use sphinx/mkdocs and probably other keys with build.commands, even if those keys don't affect the build in anything. Not really something that "interrupt" users in any way, but it can be missleading. - A next step should be to not make python required at all, right now we still create a virtual environment. We probably want to create a virtual env only if python was provided in the list of build.tools. ---- - Ref: https://github.com/readthedocs/website/pull/342 - Closes https://github.com/readthedocs/readthedocs.org/issues/10637 - Closes https://github.com/readthedocs/readthedocs.org/issues/11819 - Closes https://github.com/readthedocs/readthedocs.org/pull/11810 - Closes https://github.com/readthedocs/readthedocs.org/issues/11216 --------- Co-authored-by: Manuel Kaufmann --- docs/user/config-file/v2.rst | 13 +- readthedocs/config/config.py | 82 ++++++- readthedocs/config/exceptions.py | 3 + readthedocs/config/notifications.py | 26 +++ readthedocs/config/tests/test_config.py | 61 ++++- readthedocs/doc_builder/director.py | 18 +- .../doc_builder/python_environments.py | 5 + .../projects/tests/test_build_tasks.py | 215 ++++++++++++++++++ .../rtd_tests/fixtures/spec/v2/schema.json | 6 +- 9 files changed, 412 insertions(+), 17 deletions(-) diff --git a/docs/user/config-file/v2.rst b/docs/user/config-file/v2.rst index 48e3b465e42..30ee7bff304 100644 --- a/docs/user/config-file/v2.rst +++ b/docs/user/config-file/v2.rst @@ -493,8 +493,7 @@ The ``$READTHEDOCS_OUTPUT/html`` directory will be uploaded and hosted by Read t sphinx ~~~~~~ -Configuration for Sphinx documentation -(this is the default documentation type). +Configuration for Sphinx documentation. .. code-block:: yaml @@ -535,10 +534,7 @@ sphinx.configuration The path to the ``conf.py`` file, relative to the root of the project. :Type: ``path`` -:Default: ``null`` - -If the value is ``null``, -Read the Docs will try to find a ``conf.py`` file in your project. +:Required: ``true`` sphinx.fail_on_warning `````````````````````` @@ -580,10 +576,7 @@ mkdocs.configuration The path to the ``mkdocs.yml`` file, relative to the root of the project. :Type: ``path`` -:Default: ``null`` - -If the value is ``null``, -Read the Docs will try to find a ``mkdocs.yml`` file in your project. +:Required: ``true`` mkdocs.fail_on_warning `````````````````````` diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index e1cbbd60b33..e3c96d73e0f 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -1,11 +1,12 @@ """Build configuration for rtd.""" - import copy +import datetime import os import re from contextlib import contextmanager from functools import lru_cache +import pytz from django.conf import settings from pydantic import BaseModel @@ -87,7 +88,13 @@ class BuildConfigBase: version = None - def __init__(self, raw_config, source_file, base_path=None): + def __init__( + self, + raw_config, + source_file, + base_path=None, + deprecate_implicit_keys=None, + ): self._raw_config = copy.deepcopy(raw_config) self.source_config = copy.deepcopy(raw_config) self.source_file = source_file @@ -102,6 +109,25 @@ def __init__(self, raw_config, source_file, base_path=None): self._config = {} + if deprecate_implicit_keys is not None: + self.deprecate_implicit_keys = deprecate_implicit_keys + elif settings.RTD_ENFORCE_BROWNOUTS_FOR_DEPRECATIONS: + tzinfo = pytz.timezone("America/Los_Angeles") + now = datetime.datetime.now(tz=tzinfo) + # Dates as per https://about.readthedocs.com/blog/2024/12/deprecate-config-files-without-sphinx-or-mkdocs-config/ + # fmt: off + self.deprecate_implicit_keys = ( + # 12 hours brownout. + datetime.datetime(2025, 1, 6, 0, 0, 0, tzinfo=tzinfo) < now < datetime.datetime(2025, 1, 6, 12, 0, 0, tzinfo=tzinfo) + # 24 hours brownout. + or datetime.datetime(2025, 1, 13, 0, 0, 0, tzinfo=tzinfo) < now < datetime.datetime(2025, 1, 14, 0, 0, 0, tzinfo=tzinfo) + # Permanent removal. + or datetime.datetime(2025, 1, 20, 0, 0, 0, tzinfo=tzinfo) < now + ) + # fmt: on + else: + self.deprecate_implicit_keys = False + @contextmanager def catch_validation_error(self, key): """Catch a ``ConfigValidationError`` and raises a ``ConfigError`` error.""" @@ -219,7 +245,6 @@ def __getattr__(self, name): class BuildConfigV2(BuildConfigBase): - """Version 2 of the configuration file.""" version = "2" @@ -251,6 +276,8 @@ def validate(self): self._config["sphinx"] = self.validate_sphinx() self._config["submodules"] = self.validate_submodules() self._config["search"] = self.validate_search() + if self.deprecate_implicit_keys: + self.validate_deprecated_implicit_keys() self.validate_keys() def validate_formats(self): @@ -722,6 +749,50 @@ def validate_search(self): return search + def validate_deprecated_implicit_keys(self): + """ + Check for deprecated usages and raise an exception if found. + + - If the sphinx key is used, a path to the configuration file is required. + - If the mkdocs key is used, a path to the configuration file is required. + - If none of the sphinx or mkdocs keys are used, + and the user isn't overriding the new build jobs, + the sphinx key is explicitly required. + """ + has_sphinx_key = "sphinx" in self.source_config + has_mkdocs_key = "mkdocs" in self.source_config + if has_sphinx_key and not self.sphinx.configuration: + raise ConfigError( + message_id=ConfigError.SPHINX_CONFIG_MISSING, + ) + + if has_mkdocs_key and not self.mkdocs.configuration: + raise ConfigError( + message_id=ConfigError.MKDOCS_CONFIG_MISSING, + ) + + if not self.new_jobs_overriden and not has_sphinx_key and not has_mkdocs_key: + raise ConfigError( + message_id=ConfigError.SPHINX_CONFIG_MISSING, + ) + + @property + def new_jobs_overriden(self): + """Check if any of the new (undocumented) build jobs are overridden.""" + build_jobs = self.build.jobs + new_jobs = ( + build_jobs.create_environment, + build_jobs.install, + build_jobs.build.html, + build_jobs.build.pdf, + build_jobs.build.epub, + build_jobs.build.htmlzip, + ) + for job in new_jobs: + if job is not None: + return True + return False + def validate_keys(self): """ Checks that we don't have extra keys (invalid ones). @@ -812,6 +883,11 @@ def doctype(self): if "commands" in self._config["build"] and self._config["build"]["commands"]: return GENERIC + has_sphinx_key = "sphinx" in self.source_config + has_mkdocs_key = "mkdocs" in self.source_config + if self.new_jobs_overriden and not has_sphinx_key and not has_mkdocs_key: + return GENERIC + if self.mkdocs: return "mkdocs" return self.sphinx.builder diff --git a/readthedocs/config/exceptions.py b/readthedocs/config/exceptions.py index d0fcb30b5f1..fe7c2f36580 100644 --- a/readthedocs/config/exceptions.py +++ b/readthedocs/config/exceptions.py @@ -26,6 +26,9 @@ class ConfigError(BuildUserError): SYNTAX_INVALID = "config:base:invalid-syntax" CONDA_KEY_REQUIRED = "config:conda:required" + SPHINX_CONFIG_MISSING = "config:sphinx:missing-config" + MKDOCS_CONFIG_MISSING = "config:mkdocs:missing-config" + # TODO: improve these error messages shown to the user # See https://github.com/readthedocs/readthedocs.org/issues/10502 diff --git a/readthedocs/config/notifications.py b/readthedocs/config/notifications.py index 954c3bd5f19..bf9c9f37876 100644 --- a/readthedocs/config/notifications.py +++ b/readthedocs/config/notifications.py @@ -361,5 +361,31 @@ ), type=ERROR, ), + Message( + id=ConfigError.SPHINX_CONFIG_MISSING, + header=_("Missing Sphinx configuration key"), + body=_( + textwrap.dedent( + """ + The sphinx.configuration key is missing. + This key is now required, see our blog post for more information. + """ + ).strip(), + ), + type=ERROR, + ), + Message( + id=ConfigError.MKDOCS_CONFIG_MISSING, + header=_("Missing MkDocs configuration key"), + body=_( + textwrap.dedent( + """ + The mkdocs.configuration key is missing. + This key is now required, see our blog post for more information. + """ + ).strip(), + ), + type=ERROR, + ), ] registry.add(messages) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index e61d76a5c7a..aca038771d3 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -23,7 +23,7 @@ from .utils import apply_fs -def get_build_config(config, source_file="readthedocs.yml", validate=False): +def get_build_config(config, source_file="readthedocs.yml", validate=False, **kwargs): # I'm adding these defaults here to avoid modifying all the config file from all the tests final_config = { "version": "2", @@ -39,6 +39,7 @@ def get_build_config(config, source_file="readthedocs.yml", validate=False): build_config = BuildConfigV2( final_config, source_file=source_file, + **kwargs, ) if validate: build_config.validate() @@ -1805,6 +1806,64 @@ def test_pop_config_raise_exception(self): assert excinfo.value.format_values.get("value") == "invalid" assert excinfo.value.message_id == ConfigValidationError.VALUE_NOT_FOUND + def test_sphinx_without_explicit_configuration(self): + data = { + "sphinx": {}, + } + get_build_config(data, validate=True) + + with raises(ConfigError) as excinfo: + get_build_config(data, validate=True, deprecate_implicit_keys=True) + + assert excinfo.value.message_id == ConfigError.SPHINX_CONFIG_MISSING + + data["sphinx"]["configuration"] = "conf.py" + get_build_config(data, validate=True, deprecate_implicit_keys=True) + + def test_mkdocs_without_explicit_configuration(self): + data = { + "mkdocs": {}, + } + get_build_config(data, validate=True) + + with raises(ConfigError) as excinfo: + get_build_config(data, validate=True, deprecate_implicit_keys=True) + + assert excinfo.value.message_id == ConfigError.MKDOCS_CONFIG_MISSING + + data["mkdocs"]["configuration"] = "mkdocs.yml" + get_build_config(data, validate=True, deprecate_implicit_keys=True) + + def test_config_without_sphinx_key(self): + data = { + "build": { + "os": "ubuntu-22.04", + "tools": { + "python": "3", + }, + "jobs": {}, + }, + } + get_build_config(data, validate=True) + + with raises(ConfigError) as excinfo: + get_build_config(data, validate=True, deprecate_implicit_keys=True) + + assert excinfo.value.message_id == ConfigError.SPHINX_CONFIG_MISSING + + # No exception should be raised when overriding any of the the new jobs. + data_copy = data.copy() + data_copy["build"]["jobs"]["create_environment"] = ["echo 'Hello World'"] + get_build_config(data_copy, validate=True, deprecate_implicit_keys=True) + + data_copy = data.copy() + data_copy["build"]["jobs"]["install"] = ["echo 'Hello World'"] + get_build_config(data_copy, validate=True, deprecate_implicit_keys=True) + + data_copy = data.copy() + data_copy["build"]["jobs"]["build"] = {"html": ["echo 'Hello World'"]} + get_build_config(data_copy, validate=True, deprecate_implicit_keys=True) + def test_as_dict_new_build_config(self, tmpdir): build = get_build_config( { diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 0418a893a64..51b61fe6cf7 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -23,7 +23,7 @@ from readthedocs.doc_builder.exceptions import BuildUserError from readthedocs.doc_builder.loader import get_builder_class from readthedocs.doc_builder.python_environments import Conda, Virtualenv -from readthedocs.projects.constants import BUILD_COMMANDS_OUTPUT_PATH_HTML +from readthedocs.projects.constants import BUILD_COMMANDS_OUTPUT_PATH_HTML, GENERIC from readthedocs.projects.exceptions import RepositoryError from readthedocs.projects.signals import after_build, before_build, before_vcs from readthedocs.storage import build_tools_storage @@ -301,6 +301,12 @@ def create_environment(self): if self.data.config.build.jobs.create_environment is not None: self.run_build_job("create_environment") return + + # If the builder is generic, we have nothing to do here, + # as the commnads are provided by the user. + if self.data.config.doctype == GENERIC: + return + self.language_environment.setup_base() # Install @@ -309,6 +315,11 @@ def install(self): self.run_build_job("install") return + # If the builder is generic, we have nothing to do here, + # as the commnads are provided by the user. + if self.data.config.doctype == GENERIC: + return + self.language_environment.install_core_requirements() self.language_environment.install_requirements() @@ -642,6 +653,11 @@ def build_docs_class(self, builder_class): only raise a warning exception here. A hard error will halt the build process. """ + # If the builder is generic, we have nothing to do here, + # as the commnads are provided by the user. + if builder_class == GENERIC: + return + builder = get_builder_class(builder_class)( build_env=self.build_environment, python_env=self.language_environment, diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index e92691c2d83..a70583d4705 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -11,6 +11,7 @@ from readthedocs.config.models import PythonInstall, PythonInstallRequirements from readthedocs.core.utils.filesystem import safe_open from readthedocs.doc_builder.config import load_yaml_config +from readthedocs.projects.constants import GENERIC from readthedocs.projects.exceptions import UserFileNotFound from readthedocs.projects.models import Feature @@ -168,6 +169,10 @@ def _install_latest_requirements(self, pip_install_cmd): cwd=self.checkout_path, ) + # Nothing else to install for generic projects. + if self.config.doctype == GENERIC: + return + # Second, install all the latest core requirements requirements = [] diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 32515c1a8aa..1dad403d2c2 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1290,6 +1290,221 @@ def test_build_jobs_partial_build_override(self, load_yaml_config): ] ) + @mock.patch("readthedocs.doc_builder.director.load_yaml_config") + def test_build_jobs_partial_build_override_without_sphinx(self, load_yaml_config): + config = BuildConfigV2( + { + "version": 2, + "formats": ["pdf", "epub", "htmlzip"], + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3.12"}, + "jobs": { + "build": { + "html": ["echo build html"], + }, + "post_build": ["echo end of build"], + }, + }, + }, + source_file="readthedocs.yml", + ) + config.validate() + load_yaml_config.return_value = config + self._trigger_update_docs_task() + + python_version = settings.RTD_DOCKER_BUILD_SETTINGS["tools"]["python"]["3.12"] + self.mocker.mocks["environment.run"].assert_has_calls( + [ + mock.call("asdf", "install", "python", python_version), + mock.call("asdf", "global", "python", python_version), + mock.call("asdf", "reshim", "python", record=False), + mock.call( + "python", + "-mpip", + "install", + "-U", + "virtualenv", + "setuptools", + ), + mock.call( + "echo build html", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo end of build", + escape_command=False, + cwd=mock.ANY, + ), + ] + ) + + @mock.patch("readthedocs.doc_builder.director.load_yaml_config") + def test_build_jobs_partial_build_override_sphinx(self, load_yaml_config): + config = BuildConfigV2( + { + "version": 2, + "sphinx": { + "configuration": "docs/conf.py", + }, + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3.12"}, + "jobs": { + "build": { + "html": ["echo build html"], + }, + "post_build": ["echo end of build"], + }, + }, + }, + source_file="readthedocs.yml", + ) + config.validate() + load_yaml_config.return_value = config + self._trigger_update_docs_task() + + python_version = settings.RTD_DOCKER_BUILD_SETTINGS["tools"]["python"]["3.12"] + self.mocker.mocks["environment.run"].assert_has_calls( + [ + mock.call("asdf", "install", "python", python_version), + mock.call("asdf", "global", "python", python_version), + mock.call("asdf", "reshim", "python", record=False), + mock.call( + "python", + "-mpip", + "install", + "-U", + "virtualenv", + "setuptools", + ), + mock.call( + "python", + "-mvirtualenv", + "$READTHEDOCS_VIRTUALENV_PATH", + bin_path=None, + cwd=None, + ), + mock.call( + mock.ANY, + "-m", + "pip", + "install", + "--upgrade", + "--no-cache-dir", + "pip", + "setuptools", + bin_path=mock.ANY, + cwd=mock.ANY, + ), + mock.call( + mock.ANY, + "-m", + "pip", + "install", + "--upgrade", + "--no-cache-dir", + "sphinx", + bin_path=mock.ANY, + cwd=mock.ANY, + ), + mock.call( + "echo build html", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo end of build", + escape_command=False, + cwd=mock.ANY, + ), + ] + ) + + @mock.patch("readthedocs.doc_builder.director.load_yaml_config") + def test_build_jobs_partial_build_override_mkdocs(self, load_yaml_config): + config = BuildConfigV2( + { + "version": 2, + "formats": ["pdf", "epub", "htmlzip"], + "mkdocs": { + "configuration": "mkdocs.yml", + }, + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3.12"}, + "jobs": { + "build": { + "html": ["echo build html"], + }, + "post_build": ["echo end of build"], + }, + }, + }, + source_file="readthedocs.yml", + ) + config.validate() + load_yaml_config.return_value = config + self._trigger_update_docs_task() + + python_version = settings.RTD_DOCKER_BUILD_SETTINGS["tools"]["python"]["3.12"] + self.mocker.mocks["environment.run"].assert_has_calls( + [ + mock.call("asdf", "install", "python", python_version), + mock.call("asdf", "global", "python", python_version), + mock.call("asdf", "reshim", "python", record=False), + mock.call( + "python", + "-mpip", + "install", + "-U", + "virtualenv", + "setuptools", + ), + mock.call( + "python", + "-mvirtualenv", + "$READTHEDOCS_VIRTUALENV_PATH", + bin_path=None, + cwd=None, + ), + mock.call( + mock.ANY, + "-m", + "pip", + "install", + "--upgrade", + "--no-cache-dir", + "pip", + "setuptools", + bin_path=mock.ANY, + cwd=mock.ANY, + ), + mock.call( + mock.ANY, + "-m", + "pip", + "install", + "--upgrade", + "--no-cache-dir", + "mkdocs", + bin_path=mock.ANY, + cwd=mock.ANY, + ), + mock.call( + "echo build html", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo end of build", + escape_command=False, + cwd=mock.ANY, + ), + ] + ) + @mock.patch("readthedocs.doc_builder.director.load_yaml_config") def test_build_jobs_partial_build_override_empty_commands(self, load_yaml_config): config = BuildConfigV2( diff --git a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json index 716b04e55ef..f11436c0ca6 100644 --- a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json +++ b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json @@ -337,7 +337,8 @@ "default": false } }, - "additionalProperties": false + "additionalProperties": false, + "required": ["configuration"] }, "mkdocs": { "title": "mkdocs", @@ -356,7 +357,8 @@ "default": false } }, - "additionalProperties": false + "additionalProperties": false, + "required": ["configuration"] }, "submodules": { "title": "Submodules",