Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build: allow partial override of build steps #11710

Merged
merged 20 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 12 additions & 26 deletions readthedocs/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,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):
"""Catch a ``ConfigValidationError`` and raises a ``ConfigError`` error."""
Expand Down Expand Up @@ -179,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."""
Expand Down Expand Up @@ -250,7 +251,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):
Expand Down Expand Up @@ -344,11 +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"] = {}

with self.catch_validation_error("build.jobs.build"):
Expand Down Expand Up @@ -383,20 +378,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)
Expand Down Expand Up @@ -727,15 +722,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).
Expand Down
5 changes: 3 additions & 2 deletions readthedocs/config/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 5 additions & 5 deletions readthedocs/config/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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):
Expand Down
16 changes: 2 additions & 14 deletions readthedocs/config/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
),
Expand Down
51 changes: 22 additions & 29 deletions readthedocs/config/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from readthedocs.config.exceptions import ConfigError, ConfigValidationError
from readthedocs.config.models import (
BuildJobs,
BuildJobsBuildTypes,
BuildWithOs,
PythonInstall,
PythonInstallRequirements,
Expand Down Expand Up @@ -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"},
Expand All @@ -670,23 +672,27 @@ 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"],
},
},
},
},
)
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(
Expand All @@ -704,11 +710,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"},
Expand All @@ -730,29 +739,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",
Expand Down Expand Up @@ -1879,7 +1867,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": [],
Expand Down
41 changes: 25 additions & 16 deletions readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
stsewd marked this conversation as resolved.
Show resolved Hide resolved

self.run_build_job("post_build")
self.store_readthedocs_build_yaml()
Expand Down Expand Up @@ -331,12 +322,17 @@ def install(self):

# Build
def build_html(self):
if self.run_build_job("build.html"):
return True
stsewd marked this conversation as resolved.
Show resolved Hide resolved
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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
9 changes: 8 additions & 1 deletion readthedocs/projects/tests/test_build_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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"},
Expand Down
Loading