From 3b947be7e3603f4fbac15b6af9d1d441fa41db1d Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Wed, 23 Oct 2024 16:19:28 -0500 Subject: [PATCH 01/16] Build: allow partial override of build steps --- readthedocs/config/config.py | 60 +++++++++++++++++++++++++---- readthedocs/config/exceptions.py | 2 + readthedocs/config/models.py | 53 ++++++++++++++----------- readthedocs/config/notifications.py | 24 ++++++++++++ readthedocs/core/utils/objects.py | 28 ++++++++++++++ readthedocs/doc_builder/director.py | 38 ++++++++++++------ requirements/deploy.txt | 12 ++++++ requirements/docker.txt | 12 ++++++ requirements/pip.in | 1 + requirements/pip.txt | 8 ++++ requirements/testing.txt | 12 ++++++ 11 files changed, 208 insertions(+), 42 deletions(-) create mode 100644 readthedocs/core/utils/objects.py diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 2452621e647..262313c73b0 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -16,6 +16,7 @@ from .find import find_one from .models import ( BuildJobs, + BuildJobsBuildTypes, BuildTool, BuildWithOs, Conda, @@ -101,6 +102,9 @@ def __init__(self, raw_config, source_file, base_path=None): self._config = {} + self.is_using_build_commands = False + self.is_using_build_jobs = False + @contextmanager def catch_validation_error(self, key): """Catch a ``ConfigValidationError`` and raises a ``ConfigError`` error.""" @@ -175,10 +179,6 @@ def validate(self): def is_using_conda(self): return self.python_interpreter in ("conda", "mamba") - @property - def is_using_build_commands(self): - return self.build.commands != [] - @property def is_using_setup_py_install(self): """Check if this project is using `setup.py install` as installation method.""" @@ -250,6 +250,7 @@ def validate(self): self._config["sphinx"] = self.validate_sphinx() self._config["submodules"] = self.validate_submodules() self._config["search"] = self.validate_search() + self.validate_incompatible_keys() self.validate_keys() def validate_formats(self): @@ -318,11 +319,9 @@ def validate_build_config_with_os(self): # ones, we could validate the value of each of them is a list of # commands. However, I don't think we should validate the "command" # looks like a real command. + valid_jobs = list(BuildJobs.model_fields.keys()) for job in jobs.keys(): - validate_choice( - job, - BuildJobs.__slots__, - ) + validate_choice(job, valid_jobs) commands = [] with self.catch_validation_error("build.commands"): @@ -345,7 +344,20 @@ def validate_build_config_with_os(self): }, ) + if commands: + self.is_using_build_commands = True + else: + self.is_using_build_jobs = True + build["jobs"] = {} + + with self.catch_validation_error("build.jobs.build"): + build["jobs"]["build"] = self.validate_build_jobs_build(jobs) + # Remove the build.jobs.build key from the build.jobs dict, + # since it's the only key that should be a dictionary, + # it was already validated above. + jobs.pop("build", None) + for job, job_commands in jobs.items(): with self.catch_validation_error(f"build.jobs.{job}"): build["jobs"][job] = [ @@ -370,6 +382,29 @@ def validate_build_config_with_os(self): build["apt_packages"] = self.validate_apt_packages() return build + def validate_build_jobs_build(self, build_jobs): + # The build.jobs.build key is optional. + if "build" not in build_jobs: + return None + + result = {} + build_jobs_build = build_jobs["build"] + validate_dict(build_jobs_build) + + if not "html" in build_jobs_build: + raise ConfigError(message_id=ConfigError.HTML_BUILD_STEP_REQUIRED) + + allowed_build_types = list(BuildJobsBuildTypes.model_fields.keys()) + for build_type, build_commands in build_jobs_build.items(): + validate_choice(build_type, allowed_build_types) + with self.catch_validation_error(f"build.jobs.build.{build_type}"): + result[build_type] = [ + validate_string(build_command) + for build_command in validate_list(build_commands) + ] + + return result + def validate_apt_packages(self): apt_packages = [] with self.catch_validation_error("build.apt_packages"): @@ -692,6 +727,15 @@ def validate_search(self): return search + def validate_incompatible_keys(self): + # `formats` and `build.jobs.build.*` can't be used together. + build_overridden = ( + self.is_using_build_jobs and self.build.jobs.build is not None + ) + with self.catch_validation_error("formats"): + if build_overridden and "formats" in self.source_config: + raise ConfigError(message_id=ConfigError.FORMATS_AND_BUILD_JOBS_BUILD) + def validate_keys(self): """ Checks that we don't have extra keys (invalid ones). diff --git a/readthedocs/config/exceptions.py b/readthedocs/config/exceptions.py index 637ffd71cf7..2cde29b4f67 100644 --- a/readthedocs/config/exceptions.py +++ b/readthedocs/config/exceptions.py @@ -13,6 +13,8 @@ class ConfigError(BuildUserError): INVALID_VERSION = "config:base:invalid-version" NOT_BUILD_TOOLS_OR_COMMANDS = "config:build:missing-build-tools-commands" BUILD_JOBS_AND_COMMANDS = "config:build:jobs-and-commands" + FORMATS_AND_BUILD_JOBS_BUILD = "config:formats:formats-and-build" + HTML_BUILD_STEP_REQUIRED = "config:build:jobs:build:html-build-step-required" APT_INVALID_PACKAGE_NAME_PREFIX = "config:apt:invalid-package-name-prefix" APT_INVALID_PACKAGE_NAME = "config:apt:invalid-package-name" USE_PIP_FOR_EXTRA_REQUIREMENTS = "config:python:pip-required" diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index 16b0ad58750..88e82c9d41b 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -1,4 +1,5 @@ """Models for the response of the configuration object.""" +from pydantic import BaseModel from readthedocs.config.utils import to_dict @@ -37,33 +38,39 @@ class BuildTool(Base): __slots__ = ("version", "full_version") -class BuildJobs(Base): +class BuildJobsBuildTypes(BaseModel): + + """Object used for `build.jobs.build` key.""" + + html: list[str] + pdf: list[str] = [] + + def as_dict(self): + # Just to keep compatibility with the old implementation. + return self.model_dump() + + +class BuildJobs(BaseModel): """Object used for `build.jobs` key.""" - __slots__ = ( - "pre_checkout", - "post_checkout", - "pre_system_dependencies", - "post_system_dependencies", - "pre_create_environment", - "post_create_environment", - "pre_install", - "post_install", - "pre_build", - "post_build", - ) + pre_checkout: list[str] = [] + post_checkout: list[str] = [] + pre_system_dependencies: list[str] = [] + post_system_dependencies: list[str] = [] + pre_create_environment: list[str] = [] + create_environment: list[str] | None = None + post_create_environment: list[str] = [] + pre_install: list[str] = [] + install: list[str] | None = None + post_install: list[str] = [] + pre_build: list[str] = [] + build: BuildJobsBuildTypes | None = None + post_build: list[str] = [] - def __init__(self, **kwargs): - """ - Create an empty list as a default for all possible builds.jobs configs. - - This is necessary because it makes the code cleaner when we add items to these lists, - without having to check for a dict to be created first. - """ - for step in self.__slots__: - kwargs.setdefault(step, []) - super().__init__(**kwargs) + def as_dict(self): + # Just to keep compatibility with the old implementation. + return self.model_dump() class Python(Base): diff --git a/readthedocs/config/notifications.py b/readthedocs/config/notifications.py index ccccebd5c33..d8c73399099 100644 --- a/readthedocs/config/notifications.py +++ b/readthedocs/config/notifications.py @@ -124,6 +124,30 @@ ), type=ERROR, ), + Message( + id=ConfigError.FORMATS_AND_BUILD_JOBS_BUILD, + header=_("Invalid configuration option"), + body=_( + textwrap.dedent( + """ + The keys <code>build.jobs.build</code> and <code>formats</code> can't be used together. + """ + ).strip(), + ), + type=ERROR, + ), + Message( + id=ConfigError.HTML_BUILD_STEP_REQUIRED, + header=_("Missing configuration option"), + body=_( + textwrap.dedent( + """ + The key <code>build.jobs.build.html</code> is required when using <code>build.jobs.build</code>. + """ + ).strip(), + ), + type=ERROR, + ), Message( id=ConfigError.APT_INVALID_PACKAGE_NAME_PREFIX, header=_("Invalid APT package name"), diff --git a/readthedocs/core/utils/objects.py b/readthedocs/core/utils/objects.py new file mode 100644 index 00000000000..2d690ef100e --- /dev/null +++ b/readthedocs/core/utils/objects.py @@ -0,0 +1,28 @@ +_DEFAULT = object() + + +def get_dotted_attribute(obj, attribute, default=_DEFAULT): + """ + Allow to get nested attributes from an object using a dot notation. + + This behaves similar to getattr, but allows to get nested attributes. + Similar, if a default value is provided, it will be returned if the + attribute is not found, otherwise it will raise an AttributeError. + """ + for attr in attribute.split("."): + if hasattr(obj, attr): + obj = getattr(obj, attr) + elif default is not _DEFAULT: + return default + else: + raise AttributeError(f"Object {obj} has no attribute {attr}") + return obj + + +def has_dotted_attribute(obj, attribute): + """Check if an object has a nested attribute using a dot notation.""" + try: + get_dotted_attribute(obj, attribute) + return True + except AttributeError: + return False diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 8f49c4074cf..63ecf39a6d9 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -18,6 +18,7 @@ from readthedocs.config.config import CONFIG_FILENAME_REGEX from readthedocs.config.find import find_one from readthedocs.core.utils.filesystem import safe_open +from readthedocs.core.utils.objects import get_dotted_attribute from readthedocs.doc_builder.config import load_yaml_config from readthedocs.doc_builder.exceptions import BuildUserError from readthedocs.doc_builder.loader import get_builder_class @@ -159,6 +160,7 @@ def setup_environment(self): sender=self.data.version, environment=self.build_environment, ) + config = self.data.config self.run_build_job("pre_system_dependencies") self.system_dependencies() @@ -168,11 +170,17 @@ def setup_environment(self): self.install_build_tools() self.run_build_job("pre_create_environment") - self.create_environment() + if config.build.jobs.create_environment is not None: + self.run_build_job("create_environment") + else: + self.create_environment() self.run_build_job("post_create_environment") self.run_build_job("pre_install") - self.install() + if self.data.config.build.jobs.install is not None: + self.run_build_job("install") + else: + self.install() self.run_build_job("post_install") def build(self): @@ -184,14 +192,20 @@ def build(self): 3. build PDF 4. build ePub """ + config = self.data.config self.run_build_job("pre_build") # Build all formats - self.build_html() - self.build_htmlzip() - self.build_pdf() - self.build_epub() + build_overridden = config.build.jobs.build is not None + if build_overridden: + self.run_build_job("build.html") + self.run_build_job("build.pdf") + else: + self.build_html() + self.build_htmlzip() + self.build_pdf() + self.build_epub() self.run_build_job("post_build") self.store_readthedocs_build_yaml() @@ -372,14 +386,17 @@ def run_build_job(self, job): - python path/to/myscript.py pre_build: - sed -i **/*.rst -e "s|{version}|v3.5.1|g" + build: + html: + - make html + pdf: + - make pdf In this case, `self.data.config.build.jobs.pre_build` will contains `sed` command. """ - if ( - getattr(self.data.config.build, "jobs", None) is None - or getattr(self.data.config.build.jobs, job, None) is None - ): + commands = get_dotted_attribute(self.data.config, f"build.jobs.{job}", []) + if not commands: return cwd = self.data.project.checkout_path(self.data.version.slug) @@ -387,7 +404,6 @@ def run_build_job(self, job): if job not in ("pre_checkout", "post_checkout"): environment = self.build_environment - commands = getattr(self.data.config.build.jobs, job, []) for command in commands: environment.run(command, escape_command=False, cwd=cwd) diff --git a/requirements/deploy.txt b/requirements/deploy.txt index a6e7b361f1e..20e36c1d1bb 100644 --- a/requirements/deploy.txt +++ b/requirements/deploy.txt @@ -8,6 +8,10 @@ amqp==5.2.0 # via # -r requirements/pip.txt # kombu +annotated-types==0.7.0 + # via + # -r requirements/pip.txt + # pydantic asgiref==3.8.1 # via # -r requirements/pip.txt @@ -309,6 +313,12 @@ pycparser==2.22 # via # -r requirements/pip.txt # cffi +pydantic==2.9.2 + # via -r requirements/pip.txt +pydantic-core==2.23.4 + # via + # -r requirements/pip.txt + # pydantic pygments==2.18.0 # via # -r requirements/pip.txt @@ -423,6 +433,8 @@ typing-extensions==4.12.2 # ipython # psycopg # psycopg-pool + # pydantic + # pydantic-core tzdata==2024.2 # via # -r requirements/pip.txt diff --git a/requirements/docker.txt b/requirements/docker.txt index e7cfb4aa114..02a04d384cd 100644 --- a/requirements/docker.txt +++ b/requirements/docker.txt @@ -8,6 +8,10 @@ amqp==5.2.0 # via # -r requirements/pip.txt # kombu +annotated-types==0.7.0 + # via + # -r requirements/pip.txt + # pydantic asgiref==3.8.1 # via # -r requirements/pip.txt @@ -333,6 +337,12 @@ pycparser==2.22 # via # -r requirements/pip.txt # cffi +pydantic==2.9.2 + # via -r requirements/pip.txt +pydantic-core==2.23.4 + # via + # -r requirements/pip.txt + # pydantic pygments==2.18.0 # via # -r requirements/pip.txt @@ -455,6 +465,8 @@ typing-extensions==4.12.2 # ipython # psycopg # psycopg-pool + # pydantic + # pydantic-core # rich # tox tzdata==2024.2 diff --git a/requirements/pip.in b/requirements/pip.in index 651b8322593..25476758bae 100644 --- a/requirements/pip.in +++ b/requirements/pip.in @@ -45,6 +45,7 @@ requests-toolbelt slumber pyyaml Pygments +pydantic # Used for Redis cache Django backend (`django.core.cache.backends.redis.RedisCache`) redis diff --git a/requirements/pip.txt b/requirements/pip.txt index 6ca528a3e1c..2b714cf7a43 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -6,6 +6,8 @@ # amqp==5.2.0 # via kombu +annotated-types==0.7.0 + # via pydantic asgiref==3.8.1 # via # django @@ -218,6 +220,10 @@ psycopg-pool==3.2.3 # via psycopg pycparser==2.22 # via cffi +pydantic==2.9.2 + # via -r requirements/pip.in +pydantic-core==2.23.4 + # via pydantic pygments==2.18.0 # via -r requirements/pip.in pyjwt[crypto]==2.9.0 @@ -300,6 +306,8 @@ typing-extensions==4.12.2 # elasticsearch-dsl # psycopg # psycopg-pool + # pydantic + # pydantic-core tzdata==2024.2 # via # -r requirements/pip.in diff --git a/requirements/testing.txt b/requirements/testing.txt index 9f1c37d0cab..a5dde50f338 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -10,6 +10,10 @@ amqp==5.2.0 # via # -r requirements/pip.txt # kombu +annotated-types==0.7.0 + # via + # -r requirements/pip.txt + # pydantic asgiref==3.8.1 # via # -r requirements/pip.txt @@ -311,6 +315,12 @@ pycparser==2.22 # via # -r requirements/pip.txt # cffi +pydantic==2.9.2 + # via -r requirements/pip.txt +pydantic-core==2.23.4 + # via + # -r requirements/pip.txt + # pydantic pygments==2.18.0 # via # -r requirements/pip.txt @@ -453,6 +463,8 @@ typing-extensions==4.12.2 # elasticsearch-dsl # psycopg # psycopg-pool + # pydantic + # pydantic-core # sphinx tzdata==2024.2 # via From 004a3aacfb016b80bb9341c8e700a1b177264135 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 13:15:32 -0500 Subject: [PATCH 02/16] Fix tests --- readthedocs/config/tests/test_config.py | 3 +++ readthedocs/projects/tests/test_build_tasks.py | 15 +++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index c72e2c256b6..31796c405bf 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1757,10 +1757,13 @@ def test_as_dict_new_build_config(self, tmpdir): "pre_system_dependencies": [], "post_system_dependencies": [], "pre_create_environment": [], + "create_environment": None, "post_create_environment": [], "pre_install": [], + "install": None, "post_install": [], "pre_build": [], + "build": None, "post_build": [], }, "apt_packages": [], diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 916afb42cdd..9d62e151ac0 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -559,16 +559,19 @@ def test_successful_build( "os": "ubuntu-22.04", "commands": [], "jobs": { - "post_build": [], + "pre_checkout": [], "post_checkout": [], - "post_create_environment": [], - "post_install": [], + "pre_system_dependencies": [], "post_system_dependencies": [], - "pre_build": [], - "pre_checkout": [], "pre_create_environment": [], + "create_environment": None, + "post_create_environment": [], "pre_install": [], - "pre_system_dependencies": [], + "install": None, + "post_install": [], + "pre_build": [], + "build": None, + "post_build": [], }, "tools": { "python": { From 3a61248fcfb27d9003bb5619c296c236265c1f58 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 13:15:40 -0500 Subject: [PATCH 03/16] Update schema --- .../rtd_tests/fixtures/spec/v2/schema.json | 126 ++++++++---------- 1 file changed, 54 insertions(+), 72 deletions(-) diff --git a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json index a5f28d53997..6e44fa54ba1 100644 --- a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json +++ b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json @@ -9,9 +9,7 @@ "title": "Version", "description": "The version of the spec to use.", "type": "number", - "enum": [ - 2 - ] + "enum": [2] }, "formats": { "title": "Formats", @@ -20,17 +18,11 @@ { "type": "array", "items": { - "enum": [ - "htmlzip", - "pdf", - "epub" - ] + "enum": ["htmlzip", "pdf", "epub"] } }, { - "enum": [ - "all" - ] + "enum": ["all"] } ], "default": [] @@ -46,9 +38,7 @@ "type": "string" } }, - "required": [ - "environment" - ] + "required": ["environment"] }, "build": { "title": "Build", @@ -98,6 +88,14 @@ "type": "string" } }, + "create_environment": { + "description": "Override the default environment creation process.", + "type": "array", + "items": { + "title": "Custom commands", + "type": "string" + } + }, "post_create_environment": { "type": "array", "items": { @@ -112,6 +110,14 @@ "type": "string" } }, + "install": { + "description": "Override the default installation process.", + "type": "array", + "items": { + "title": "Custom commands", + "type": "string" + } + }, "post_install": { "type": "array", "items": { @@ -126,6 +132,28 @@ "type": "string" } }, + "build": { + "description": "Override the default build process.", + "type": "object", + "additionalProperties": false, + "required": ["html"], + "properties": { + "html": { + "type": "array", + "items": { + "title": "Custom commands", + "type": "string" + } + }, + "pdf": { + "type": "array", + "items": { + "title": "Custom commands", + "type": "string" + } + } + } + }, "post_build": { "type": "array", "items": { @@ -163,42 +191,16 @@ ] }, "nodejs": { - "enum": [ - "14", - "16", - "18", - "19", - "20", - "latest" - ] + "enum": ["14", "16", "18", "19", "20", "latest"] }, "ruby": { - "enum": [ - "3.3", - "latest" - ] + "enum": ["3.3", "latest"] }, "rust": { - "enum": [ - "1.55", - "1.61", - "1.64", - "1.70", - "1.75", - "1.78", - "latest" - ] + "enum": ["1.55", "1.61", "1.64", "1.70", "1.75", "1.78", "latest"] }, "golang": { - "enum": [ - "1.17", - "1.18", - "1.19", - "1.20", - "1.21", - "1.22", - "latest" - ] + "enum": ["1.17", "1.18", "1.19", "1.20", "1.21", "1.22", "latest"] } }, "minProperties": 1, @@ -224,10 +226,7 @@ } } }, - "required": [ - "os", - "tools" - ], + "required": ["os", "tools"], "additionalProperties": false }, "python": { @@ -249,9 +248,7 @@ "type": "string" } }, - "required": [ - "requirements" - ] + "required": ["requirements"] }, { "properties": { @@ -263,10 +260,7 @@ "method": { "title": "Method", "description": "Install using python setup.py install or pip.", - "enum": [ - "pip", - "setuptools" - ], + "enum": ["pip", "setuptools"], "default": "pip" }, "extra_requirements": { @@ -279,9 +273,7 @@ "default": [] } }, - "required": [ - "path" - ] + "required": ["path"] } ] } @@ -297,11 +289,7 @@ "builder": { "title": "Builder", "description": "The builder type for the sphinx documentation.", - "enum": [ - "html", - "dirhtml", - "singlehtml" - ], + "enum": ["html", "dirhtml", "singlehtml"], "default": "html" }, "configuration": { @@ -353,9 +341,7 @@ } }, { - "enum": [ - "all" - ] + "enum": ["all"] } ], "default": [] @@ -371,9 +357,7 @@ } }, { - "enum": [ - "all" - ] + "enum": ["all"] } ], "default": [] @@ -419,8 +403,6 @@ "additionalProperties": false } }, - "required": [ - "version" - ], + "required": ["version"], "additionalProperties": false } From 6228aebe483324b78dbdc2ad69214a3d43aeaa7d Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 13:45:56 -0500 Subject: [PATCH 04/16] Tests --- readthedocs/config/tests/test_config.py | 85 +++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 31796c405bf..e8a12f6eae8 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -627,17 +627,102 @@ def test_jobs_build_config(self): assert build.build.jobs.pre_create_environment == [ "echo pre_create_environment" ] + assert build.build.jobs.create_environment is None assert build.build.jobs.post_create_environment == [ "echo post_create_environment" ] assert build.build.jobs.pre_install == ["echo pre_install", "echo `date`"] + assert build.build.jobs.install is None assert build.build.jobs.post_install == ["echo post_install"] assert build.build.jobs.pre_build == [ "echo pre_build", 'sed -i -e "s|{VERSION}|${READTHEDOCS_VERSION_NAME}|g"', ] + assert build.build.jobs.build is None assert build.build.jobs.post_build == ["echo post_build"] + def test_build_jobs_partial_override(self): + build = get_build_config( + { + "build": { + "os": "ubuntu-20.04", + "tools": {"python": "3"}, + "jobs": { + "create_environment": ["echo make_environment"], + "install": ["echo install"], + "build": { + "html": ["echo build html"], + "pdf": ["echo build pdf"], + }, + }, + }, + }, + ) + build.validate() + assert isinstance(build.build, BuildWithOs) + assert isinstance(build.build.jobs, BuildJobs) + assert build.build.jobs.create_environment == ["echo make_environment"] + assert build.build.jobs.install == ["echo install"] + assert build.build.jobs.build.html == ["echo build html"] + assert build.build.jobs.build.pdf == ["echo build pdf"] + + def test_build_jobs_build_html_is_required(self): + build = get_build_config( + { + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3"}, + "jobs": { + "build": { + "pdf": ["echo build pdf"], + }, + }, + }, + }, + ) + with raises(ConfigError) as excinfo: + build.validate() + assert excinfo.value.message_id == ConfigError.HTML_BUILD_STEP_REQUIRED + + def test_build_jobs_build_defaults(self): + build = get_build_config( + { + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3"}, + "jobs": { + "build": { + "html": ["echo build html"], + }, + }, + }, + }, + ) + build.validate() + assert build.build.jobs.build.html == ["echo build html"] + assert build.build.jobs.build.pdf == [] + + def test_build_jobs_build_cant_be_used_with_formats(self): + build = get_build_config( + { + "formats": ["pdf"], + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3"}, + "jobs": { + "build": { + "html": ["echo build html"], + }, + }, + }, + }, + ) + + with raises(ConfigError) as excinfo: + build.validate() + + assert excinfo.value.message_id == ConfigError.FORMATS_AND_BUILD_JOBS_BUILD + @pytest.mark.parametrize( "value", [ From 6cd3ad75ccdf99f3d7496b21e371cb986f911c1c Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 14:12:11 -0500 Subject: [PATCH 05/16] Tests --- readthedocs/config/tests/test_config.py | 25 ++++++++ .../projects/tests/test_build_tasks.py | 61 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index e8a12f6eae8..29b56f6bb9c 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -702,6 +702,31 @@ def test_build_jobs_build_defaults(self): assert build.build.jobs.build.html == ["echo build html"] assert build.build.jobs.build.pdf == [] + def test_build_jobs_partial_override_empty_commands(self): + build = get_build_config( + { + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3"}, + "jobs": { + "create_environment": [], + "install": [], + "build": { + "html": [], + "pdf": [], + }, + }, + }, + }, + ) + build.validate() + assert isinstance(build.build, BuildWithOs) + assert isinstance(build.build.jobs, BuildJobs) + assert build.build.jobs.create_environment == [] + assert build.build.jobs.install == [] + assert build.build.jobs.build.html == [] + assert build.build.jobs.build.pdf == [] + def test_build_jobs_build_cant_be_used_with_formats(self): build = get_build_config( { diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 9d62e151ac0..8dbb672357c 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1211,6 +1211,67 @@ def test_build_jobs(self, load_yaml_config): any_order=True, ) + @mock.patch("readthedocs.doc_builder.director.load_yaml_config") + def test_build_jobs_partial_build_override(self, load_yaml_config): + config = BuildConfigV2( + { + "version": 2, + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3.12"}, + "jobs": { + "create_environment": ["echo create_environment"], + "install": ["echo install"], + "build": { + "html": ["echo build html"], + "pdf": ["echo build pdf"], + }, + }, + }, + }, + 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 create_environment", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo install", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo build html", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo build pdf", + escape_command=False, + cwd=mock.ANY, + ), + ] + ) + @mock.patch("readthedocs.doc_builder.director.tarfile") @mock.patch("readthedocs.doc_builder.director.build_tools_storage") @mock.patch("readthedocs.doc_builder.director.load_yaml_config") From 0034f5383a6c540b3b8e9834512006f287b337d9 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 14:18:34 -0500 Subject: [PATCH 06/16] Test empty overrides --- .../projects/tests/test_build_tasks.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 8dbb672357c..072c0d87338 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1272,6 +1272,49 @@ 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_empty_commands(self, load_yaml_config): + config = BuildConfigV2( + { + "version": 2, + "build": { + "os": "ubuntu-24.04", + "tools": {"python": "3.12"}, + "jobs": { + "create_environment": [], + "install": [], + "build": { + "html": [], + "pdf": [], + }, + "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 end of build", escape_command=False, cwd=mock.ANY), + ] + ) + @mock.patch("readthedocs.doc_builder.director.tarfile") @mock.patch("readthedocs.doc_builder.director.build_tools_storage") @mock.patch("readthedocs.doc_builder.director.load_yaml_config") From 8cc5421d152c209c63015b0b16f40418116d417c Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 14:40:56 -0500 Subject: [PATCH 07/16] Maybe is tox? --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c5d62d7fa5e..0ce781b9945 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -81,7 +81,7 @@ jobs: - restore_cache: keys: - pre-commit-cache-{{ checksum "pre-commit-cache-key.txt" }} - - run: pip install --user 'tox<5' + - run: pip install --user tox - run: tox -e pre-commit - run: tox -e migrations - node/install: From 8e0c438358dfcdca2ff32a9d87aed787b3438dc6 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Thu, 24 Oct 2024 16:52:49 -0500 Subject: [PATCH 08/16] Updates from review --- readthedocs/config/models.py | 2 ++ readthedocs/config/tests/test_config.py | 6 ++++++ readthedocs/core/utils/objects.py | 12 +++--------- readthedocs/doc_builder/director.py | 2 ++ readthedocs/projects/tests/test_build_tasks.py | 18 ++++++++++++++++++ .../rtd_tests/fixtures/spec/v2/schema.json | 14 ++++++++++++++ 6 files changed, 45 insertions(+), 9 deletions(-) diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index 88e82c9d41b..6345d24200a 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -44,6 +44,8 @@ class BuildJobsBuildTypes(BaseModel): html: list[str] pdf: list[str] = [] + epub: list[str] = [] + htmlzip: list[str] = [] def as_dict(self): # Just to keep compatibility with the old implementation. diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 29b56f6bb9c..dab1ed01ee0 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -653,6 +653,8 @@ def test_build_jobs_partial_override(self): "build": { "html": ["echo build html"], "pdf": ["echo build pdf"], + "epub": ["echo build epub"], + "htmlzip": ["echo build htmlzip"], }, }, }, @@ -665,6 +667,8 @@ def test_build_jobs_partial_override(self): assert build.build.jobs.install == ["echo install"] assert build.build.jobs.build.html == ["echo build html"] assert build.build.jobs.build.pdf == ["echo build pdf"] + assert build.build.jobs.build.epub == ["echo build epub"] + assert build.build.jobs.build.htmlzip == ["echo build htmlzip"] def test_build_jobs_build_html_is_required(self): build = get_build_config( @@ -726,6 +730,8 @@ def test_build_jobs_partial_override_empty_commands(self): assert build.build.jobs.install == [] assert build.build.jobs.build.html == [] assert build.build.jobs.build.pdf == [] + assert build.build.jobs.build.epub == [] + assert build.build.jobs.build.htmlzip == [] def test_build_jobs_build_cant_be_used_with_formats(self): build = get_build_config( diff --git a/readthedocs/core/utils/objects.py b/readthedocs/core/utils/objects.py index 2d690ef100e..a1b26da700e 100644 --- a/readthedocs/core/utils/objects.py +++ b/readthedocs/core/utils/objects.py @@ -1,3 +1,6 @@ +# Sentinel value to check if a default value was provided, +# so we can differentiate when None is provided as a default value +# and when it was not provided at all. _DEFAULT = object() @@ -17,12 +20,3 @@ def get_dotted_attribute(obj, attribute, default=_DEFAULT): else: raise AttributeError(f"Object {obj} has no attribute {attr}") return obj - - -def has_dotted_attribute(obj, attribute): - """Check if an object has a nested attribute using a dot notation.""" - try: - get_dotted_attribute(obj, attribute) - return True - except AttributeError: - return False diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 63ecf39a6d9..88fc286bace 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -200,7 +200,9 @@ def build(self): build_overridden = config.build.jobs.build is not None if build_overridden: self.run_build_job("build.html") + self.run_build_job("build.htmlzip") self.run_build_job("build.pdf") + self.run_build_job("build.epub") else: self.build_html() self.build_htmlzip() diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 072c0d87338..83e8e9658f6 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -1225,7 +1225,10 @@ def test_build_jobs_partial_build_override(self, load_yaml_config): "build": { "html": ["echo build html"], "pdf": ["echo build pdf"], + "epub": ["echo build epub"], + "htmlzip": ["echo build htmlzip"], }, + "post_build": ["echo end of build"], }, }, }, @@ -1264,11 +1267,26 @@ def test_build_jobs_partial_build_override(self, load_yaml_config): escape_command=False, cwd=mock.ANY, ), + mock.call( + "echo build htmlzip", + escape_command=False, + cwd=mock.ANY, + ), mock.call( "echo build pdf", escape_command=False, cwd=mock.ANY, ), + mock.call( + "echo build epub", + escape_command=False, + cwd=mock.ANY, + ), + mock.call( + "echo end of build", + escape_command=False, + cwd=mock.ANY, + ), ] ) diff --git a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json index 6e44fa54ba1..b7e635b90cd 100644 --- a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json +++ b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json @@ -145,12 +145,26 @@ "type": "string" } }, + "htmlzip": { + "type": "array", + "items": { + "title": "Custom commands", + "type": "string" + } + }, "pdf": { "type": "array", "items": { "title": "Custom commands", "type": "string" } + }, + "epub": { + "type": "array", + "items": { + "title": "Custom commands", + "type": "string" + } } } }, From 3ef8fe628ba0574ef7eed3ae8468043a1aa4c874 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Wed, 20 Nov 2024 11:56:14 -0500 Subject: [PATCH 09/16] Allow formats with build.jobs.build.* --- readthedocs/config/config.py | 26 ++++------ readthedocs/config/exceptions.py | 5 +- readthedocs/config/models.py | 10 ++-- readthedocs/config/notifications.py | 16 +------ readthedocs/config/tests/test_config.py | 48 ++++++++----------- readthedocs/doc_builder/director.py | 41 +++++++++------- .../projects/tests/test_build_tasks.py | 9 +++- .../rtd_tests/fixtures/spec/v2/schema.json | 1 - 8 files changed, 70 insertions(+), 86 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 262313c73b0..79357ec88a8 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -250,7 +250,6 @@ def validate(self): self._config["sphinx"] = self.validate_sphinx() self._config["submodules"] = self.validate_submodules() self._config["search"] = self.validate_search() - self.validate_incompatible_keys() self.validate_keys() def validate_formats(self): @@ -383,20 +382,20 @@ def validate_build_config_with_os(self): return build def validate_build_jobs_build(self, build_jobs): - # The build.jobs.build key is optional. - if "build" not in build_jobs: - return None - result = {} - build_jobs_build = build_jobs["build"] + build_jobs_build = build_jobs.get("build", {}) validate_dict(build_jobs_build) - if not "html" in build_jobs_build: - raise ConfigError(message_id=ConfigError.HTML_BUILD_STEP_REQUIRED) - allowed_build_types = list(BuildJobsBuildTypes.model_fields.keys()) for build_type, build_commands in build_jobs_build.items(): validate_choice(build_type, allowed_build_types) + if build_type != "html" and build_type not in self.formats: + raise ConfigError( + message_id=ConfigError.BUILD_JOBS_BUILD_TYPE_MISSING_IN_FORMATS, + format_values={ + "build_type": build_type, + }, + ) with self.catch_validation_error(f"build.jobs.build.{build_type}"): result[build_type] = [ validate_string(build_command) @@ -727,15 +726,6 @@ def validate_search(self): return search - def validate_incompatible_keys(self): - # `formats` and `build.jobs.build.*` can't be used together. - build_overridden = ( - self.is_using_build_jobs and self.build.jobs.build is not None - ) - with self.catch_validation_error("formats"): - if build_overridden and "formats" in self.source_config: - raise ConfigError(message_id=ConfigError.FORMATS_AND_BUILD_JOBS_BUILD) - def validate_keys(self): """ Checks that we don't have extra keys (invalid ones). diff --git a/readthedocs/config/exceptions.py b/readthedocs/config/exceptions.py index 2cde29b4f67..d0fcb30b5f1 100644 --- a/readthedocs/config/exceptions.py +++ b/readthedocs/config/exceptions.py @@ -13,8 +13,9 @@ class ConfigError(BuildUserError): INVALID_VERSION = "config:base:invalid-version" NOT_BUILD_TOOLS_OR_COMMANDS = "config:build:missing-build-tools-commands" BUILD_JOBS_AND_COMMANDS = "config:build:jobs-and-commands" - FORMATS_AND_BUILD_JOBS_BUILD = "config:formats:formats-and-build" - HTML_BUILD_STEP_REQUIRED = "config:build:jobs:build:html-build-step-required" + BUILD_JOBS_BUILD_TYPE_MISSING_IN_FORMATS = ( + "config:build:jobs:build:missing-in-formats" + ) APT_INVALID_PACKAGE_NAME_PREFIX = "config:apt:invalid-package-name-prefix" APT_INVALID_PACKAGE_NAME = "config:apt:invalid-package-name" USE_PIP_FOR_EXTRA_REQUIREMENTS = "config:python:pip-required" diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index 6345d24200a..c96e2462586 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -42,10 +42,10 @@ class BuildJobsBuildTypes(BaseModel): """Object used for `build.jobs.build` key.""" - html: list[str] - pdf: list[str] = [] - epub: list[str] = [] - htmlzip: list[str] = [] + html: list[str] | None = None + pdf: list[str] | None = None + epub: list[str] | None = None + htmlzip: list[str] | None = None def as_dict(self): # Just to keep compatibility with the old implementation. @@ -67,7 +67,7 @@ class BuildJobs(BaseModel): install: list[str] | None = None post_install: list[str] = [] pre_build: list[str] = [] - build: BuildJobsBuildTypes | None = None + build: BuildJobsBuildTypes = BuildJobsBuildTypes() post_build: list[str] = [] def as_dict(self): diff --git a/readthedocs/config/notifications.py b/readthedocs/config/notifications.py index d8c73399099..d3334390d00 100644 --- a/readthedocs/config/notifications.py +++ b/readthedocs/config/notifications.py @@ -125,24 +125,12 @@ type=ERROR, ), Message( - id=ConfigError.FORMATS_AND_BUILD_JOBS_BUILD, + id=ConfigError.BUILD_JOBS_BUILD_TYPE_MISSING_IN_FORMATS, header=_("Invalid configuration option"), body=_( textwrap.dedent( """ - The keys <code>build.jobs.build</code> and <code>formats</code> can't be used together. - """ - ).strip(), - ), - type=ERROR, - ), - Message( - id=ConfigError.HTML_BUILD_STEP_REQUIRED, - header=_("Missing configuration option"), - body=_( - textwrap.dedent( - """ - The key <code>build.jobs.build.html</code> is required when using <code>build.jobs.build</code>. + The <code>{{ build_type }}</code> build type was defined in <code>build.jobs.build</code>, but it wasn't included in <code>formats</code>. """ ).strip(), ), diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index dab1ed01ee0..dc60b330ea0 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -14,6 +14,7 @@ from readthedocs.config.exceptions import ConfigError, ConfigValidationError from readthedocs.config.models import ( BuildJobs, + BuildJobsBuildTypes, BuildWithOs, PythonInstall, PythonInstallRequirements, @@ -638,12 +639,13 @@ def test_jobs_build_config(self): "echo pre_build", 'sed -i -e "s|{VERSION}|${READTHEDOCS_VERSION_NAME}|g"', ] - assert build.build.jobs.build is None + assert build.build.jobs.build == BuildJobsBuildTypes() assert build.build.jobs.post_build == ["echo post_build"] def test_build_jobs_partial_override(self): build = get_build_config( { + "formats": ["pdf", "htmlzip", "epub"], "build": { "os": "ubuntu-20.04", "tools": {"python": "3"}, @@ -670,15 +672,16 @@ def test_build_jobs_partial_override(self): assert build.build.jobs.build.epub == ["echo build epub"] assert build.build.jobs.build.htmlzip == ["echo build htmlzip"] - def test_build_jobs_build_html_is_required(self): + def test_build_jobs_build_should_match_formats(self): build = get_build_config( { + "formats": ["pdf"], "build": { "os": "ubuntu-24.04", "tools": {"python": "3"}, "jobs": { "build": { - "pdf": ["echo build pdf"], + "epub": ["echo build epub"], }, }, }, @@ -686,7 +689,7 @@ def test_build_jobs_build_html_is_required(self): ) with raises(ConfigError) as excinfo: build.validate() - assert excinfo.value.message_id == ConfigError.HTML_BUILD_STEP_REQUIRED + assert excinfo.value.message_id == ConfigError.BUILD_JOBS_BUILD_TYPE_MISSING_IN_FORMATS def test_build_jobs_build_defaults(self): build = get_build_config( @@ -704,11 +707,14 @@ def test_build_jobs_build_defaults(self): ) build.validate() assert build.build.jobs.build.html == ["echo build html"] - assert build.build.jobs.build.pdf == [] + assert build.build.jobs.build.pdf is None + assert build.build.jobs.build.htmlzip is None + assert build.build.jobs.build.epub is None def test_build_jobs_partial_override_empty_commands(self): build = get_build_config( { + "formats": ["pdf"], "build": { "os": "ubuntu-24.04", "tools": {"python": "3"}, @@ -730,29 +736,8 @@ def test_build_jobs_partial_override_empty_commands(self): assert build.build.jobs.install == [] assert build.build.jobs.build.html == [] assert build.build.jobs.build.pdf == [] - assert build.build.jobs.build.epub == [] - assert build.build.jobs.build.htmlzip == [] - - def test_build_jobs_build_cant_be_used_with_formats(self): - build = get_build_config( - { - "formats": ["pdf"], - "build": { - "os": "ubuntu-24.04", - "tools": {"python": "3"}, - "jobs": { - "build": { - "html": ["echo build html"], - }, - }, - }, - }, - ) - - with raises(ConfigError) as excinfo: - build.validate() - - assert excinfo.value.message_id == ConfigError.FORMATS_AND_BUILD_JOBS_BUILD + assert build.build.jobs.build.epub == None + assert build.build.jobs.build.htmlzip == None @pytest.mark.parametrize( "value", @@ -1879,7 +1864,12 @@ def test_as_dict_new_build_config(self, tmpdir): "install": None, "post_install": [], "pre_build": [], - "build": None, + "build": { + "html": None, + "pdf": None, + "epub": None, + "htmlzip": None, + }, "post_build": [], }, "apt_packages": [], diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 88fc286bace..117fe7e352d 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -177,7 +177,7 @@ def setup_environment(self): self.run_build_job("post_create_environment") self.run_build_job("pre_install") - if self.data.config.build.jobs.install is not None: + if config.build.jobs.install is not None: self.run_build_job("install") else: self.install() @@ -192,22 +192,13 @@ def build(self): 3. build PDF 4. build ePub """ - config = self.data.config - self.run_build_job("pre_build") # Build all formats - build_overridden = config.build.jobs.build is not None - if build_overridden: - self.run_build_job("build.html") - self.run_build_job("build.htmlzip") - self.run_build_job("build.pdf") - self.run_build_job("build.epub") - else: - self.build_html() - self.build_htmlzip() - self.build_pdf() - self.build_epub() + self.build_html() + self.build_htmlzip() + self.build_pdf() + self.build_epub() self.run_build_job("post_build") self.store_readthedocs_build_yaml() @@ -331,12 +322,17 @@ def install(self): # Build def build_html(self): + if self.run_build_job("build.html"): + return True return self.build_docs_class(self.data.config.doctype) def build_pdf(self): if "pdf" not in self.data.config.formats or self.data.version.type == EXTERNAL: return False + if self.run_build_job("build.pdf"): + return True + # Mkdocs has no pdf generation currently. if self.is_type_sphinx(): return self.build_docs_class("sphinx_pdf") @@ -350,6 +346,9 @@ def build_htmlzip(self): ): return False + if self.run_build_job("build.htmlzip"): + return True + # We don't generate a zip for mkdocs currently. if self.is_type_sphinx(): return self.build_docs_class("sphinx_singlehtmllocalmedia") @@ -359,6 +358,9 @@ def build_epub(self): if "epub" not in self.data.config.formats or self.data.version.type == EXTERNAL: return False + if self.run_build_job("build.epub"): + return True + # Mkdocs has no epub generation currently. if self.is_type_sphinx(): return self.build_docs_class("sphinx_epub") @@ -397,9 +399,14 @@ def run_build_job(self, job): In this case, `self.data.config.build.jobs.pre_build` will contains `sed` command. """ - commands = get_dotted_attribute(self.data.config, f"build.jobs.{job}", []) + commands = get_dotted_attribute(self.data.config, f"build.jobs.{job}", None) + # If it's None, the job wasn't defined. + if commands is None: + return False + + # The job was defined, but it's empty. if not commands: - return + return True cwd = self.data.project.checkout_path(self.data.version.slug) environment = self.vcs_environment @@ -409,6 +416,8 @@ def run_build_job(self, job): for command in commands: environment.run(command, escape_command=False, cwd=cwd) + return True + def check_old_output_directory(self): """ Check if there the directory '_build/html' exists and fail the build if so. diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index 61c4900888b..5fa64b6673b 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -567,7 +567,12 @@ def test_successful_build( "install": None, "post_install": [], "pre_build": [], - "build": None, + "build": { + "html": None, + "pdf": None, + "epub": None, + "htmlzip": None, + }, "post_build": [], }, "tools": { @@ -1211,6 +1216,7 @@ def test_build_jobs_partial_build_override(self, load_yaml_config): config = BuildConfigV2( { "version": 2, + "formats": ["pdf", "epub", "htmlzip"], "build": { "os": "ubuntu-24.04", "tools": {"python": "3.12"}, @@ -1290,6 +1296,7 @@ def test_build_jobs_partial_build_override_empty_commands(self, load_yaml_config config = BuildConfigV2( { "version": 2, + "formats": ["pdf"], "build": { "os": "ubuntu-24.04", "tools": {"python": "3.12"}, diff --git a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json index bf5857f41f9..716b04e55ef 100644 --- a/readthedocs/rtd_tests/fixtures/spec/v2/schema.json +++ b/readthedocs/rtd_tests/fixtures/spec/v2/schema.json @@ -136,7 +136,6 @@ "description": "Override the default build process.", "type": "object", "additionalProperties": false, - "required": ["html"], "properties": { "html": { "type": "array", From 5a8a52e46a0a8958b713a453d41270c90d070986 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Wed, 20 Nov 2024 13:18:40 -0500 Subject: [PATCH 10/16] Remove unused variable --- readthedocs/config/config.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 79357ec88a8..d786148b0a8 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -103,7 +103,6 @@ def __init__(self, raw_config, source_file, base_path=None): self._config = {} self.is_using_build_commands = False - self.is_using_build_jobs = False @contextmanager def catch_validation_error(self, key): @@ -345,8 +344,6 @@ def validate_build_config_with_os(self): if commands: self.is_using_build_commands = True - else: - self.is_using_build_jobs = True build["jobs"] = {} From 890552df85c97969f43797c1f9df24d49284da39 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Wed, 20 Nov 2024 13:30:27 -0500 Subject: [PATCH 11/16] Format --- readthedocs/config/tests/test_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index dc60b330ea0..e61d76a5c7a 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -689,7 +689,10 @@ def test_build_jobs_build_should_match_formats(self): ) with raises(ConfigError) as excinfo: build.validate() - assert excinfo.value.message_id == ConfigError.BUILD_JOBS_BUILD_TYPE_MISSING_IN_FORMATS + assert ( + excinfo.value.message_id + == ConfigError.BUILD_JOBS_BUILD_TYPE_MISSING_IN_FORMATS + ) def test_build_jobs_build_defaults(self): build = get_build_config( From 806ff86b67f2e87ac78cf19f2725d04c4c05422b Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Wed, 20 Nov 2024 16:59:21 -0500 Subject: [PATCH 12/16] Use old method for now --- readthedocs/config/config.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index d786148b0a8..53690534716 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -102,8 +102,6 @@ def __init__(self, raw_config, source_file, base_path=None): self._config = {} - self.is_using_build_commands = False - @contextmanager def catch_validation_error(self, key): """Catch a ``ConfigValidationError`` and raises a ``ConfigError`` error.""" @@ -178,6 +176,10 @@ def validate(self): def is_using_conda(self): return self.python_interpreter in ("conda", "mamba") + @property + def is_using_build_commands(self): + return self.build.commands != [] + @property def is_using_setup_py_install(self): """Check if this project is using `setup.py install` as installation method.""" @@ -342,9 +344,6 @@ def validate_build_config_with_os(self): }, ) - if commands: - self.is_using_build_commands = True - build["jobs"] = {} with self.catch_validation_error("build.jobs.build"): From 0c5fea8624deafaff7700ba78a41e2832b679d69 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Mon, 25 Nov 2024 11:46:51 -0500 Subject: [PATCH 13/16] Use conditional --- readthedocs/doc_builder/director.py | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 117fe7e352d..e523be06ada 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -322,16 +322,18 @@ def install(self): # Build def build_html(self): - if self.run_build_job("build.html"): - return True + if self.data.config.build.jobs.build.html is not None: + self.run_build_job("build.html") + return return self.build_docs_class(self.data.config.doctype) def build_pdf(self): if "pdf" not in self.data.config.formats or self.data.version.type == EXTERNAL: return False - if self.run_build_job("build.pdf"): - return True + if self.data.config.build.jobs.build.pdf is not None: + self.run_build_job("build.pdf") + return # Mkdocs has no pdf generation currently. if self.is_type_sphinx(): @@ -346,8 +348,9 @@ def build_htmlzip(self): ): return False - if self.run_build_job("build.htmlzip"): - return True + if self.data.config.build.jobs.build.htmlzip is not None: + self.run_build_job("build.htmlzip") + return # We don't generate a zip for mkdocs currently. if self.is_type_sphinx(): @@ -358,8 +361,9 @@ def build_epub(self): if "epub" not in self.data.config.formats or self.data.version.type == EXTERNAL: return False - if self.run_build_job("build.epub"): - return True + if self.data.config.build.jobs.build.epub is not None: + self.run_build_job("build.epub") + return # Mkdocs has no epub generation currently. if self.is_type_sphinx(): @@ -400,13 +404,8 @@ def run_build_job(self, job): `sed` command. """ commands = get_dotted_attribute(self.data.config, f"build.jobs.{job}", None) - # If it's None, the job wasn't defined. - if commands is None: - return False - - # The job was defined, but it's empty. if not commands: - return True + return cwd = self.data.project.checkout_path(self.data.version.slug) environment = self.vcs_environment @@ -416,8 +415,6 @@ def run_build_job(self, job): for command in commands: environment.run(command, escape_command=False, cwd=cwd) - return True - def check_old_output_directory(self): """ Check if there the directory '_build/html' exists and fail the build if so. From c8cba449b49480f5e0b651e7cd3fb4cbc25a1618 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Mon, 25 Nov 2024 11:50:17 -0500 Subject: [PATCH 14/16] Move all actions to their method --- readthedocs/doc_builder/director.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index e523be06ada..fcc72564b1a 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -170,17 +170,11 @@ def setup_environment(self): self.install_build_tools() self.run_build_job("pre_create_environment") - if config.build.jobs.create_environment is not None: - self.run_build_job("create_environment") - else: - self.create_environment() + self.create_environment() self.run_build_job("post_create_environment") self.run_build_job("pre_install") - if config.build.jobs.install is not None: - self.run_build_job("install") - else: - self.install() + self.install() self.run_build_job("post_install") def build(self): @@ -313,10 +307,17 @@ def system_dependencies(self): # Language environment def create_environment(self): + if self.config.build.jobs.create_environment is not None: + self.run_build_job("create_environment") + return self.language_environment.setup_base() # Install def install(self): + if self.config.build.jobs.install is not None: + self.run_build_job("install") + return + self.language_environment.install_core_requirements() self.language_environment.install_requirements() From c0b463c756ce4876d4b4cdeaedd7ce48d80ac494 Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Mon, 25 Nov 2024 12:01:13 -0500 Subject: [PATCH 15/16] Unused variable --- readthedocs/doc_builder/director.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index fcc72564b1a..9e52b59986e 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -160,7 +160,6 @@ def setup_environment(self): sender=self.data.version, environment=self.build_environment, ) - config = self.data.config self.run_build_job("pre_system_dependencies") self.system_dependencies() From f22413d7e2d205d5ecf279b4ec7e6194d19ecbcd Mon Sep 17 00:00:00 2001 From: Santos Gallegos <stsewd@proton.me> Date: Mon, 25 Nov 2024 12:24:32 -0500 Subject: [PATCH 16/16] Fix --- readthedocs/doc_builder/director.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py index 9e52b59986e..02a9b806757 100644 --- a/readthedocs/doc_builder/director.py +++ b/readthedocs/doc_builder/director.py @@ -306,14 +306,14 @@ def system_dependencies(self): # Language environment def create_environment(self): - if self.config.build.jobs.create_environment is not None: + if self.data.config.build.jobs.create_environment is not None: self.run_build_job("create_environment") return self.language_environment.setup_base() # Install def install(self): - if self.config.build.jobs.install is not None: + if self.data.config.build.jobs.install is not None: self.run_build_job("install") return