Build: allow to install packages with apt

docs/config-file/v2.rst

@@ -304,8 +304,8 @@ Configuration for the documentation build process.
    build:
      image: latest
      apt_packages:
-       - mysql-client
-       - cmatrix
+       - libclang
+       - cmake
 
    python:
      version: 3.7
@@ -330,6 +330,8 @@ build.apt_packages
 ``````````````````
 
 List of `APT packages`_ to install.
+Our build servers run Ubuntu 18.04, with the default set of package repositories installed.
+We don't currently support PPA's or other custom repositories.
 
 .. _APT packages: https://packages.ubuntu.com/
 
@@ -342,8 +344,13 @@ List of `APT packages`_ to install.
 
    build:
      apt_packages:
-       - mysql-client
-       - cmatrix
+       - libclang
+       - cmake
+
+.. note::
+
+   When possible avoid installing Python packages using apt (``python3-numpy`` for example),
+   :ref:`use pip or Conda instead <guides/reproducible-builds:pinning dependencies>`.
 
 sphinx
 ~~~~~~

docs/guides/reproducible-builds.rst

@@ -173,6 +173,6 @@ or our Conda docs about :ref:`environment files <guides/conda:creating the \`\`e
 .. tip::
 
    Remember to update your docs' dependencies from time to time to get new improvements and fixes.
-   It also makes it easy to manage in case a version reaches it's end of support date.
+   It also makes it easy to manage in case a version reaches its end of support date.
 
    .. TODO: link to the supported versions policy.

readthedocs/config/config.py

@@ -122,8 +122,14 @@ def __init__(self, key, code, error_message, source_file=None):
         super().__init__(message, code=code)
 
     def _get_display_key(self):
-        # Checks for patterns similar to `python.install.0.requirements`
-        # if matched change to `python.install[0].requirements` using backreference.
+        """
+        Display keys in a more friendly format.
+
+        Indexes are displayed like ``n``,
+        but users may be more familiar with the ``[n]`` syntax.
+        For example ``python.install.0.requirements``
+        is changed to `python.install[0].requirements`.
+        """
         return re.sub(
             r'^([a-zA-Z_.-]+)\.(\d+)([a-zA-Z_.-]*)$',
             r'\1[\2]\3',
@@ -625,10 +631,7 @@ def conda(self):
     @property
     def build(self):
         """The docker image used by the builders."""
-        return Build(
-            apt_packages=[],
-            **self._config['build'],
-        )
+        return Build(**self._config['build'])
 
     @property
     def doctype(self):
@@ -777,18 +780,19 @@ def validate_apt_package(self, index):
         """
         Validate the package name to avoid injections of extra options.
 
-        Packages names can be a regex pattern.
-        We just validate that they aren't interpreted as an option or file.
+        We validate that they aren't interpreted as an option or file.
+        See https://manpages.ubuntu.com/manpages/xenial/man8/apt-get.8.html
+        and https://www.debian.org/doc/manuals/debian-reference/ch02.en.html#_debian_package_file_names  # noqa
+        for allowed chars in packages names.
         """
         key = f'build.apt_packages.{index}'
         package = self.pop_config(key)
         with self.catch_validation_error(key):
             validate_string(package)
             package = package.strip()
             invalid_starts = [
-                # Don't allow to inject extra options.
+                # Don't allow extra options.
                 '-',
-                '\\',
                 # Don't allow to install from a path.
                 '/',
                 '.',
@@ -799,10 +803,18 @@ def validate_apt_package(self, index):
                         key=key,
                         message=(
                             'Invalid package name. '
-                            f'Package can\'t start with {start}',
+                            f'Package can\'t start with {start}.',
                         ),
                         code=INVALID_NAME,
                     )
+            # List of valid chars in packages names.
+            pattern = re.compile(r'^[a-zA-Z0-9]+[a-zA-Z0-9.+-]*$')
+            if not pattern.match(package):
+                self.error(
+                    key=key,
+                    message='Invalid package name.',
+                    code=INVALID_NAME,
+                )
         return package
 
     def validate_python(self):

readthedocs/config/models.py

@@ -30,6 +30,10 @@ class Build(Base):
 
     __slots__ = ('image', 'apt_packages')
 
+    def __init__(self, **kwargs):
+        kwargs.setdefault('apt_packages', [])
+        super().__init__(**kwargs)
+
 
 class Python(Base):
 

readthedocs/config/tests/test_config.py

@@ -942,8 +942,7 @@ def test_build_image_check_invalid_type(self, value):
         [
             [],
             ['cmatrix'],
-            ['mysql', 'cmatrix', 'postgresql-dev'],
-            ['mysql', 'cmatrix', 'postgresql=1.2.3'],
+            ['Mysql', 'cmatrix', 'postgresql-dev'],
         ],
     )
     def test_build_apt_packages_check_valid(self, value):
@@ -976,6 +975,15 @@ def test_build_apt_packages_invalid_type(self, value):
             (2, ['cmatrix', 'quiet', ' /home/user/package.deb']),
             (2, ['cmatrix', 'quiet', '../package.deb']),
             (2, ['cmatrix', 'quiet', ' ../package.deb']),
+            (1, ['one', '$two']),
+            (1, ['one', 'non-ascíí']),
+            # We don't allow regex for now.
+            (1, ['mysql', 'cmatrix$']),
+            (0, ['^mysql-*', 'cmatrix$']),
+            # We don't allow specifying versions for now.
+            (0, ['postgresql=1.2.3']),
+            # We don't allow specifying distributions for now.
+            (0, ['cmatrix/bionic']),
         ],
     )
     def test_build_apt_packages_invalid_value(self, error_index, value):

readthedocs/projects/tasks.py

@@ -1182,15 +1182,27 @@ def setup_python_environment(self):
             self.python_env.list_packages_installed()
 
     def install_system_dependencies(self):
+        """
+        Install apt packages from the config file.
+
+        We don't allow to pass custom options or install from a path.
+        The packages names are already validated when reading the config file.
+
+        .. note::
+
+           ``--quiet`` won't suppress the output,
+           it would just remove the progress bar.
+        """
         packages = self.config.build.apt_packages
         if packages:
             self.build_env.run(
-                'apt-get', 'update', '-y', '-q',
-                user=settings.RTD_BUILD_SUPER_USER,
+                'apt-get', 'update', '--assume-yes', '--quiet',
+                user=settings.RTD_DOCKER_SUPER_USER,
             )
+            # put ``--`` to end all command arguments.
             self.build_env.run(
-                'apt-get', 'install', '-y', '-q', *packages,
-                user=settings.RTD_BUILD_SUPER_USER,
+                'apt-get', 'install', '--assume-yes', '--quiet', '--', *packages,
+                user=settings.RTD_DOCKER_SUPER_USER,
             )
 
     def build_docs(self):

readthedocs/rtd_tests/tests/test_celery.py

@@ -477,7 +477,6 @@ def test_send_build_status_no_remote_repo_or_social_account_gitlab(self, send_bu
 
     @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.setup_python_environment', new=MagicMock)
     @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.build_docs', new=MagicMock)
-    @patch('readthedocs.doc_builder.environments.BuildEnvironment.update_build', new=MagicMock)
     @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.setup_vcs', new=MagicMock)
     @patch.object(BuildEnvironment, 'run')
     @patch('readthedocs.doc_builder.config.load_config')
@@ -521,8 +520,8 @@ def test_install_apt_packages(self, load_config, run):
             mock.call(
                 'apt-get',
                 'update',
-                '-y',
-                '-q',
+                '--assume-yes',
+                '--quiet',
                 user='root:root',
             )
         )
@@ -531,8 +530,9 @@ def test_install_apt_packages(self, load_config, run):
             mock.call(
                 'apt-get',
                 'install',
-                '-y',
-                '-q',
+                '--assume-yes',
+                '--quiet',
+                '--',
                 'clangd',
                 'cmatrix',
                 user='root:root',

readthedocs/settings/base.py

@@ -433,7 +433,7 @@ def TEMPLATES(self):
     # instance to avoid file permissions issues.
     # https://docs.docker.com/engine/reference/run/#user
     RTD_DOCKER_USER = 'docs:docs'
-    RTD_BUILD_SUPER_USER = 'root:root'
+    RTD_DOCKER_SUPER_USER = 'root:root'
 
     RTD_DOCKER_COMPOSE = False