Skip to content

Commit

Permalink
Config file: make sphinx or mkdocs configuration required for project…
Browse files Browse the repository at this point in the history
…s 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
readthedocs/website#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: readthedocs/website#342
- Closes #10637
- Closes #11819
- Closes #11810
- Closes #11216

---------

Co-authored-by: Manuel Kaufmann <[email protected]>
  • Loading branch information
stsewd and humitos authored Dec 23, 2024
1 parent ac87f67 commit 67b01e2
Show file tree
Hide file tree
Showing 9 changed files with 412 additions and 17 deletions.
13 changes: 3 additions & 10 deletions docs/user/config-file/v2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
``````````````````````
Expand Down Expand Up @@ -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
``````````````````````
Expand Down
82 changes: 79 additions & 3 deletions readthedocs/config/config.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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."""
Expand Down Expand Up @@ -219,7 +245,6 @@ def __getattr__(self, name):


class BuildConfigV2(BuildConfigBase):

"""Version 2 of the configuration file."""

version = "2"
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions readthedocs/config/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions readthedocs/config/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,5 +361,31 @@
),
type=ERROR,
),
Message(
id=ConfigError.SPHINX_CONFIG_MISSING,
header=_("Missing Sphinx configuration key"),
body=_(
textwrap.dedent(
"""
The <code>sphinx.configuration</code> key is missing.
This key is now required, see our <a href="https://about.readthedocs.com/blog/2024/12/deprecate-config-files-without-sphinx-or-mkdocs-config/">blog post</a> for more information.
"""
).strip(),
),
type=ERROR,
),
Message(
id=ConfigError.MKDOCS_CONFIG_MISSING,
header=_("Missing MkDocs configuration key"),
body=_(
textwrap.dedent(
"""
The <code>mkdocs.configuration</code> key is missing.
This key is now required, see our <a href="https://about.readthedocs.com/blog/2024/12/deprecate-config-files-without-sphinx-or-mkdocs-config/">blog post</a> for more information.
"""
).strip(),
),
type=ERROR,
),
]
registry.add(messages)
61 changes: 60 additions & 1 deletion readthedocs/config/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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()
Expand Down Expand Up @@ -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(
{
Expand Down
18 changes: 17 additions & 1 deletion readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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()

Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions readthedocs/doc_builder/python_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 = []

Expand Down
Loading

0 comments on commit 67b01e2

Please sign in to comment.