Updates: Rename new field to build_config_file, add on Build and Version data, relax user input validation, introduce very basic reproducibility of builds

Author: benjaoming

readthedocs/api/v2/serializers.py

@@ -92,7 +92,7 @@ class Meta(ProjectSerializer.Meta):
             "show_advertising",
             "environment_variables",
             "max_concurrent_builds",
-            "rtd_conf_file",
+            "build_config_file",
         )
 
 

readthedocs/builds/migrations/0048_add_build_config_file.py

@@ -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",
+            ),
+        ),
+    ]

readthedocs/builds/models.py

@@ -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,
@@ -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']
@@ -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)
 

readthedocs/config/config.py

@@ -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'
@@ -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."""
@@ -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(

readthedocs/doc_builder/backends/mkdocs.py

@@ -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

readthedocs/doc_builder/config.py

@@ -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
@@ -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(

readthedocs/doc_builder/director.py

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

readthedocs/projects/forms.py

@@ -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 = (
@@ -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):
@@ -391,7 +384,9 @@ class UpdateProjectForm(
         ProjectExtraForm,
 ):
 
-    class Meta:
+    """Basic project settings form for Admin."""
+
+    class Meta:  # noqa
         model = Project
         fields = (
             # Basics
@@ -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:
@@ -540,6 +536,8 @@ def save(self):
 
 class WebHookForm(forms.ModelForm):
 
+    """Webhook form."""
+
     project = forms.CharField(widget=forms.HiddenInput(), required=False)
 
     class Meta:
@@ -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
@@ -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)
 
@@ -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(