From 9d018b03ecc0fd36f6b4c09536b5b13347ad7632 Mon Sep 17 00:00:00 2001 From: Niklas Rosenstein Date: Tue, 14 Jul 2020 00:13:53 +0200 Subject: [PATCH 01/10] add support for "packages" install key in v2 config --- docs/config-file/v2.rst | 23 ++++++++++++ readthedocs/config/config.py | 13 +++++++ readthedocs/config/models.py | 5 +++ readthedocs/config/tests/test_config.py | 21 +++++++++++ .../doc_builder/python_environments.py | 35 ++++++++++++++++++- 5 files changed, 96 insertions(+), 1 deletion(-) diff --git a/docs/config-file/v2.rst b/docs/config-file/v2.rst index 18487831589..d7d0f677bfd 100644 --- a/docs/config-file/v2.rst +++ b/docs/config-file/v2.rst @@ -175,6 +175,29 @@ Example: manage the build, this setting will not have any effect. Instead add the extra requirements to the ``environment`` file of Conda. +Packages list +''''''''''''' + +Install packages from a list of requirements. This allows you to embed requirements in the +``.readthedocs.yml`` configuration directly. + +:Key: `packages` +:Type: ``string`` +:Required: ``true`` + +Example: + +.. code-block:: yaml + + version: 2 + + python: + version: 3.7 + install: + - packages: + - mkdocs-material + - Pygments + Packages '''''''' diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 244135277eb..61ed3c146c4 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -19,6 +19,7 @@ Python, PythonInstall, PythonInstallRequirements, + PythonInstallPackages, Search, Sphinx, Submodules, @@ -833,6 +834,16 @@ def validate_python_install(self, index): self.base_path, ) python_install['requirements'] = requirements + elif 'packages' in raw_install: + packages_key = key + '.packages' + packages = self.pop_config(packages_key) + if not isinstance(packages, list): + self.error( + packages_key, + '"{}" key must be a list'.format(packages_key), + code=PYTHON_INVALID, + ) + python_install['packages'] = packages elif 'path' in raw_install: path_key = key + '.path' with self.catch_validation_error(path_key): @@ -1112,6 +1123,8 @@ def python(self): for install in python['install']: if 'requirements' in install: python_install.append(PythonInstallRequirements(**install),) + elif 'packages' in install: + python_install.append(PythonInstallPackages(**install),) elif 'path' in install: python_install.append(PythonInstall(**install),) return Python( diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index 55932e991d4..8f9dce9fe91 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -41,6 +41,11 @@ class PythonInstallRequirements(Base): __slots__ = ('requirements',) +class PythonInstallPackages(Base): + + __slots__ = ('packages',) + + class PythonInstall(Base): __slots__ = ( diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 3ee20d1653c..512f53606ee 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -33,6 +33,7 @@ Conda, PythonInstall, PythonInstallRequirements, + PythonInstallPackages, ) from readthedocs.config.validation import ( INVALID_BOOL, @@ -1104,6 +1105,26 @@ def test_python_install_requirements_check_valid(self, tmpdir): assert isinstance(install[0], PythonInstallRequirements) assert install[0].requirements == 'requirements.txt' + def test_python_install_requires_check_valid(self, tmpdir): + build = self.get_build_config( + { + 'python': { + 'install': [{ + 'packages': [ + 'package_a', + 'package_b >=3.0.0,<4.0.0', + ], + }], + }, + }, + source_file=str(tmpdir.join('readthedocs.yml')), + ) + build.validate() + install = build.python.install + assert len(install) == 1 + assert isinstance(install[0], PythonInstallPackages) + assert install[0].packages == ['package_a', 'package_b >=3.0.0,<4.0.0'] + def test_python_install_requirements_does_not_allow_null(self, tmpdir): build = self.get_build_config( { diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index d165ab2213d..1dc4bdabd5c 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -14,7 +14,11 @@ from readthedocs.builds.constants import EXTERNAL from readthedocs.config import PIP, SETUPTOOLS, ParseError, parse as parse_yaml -from readthedocs.config.models import PythonInstall, PythonInstallRequirements +from readthedocs.config.models import ( + PythonInstall, + PythonInstallRequirements, + PythonInstallPackages, +) from readthedocs.doc_builder.config import load_yaml_config from readthedocs.doc_builder.constants import DOCKER_IMAGE from readthedocs.doc_builder.environments import DockerBuildEnvironment @@ -77,6 +81,8 @@ def install_requirements(self): for install in self.config.python.install: if isinstance(install, PythonInstallRequirements): self.install_requirements_file(install) + if isinstance(install, PythonInstallPackages): + self.install_packages_list(install) if isinstance(install, PythonInstall): self.install_package(install) @@ -430,6 +436,33 @@ def install_requirements_file(self, install): bin_path=self.venv_bin(), ) + def install_packages_list(self, install): + """ + Install requirements from a string specification using pip. + + :param install: A instal object from the config module. + :type install: readthedocs.config.modules.PythonInstallPackages + """ + + args = [ + self.venv_bin(filename='python'), + '-m', + 'pip', + 'install', + ] + if self.project.has_feature(Feature.PIP_ALWAYS_UPGRADE): + args += ['--upgrade'] + args += [ + '--exists-action=w', + *self._pip_cache_cmd_argument(), + ] + args += install.requirements + self.build_env.run( + *args, + cwd=self.checkout_path, + bin_path=self.venv_bin(), + ) + def list_packages_installed(self): """List packages installed in pip.""" args = [ From 7e57736f2afca55b1fc571cc2e7699eefacae91d Mon Sep 17 00:00:00 2001 From: Niklas Rosenstein Date: Tue, 14 Jul 2020 00:17:29 +0200 Subject: [PATCH 02/10] fix indentation of samle code --- docs/config-file/v2.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/config-file/v2.rst b/docs/config-file/v2.rst index d7d0f677bfd..69be421e361 100644 --- a/docs/config-file/v2.rst +++ b/docs/config-file/v2.rst @@ -192,11 +192,11 @@ Example: version: 2 python: - version: 3.7 - install: - - packages: - - mkdocs-material - - Pygments + version: 3.7 + install: + - packages: + - mkdocs-material + - Pygments Packages '''''''' From 4582e589eee6fcefdeee62fac4c5c2b7f876eaf1 Mon Sep 17 00:00:00 2001 From: Niklas Rosenstein Date: Tue, 14 Jul 2020 00:18:24 +0200 Subject: [PATCH 03/10] fix test case name --- readthedocs/config/tests/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 512f53606ee..55650616aaa 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1105,7 +1105,7 @@ def test_python_install_requirements_check_valid(self, tmpdir): assert isinstance(install[0], PythonInstallRequirements) assert install[0].requirements == 'requirements.txt' - def test_python_install_requires_check_valid(self, tmpdir): + def test_python_install_packages_check_valid(self, tmpdir): build = self.get_build_config( { 'python': { From 2f9c98bd53cc0d0efd1737f6eb8660498d0eca75 Mon Sep 17 00:00:00 2001 From: Niklas Rosenstein Date: Tue, 14 Jul 2020 00:20:20 +0200 Subject: [PATCH 04/10] fix attr access in install_packages_list() (artifact from previous PR) --- readthedocs/doc_builder/python_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 1dc4bdabd5c..83493d726bf 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -456,7 +456,7 @@ def install_packages_list(self, install): '--exists-action=w', *self._pip_cache_cmd_argument(), ] - args += install.requirements + args += install.packages self.build_env.run( *args, cwd=self.checkout_path, From 899667e688fc90bbed497cd65d00456e54e0fb83 Mon Sep 17 00:00:00 2001 From: NiklasRosenstein Date: Tue, 4 Aug 2020 10:20:11 +0200 Subject: [PATCH 05/10] Update docs/config-file/v2.rst Co-authored-by: Santos Gallegos --- docs/config-file/v2.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/config-file/v2.rst b/docs/config-file/v2.rst index 69be421e361..ac540f37991 100644 --- a/docs/config-file/v2.rst +++ b/docs/config-file/v2.rst @@ -178,8 +178,8 @@ Example: Packages list ''''''''''''' -Install packages from a list of requirements. This allows you to embed requirements in the -``.readthedocs.yml`` configuration directly. +Install packages from a list of requirements. +This allows you to declare dependencies in the ``.readthedocs.yml`` file instead of creating a separate file. :Key: `packages` :Type: ``string`` From bf773e8d8393d64a88c5768c09eeeb831202ada4 Mon Sep 17 00:00:00 2001 From: NiklasRosenstein Date: Tue, 4 Aug 2020 10:20:19 +0200 Subject: [PATCH 06/10] Update docs/config-file/v2.rst Co-authored-by: Santos Gallegos --- docs/config-file/v2.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/config-file/v2.rst b/docs/config-file/v2.rst index ac540f37991..1ee4310b2db 100644 --- a/docs/config-file/v2.rst +++ b/docs/config-file/v2.rst @@ -182,7 +182,8 @@ Install packages from a list of requirements. This allows you to declare dependencies in the ``.readthedocs.yml`` file instead of creating a separate file. :Key: `packages` -:Type: ``string`` +:Type: ``list`` +:Default: ``[]`` :Required: ``true`` Example: From d9063664e3359d1e4e40df775921957da775556e Mon Sep 17 00:00:00 2001 From: NiklasRosenstein Date: Tue, 4 Aug 2020 10:20:33 +0200 Subject: [PATCH 07/10] Update readthedocs/doc_builder/python_environments.py Co-authored-by: Santos Gallegos --- readthedocs/doc_builder/python_environments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 83493d726bf..c8428fc83ae 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -438,7 +438,7 @@ def install_requirements_file(self, install): def install_packages_list(self, install): """ - Install requirements from a string specification using pip. + Install requirements from a list of strings using pip. :param install: A instal object from the config module. :type install: readthedocs.config.modules.PythonInstallPackages From c776387133c77beec7a3478f94fb92ea8b2d0643 Mon Sep 17 00:00:00 2001 From: NiklasRosenstein Date: Tue, 4 Aug 2020 10:24:11 +0200 Subject: [PATCH 08/10] Update v2.rst --- docs/config-file/v2.rst | 54 +++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/docs/config-file/v2.rst b/docs/config-file/v2.rst index 1ee4310b2db..90b710d6d4f 100644 --- a/docs/config-file/v2.rst +++ b/docs/config-file/v2.rst @@ -175,30 +175,6 @@ Example: manage the build, this setting will not have any effect. Instead add the extra requirements to the ``environment`` file of Conda. -Packages list -''''''''''''' - -Install packages from a list of requirements. -This allows you to declare dependencies in the ``.readthedocs.yml`` file instead of creating a separate file. - -:Key: `packages` -:Type: ``list`` -:Default: ``[]`` -:Required: ``true`` - -Example: - -.. code-block:: yaml - - version: 2 - - python: - version: 3.7 - install: - - packages: - - mkdocs-material - - Pygments - Packages '''''''' @@ -252,6 +228,36 @@ With the previous settings, Read the Docs will execute the next commands: pip install .[docs] python package/setup.py install +Extra dependencies +'''''''''''''''''' + +Install packages from a list of requirements. +This allows you to declare dependencies in the ``.readthedocs.yml`` file instead of creating a separate file. + +:Key: `packages` +:Type: ``list`` +:Default: ``[]`` +:Required: ``true`` + +Example: + +.. code-block:: yaml + + version: 2 + + python: + version: 3.7 + install: + - packages: + - mkdocs-material + - Pygments + +With the previous settings, Read the Docs will execute the next commands: + +.. prompt:: bash $ + + pip install mkdocs-material Pygments + python.system_packages `````````````````````` From 336207291fce32dfba2a732e12ff8ea2ac9e0252 Mon Sep 17 00:00:00 2001 From: NiklasRosenstein Date: Tue, 4 Aug 2020 18:58:00 +0200 Subject: [PATCH 09/10] Update docs/config-file/v2.rst Co-authored-by: Santos Gallegos --- docs/config-file/v2.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/config-file/v2.rst b/docs/config-file/v2.rst index 90b710d6d4f..510b702ecbc 100644 --- a/docs/config-file/v2.rst +++ b/docs/config-file/v2.rst @@ -237,7 +237,6 @@ This allows you to declare dependencies in the ``.readthedocs.yml`` file instead :Key: `packages` :Type: ``list`` :Default: ``[]`` -:Required: ``true`` Example: From 0a99cbbf2a6945282b81a9cc5b7217573c83ad98 Mon Sep 17 00:00:00 2001 From: Niklas Rosenstein Date: Fri, 4 Sep 2020 14:17:09 +0200 Subject: [PATCH 10/10] ensure that "packages" is not empty, add unit tests for invalid configuration --- readthedocs/config/config.py | 6 +++++ readthedocs/config/tests/test_config.py | 32 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 61ed3c146c4..d3f1dff88b7 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -843,6 +843,12 @@ def validate_python_install(self, index): '"{}" key must be a list'.format(packages_key), code=PYTHON_INVALID, ) + if not packages: + self.error( + packages_key, + '"{}" cannot be empty'.format(packages_key), + code=PYTHON_INVALID, + ) python_install['packages'] = packages elif 'path' in raw_install: path_key = key + '.path' diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 55650616aaa..fe2474c2a46 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1125,6 +1125,38 @@ def test_python_install_packages_check_valid(self, tmpdir): assert isinstance(install[0], PythonInstallPackages) assert install[0].packages == ['package_a', 'package_b >=3.0.0,<4.0.0'] + def test_python_install_packages_must_be_list(self, tmpdir): + build = self.get_build_config( + { + 'python': { + 'install': [{ + 'packages': {}, + }], + }, + }, + source_file=str(tmpdir.join('readthedocs.yml')), + ) + with raises(InvalidConfig) as excinfo: + build.validate() + assert excinfo.value.key == 'python.install.0.packages' + assert '"python.install.0.packages" key must be a list' in str(excinfo.value) + + def test_python_install_packages_must_not_be_empty(self, tmpdir): + build = self.get_build_config( + { + 'python': { + 'install': [{ + 'packages': [], + }], + }, + }, + source_file=str(tmpdir.join('readthedocs.yml')), + ) + with raises(InvalidConfig) as excinfo: + build.validate() + assert excinfo.value.key == 'python.install.0.packages' + assert '"python.install.0.packages" cannot be empty' in str(excinfo.value) + def test_python_install_requirements_does_not_allow_null(self, tmpdir): build = self.get_build_config( {