Build: allow partial override of build steps

.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:

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).

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"

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):

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"),

readthedocs/config/tests/test_config.py

@@ -627,17 +627,127 @@ 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_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(
+            {
+                "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",
         [
@@ -1757,10 +1867,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": [],