Skip to content

Commit

Permalink
Updates: Rename new field to build_config_file, add on Build and Vers…
Browse files Browse the repository at this point in the history
…ion data, relax user input validation, introduce very basic reproducibility of builds
  • Loading branch information
benjaoming committed Mar 10, 2023
1 parent 1bd6a69 commit 80c3140
Show file tree
Hide file tree
Showing 14 changed files with 197 additions and 91 deletions.
2 changes: 1 addition & 1 deletion readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class Meta(ProjectSerializer.Meta):
"show_advertising",
"environment_variables",
"max_concurrent_builds",
"rtd_conf_file",
"build_config_file",
)


Expand Down
1 change: 1 addition & 0 deletions readthedocs/builds/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class BuildAdmin(admin.ModelAdmin):
"date",
"builder",
"length",
"build_config_file",
"pretty_config",
)
readonly_fields = (
Expand Down
35 changes: 35 additions & 0 deletions readthedocs/builds/migrations/0048_add_build_config_file.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Generated by Django 3.2.18 on 2023-03-10 16:20

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("builds", "0047_build_default_triggered"),
]

operations = [
migrations.AddField(
model_name="build",
name="build_config_file",
field=models.CharField(
blank=True,
default=None,
max_length=1024,
null=True,
verbose_name="Non-default build configuration file used",
),
),
migrations.AddField(
model_name="version",
name="build_config_file",
field=models.CharField(
blank=True,
default=None,
max_length=1024,
null=True,
verbose_name="Non-default build configuration file used",
),
),
]
17 changes: 15 additions & 2 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,9 @@
BITBUCKET_COMMIT_URL,
BITBUCKET_URL,
DOCTYPE_CHOICES,
GITHUB_BRAND,
GITHUB_COMMIT_URL,
GITHUB_PULL_REQUEST_COMMIT_URL,
GITHUB_URL,
GITLAB_BRAND,
GITLAB_COMMIT_URL,
GITLAB_MERGE_REQUEST_COMMIT_URL,
GITLAB_URL,
Expand Down Expand Up @@ -192,6 +190,14 @@ class Version(TimeStampedModel):
# Only include EXTERNAL type Versions.
external = ExternalVersionManager.from_queryset(partial(VersionQuerySet, external_only=True))()

build_config_file = models.CharField(
_("Non-default build configuration file used"),
max_length=1024,
default=None,
blank=True,
null=True,
)

class Meta:
unique_together = [('project', 'slug')]
ordering = ['-verbose_name']
Expand Down Expand Up @@ -702,6 +708,13 @@ class Build(models.Model):
null=True,
blank=True,
)
build_config_file = models.CharField(
_("Non-default build configuration file used"),
max_length=1024,
default=None,
blank=True,
null=True,
)

length = models.IntegerField(_('Build Length'), null=True, blank=True)

Expand Down
49 changes: 31 additions & 18 deletions readthedocs/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,18 @@
)

__all__ = (
'ALL',
'load',
'BuildConfigV1',
'BuildConfigV2',
'ConfigError',
'ConfigOptionNotSupportedError',
'ConfigFileNotFound',
'InvalidConfig',
'PIP',
'SETUPTOOLS',
'LATEST_CONFIGURATION_VERSION',
"ALL",
"load",
"BuildConfigV1",
"BuildConfigV2",
"ConfigError",
"ConfigOptionNotSupportedError",
"ConfigFileNotFound",
"DefaultConfigFileNotFound",
"InvalidConfig",
"PIP",
"SETUPTOOLS",
"LATEST_CONFIGURATION_VERSION",
)

ALL = 'all'
Expand Down Expand Up @@ -96,6 +97,17 @@ def __init__(self, directory):
)


class DefaultConfigFileNotFound(ConfigError):

"""Error when we can't find a configuration file."""

def __init__(self, directory):
super().__init__(
f"No default configuration file in: {directory}",
CONFIG_FILE_REQUIRED,
)


class ConfigOptionNotSupportedError(ConfigError):

"""Error for unsupported configuration options in a version."""
Expand Down Expand Up @@ -1377,17 +1389,18 @@ def load(path, env_config, config_file=None):
the version of the configuration a build object would be load and validated,
``BuildConfigV1`` is the default.
"""
if config_file is None or config_file == "":
if not config_file:
filename = find_one(path, CONFIG_FILENAME_REGEX)
if not filename:
# This exception is current caught higher up and will result in an attempt
# to load the v1 config schema.
raise DefaultConfigFileNotFound(os.path.relpath(filename, path))
else:
filename = os.path.join(path, config_file)
# When a config file is specified and not found, we raise ConfigError
# because ConfigFileNotFound
if not os.path.exists(filename):
raise ConfigError(
f".readthedocs.yml not found at {config_file}", CONFIG_FILE_REQUIRED
)

if not filename:
raise ConfigFileNotFound(path)
raise ConfigFileNotFound(os.path.relpath(filename, path))

# Allow symlinks, but only the ones that resolve inside the base directory.
with safe_open(
Expand Down
2 changes: 2 additions & 0 deletions readthedocs/doc_builder/backends/mkdocs.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class BaseMkdocs(BaseBuilder):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

# This is the *MkDocs* yaml file
self.yaml_file = self.get_yaml_config()

# README: historically, the default theme was ``readthedocs`` but in
Expand Down
13 changes: 8 additions & 5 deletions readthedocs/doc_builder/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,22 @@

from os import path

from readthedocs.config import BuildConfigV1, ConfigFileNotFound
from readthedocs.config import BuildConfigV1
from readthedocs.config import load as load_config
from readthedocs.projects.models import ProjectConfigurationError

from ..config.config import DefaultConfigFileNotFound
from .constants import DOCKER_IMAGE, DOCKER_IMAGE_SETTINGS


def load_yaml_config(version):
def load_yaml_config(version, config_file=None):
"""
Load a configuration from `readthedocs.yml` file.
Load a build configuration file.
This uses the configuration logic from `readthedocs-build`, which will keep
parsing consistent between projects.
:param config_file: Optionally, we are told which config_file to load instead of using defaults.
"""
checkout_path = version.project.checkout_path(version.slug)
project = version.project
Expand Down Expand Up @@ -56,9 +59,9 @@ def load_yaml_config(version):
config = load_config(
path=checkout_path,
env_config=env_config,
config_file=project.rtd_conf_file,
config_file=config_file,
)
except ConfigFileNotFound:
except DefaultConfigFileNotFound:
# Default to use v1 with some defaults from the web interface
# if we don't find a configuration file.
config = BuildConfigV1(
Expand Down
37 changes: 33 additions & 4 deletions readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
"""
The ``director`` module can be seen as the entrypoint of the build process.
It "directs" all of the high-level build jobs:
* checking out the repo
* setting up the environment
* fetching instructions etc.
"""
import os
import tarfile

Expand Down Expand Up @@ -194,17 +203,36 @@ def build(self):

# VCS checkout
def checkout(self):
log.info(
"Clonning repository.",
)
"""Checkout Git repo and load build config file."""

log.info("Cloning repository.")
self.vcs_repository.update()

identifier = self.data.build_commit or self.data.version.identifier
log.info("Checking out.", identifier=identifier)
self.vcs_repository.checkout(identifier)

self.data.config = load_yaml_config(version=self.data.version)
# The director is responsible for understanding which config file to use for a build.
# Reproducibility: Build already has a config_file set.
non_default_config_file = self.data.build.get("build_config_file", None)

# This logic can be extended with version-specific config files
if not non_default_config_file and self.data.version.project.build_config_file:
non_default_config_file = self.data.version.project.build_config_file

if non_default_config_file:
log.info(f"Using a non-default config file {non_default_config_file}.")
self.data.config = load_yaml_config(
version=self.data.version,
config_file=non_default_config_file,
)
self.data.build["config"] = self.data.config.as_dict()
self.data.build["build_config_file"] = non_default_config_file

# Config file was successfully loaded and a custom path was used
if non_default_config_file and not self.data.version.build_config_file:
self.data.version.build_config_file = non_default_config_file
# TODO: Do we call self.data.version.save() here?

if self.vcs_repository.supports_submodules:
self.vcs_repository.update_submodules(self.data.config)
Expand Down Expand Up @@ -357,6 +385,7 @@ def check_old_output_directory(self):
raise BuildUserError(BuildUserError.BUILD_OUTPUT_OLD_DIRECTORY_USED)

def run_build_commands(self):
"""Runs each build command in the build environment."""
reshim_commands = (
{"pip", "install"},
{"conda", "create"},
Expand Down
24 changes: 13 additions & 11 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class Meta:
"single_version",
"external_builds_enabled",
"external_builds_privacy_level",
"rtd_conf_file",
"build_config_file",
)
# These that can be set per-version using a config file.
per_version_settings = (
Expand Down Expand Up @@ -358,15 +358,8 @@ def clean_conf_py_file(self):
) # yapf: disable
return filename

def clean_rtd_conf_file(self):
filename = self.cleaned_data.get("rtd_conf_file", "").strip()
if filename and ".readthedocs.yml" not in filename:
raise forms.ValidationError(
_(
"Your configuration file is invalid, make sure it contains "
".readthedocs.yml in it.",
),
) # yapf: disable
def clean_build_config_file(self):
filename = self.cleaned_data.get("build_config_file", "").strip()
return filename

def get_all_active_versions(self):
Expand All @@ -391,7 +384,9 @@ class UpdateProjectForm(
ProjectExtraForm,
):

class Meta:
"""Basic project settings form for Admin."""

class Meta: # noqa
model = Project
fields = (
# Basics
Expand All @@ -407,6 +402,7 @@ class Meta:
)

def clean_language(self):
"""Ensure that language isn't already active."""
language = self.cleaned_data['language']
project = self.instance
if project:
Expand Down Expand Up @@ -540,6 +536,8 @@ def save(self):

class WebHookForm(forms.ModelForm):

"""Webhook form."""

project = forms.CharField(widget=forms.HiddenInput(), required=False)

class Meta:
Expand Down Expand Up @@ -607,6 +605,8 @@ def get_choices(self):
) for project in self.get_translation_queryset().all()]

def clean_project(self):
"""Ensures that selected project is valid as a translation."""

translation_project_slug = self.cleaned_data['project']

# Ensure parent project isn't already itself a translation
Expand Down Expand Up @@ -726,6 +726,7 @@ def clean_project(self):
return self.project

def clean_domain(self):
"""Validates domain."""
domain = self.cleaned_data['domain'].lower()
parsed = urlparse(domain)

Expand Down Expand Up @@ -864,6 +865,7 @@ def clean_project(self):
return self.project

def clean_name(self):
"""Validate environment variable name chosen."""
name = self.cleaned_data['name']
if name.startswith('__'):
raise forms.ValidationError(
Expand Down
35 changes: 0 additions & 35 deletions readthedocs/projects/migrations/0096_auto_20230207_1642.py

This file was deleted.

Loading

0 comments on commit 80c3140

Please sign in to comment.