From d373487a7d33467280f040804e1e2c155a70da29 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 28 Feb 2019 19:37:53 -0500 Subject: [PATCH 1/2] Use relative paths in config module Closes #5364 --- readthedocs/config/tests/test_config.py | 41 +++++++++---------- readthedocs/config/tests/test_validation.py | 7 +--- readthedocs/config/validation.py | 21 ++++++---- readthedocs/doc_builder/backends/mkdocs.py | 5 +++ readthedocs/doc_builder/backends/sphinx.py | 5 +++ .../doc_builder/python_environments.py | 19 ++++----- .../tests/test_config_integration.py | 10 ++--- 7 files changed, 55 insertions(+), 53 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 3ad5cfe8a9d..5a2db4583d2 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- import os import re import textwrap @@ -571,7 +570,7 @@ def test_validates_conda_file(tmpdir): ) build.validate() assert isinstance(build.conda, Conda) - assert build.conda.environment == str(tmpdir.join('environment.yml')) + assert build.conda.environment == 'environment.yml' def test_file_is_required_when_using_conda(tmpdir): @@ -607,7 +606,7 @@ def test_requirements_file_repects_default_value(tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].requirements == str(tmpdir.join('myrequirements.txt')) + assert install[0].requirements == 'myrequirements.txt' def test_requirements_file_respects_configuration(tmpdir): @@ -619,7 +618,7 @@ def test_requirements_file_respects_configuration(tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].requirements == str(tmpdir.join('requirements.txt')) + assert install[0].requirements == 'requirements.txt' def test_requirements_file_is_null(tmpdir): @@ -711,7 +710,7 @@ def test_as_dict(tmpdir): 'python': { 'version': 3.5, 'install': [{ - 'requirements': str(tmpdir.join('requirements.txt')), + 'requirements': 'requirements.txt', }], 'use_system_site_packages': False, }, @@ -826,7 +825,7 @@ def test_conda_check_valid(self, tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.conda.environment == str(tmpdir.join('environment.yml')) + assert build.conda.environment == 'environment.yml' def test_conda_check_invalid(self, tmpdir): apply_fs(tmpdir, {'environment.yml': ''}) @@ -1021,7 +1020,7 @@ def test_python_install_check_default(self, tmpdir): install = build.python.install assert len(install) == 1 assert isinstance(install[0], PythonInstall) - assert install[0].path == str(tmpdir) + assert install[0].path == '.' assert install[0].method == PIP assert install[0].extra_requirements == [] @@ -1074,7 +1073,7 @@ def test_python_install_requirements_check_valid(self, tmpdir): install = build.python.install assert len(install) == 1 assert isinstance(install[0], PythonInstallRequirements) - assert install[0].requirements == str(tmpdir.join('requirements.txt')) + assert install[0].requirements == 'requirements.txt' def test_python_install_requirements_check_invalid(self, tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) @@ -1154,7 +1153,7 @@ def test_python_install_requirements_priority_over_default(self, tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].requirements == str(tmpdir.join('requirements.txt')) + assert install[0].requirements == 'requirements.txt' @pytest.mark.parametrize('value', [3, [], {}]) def test_python_install_requirements_check_invalid_types(self, value, tmpdir): @@ -1204,7 +1203,7 @@ def test_python_install_pip_check_valid(self, tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].path == str(tmpdir) + assert install[0].path == '.' assert install[0].method == PIP def test_python_install_pip_have_priority_over_default(self, tmpdir): @@ -1223,7 +1222,7 @@ def test_python_install_pip_have_priority_over_default(self, tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].path == str(tmpdir) + assert install[0].path == '.' assert install[0].method == PIP def test_python_install_setuptools_check_valid(self, tmpdir): @@ -1241,7 +1240,7 @@ def test_python_install_setuptools_check_valid(self, tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].path == str(tmpdir) + assert install[0].path == '.' assert install[0].method == SETUPTOOLS def test_python_install_setuptools_ignores_default(self): @@ -1268,7 +1267,7 @@ def test_python_install_setuptools_priority_over_default(self, tmpdir): build.validate() install = build.python.install assert len(install) == 1 - assert install[0].path == str(tmpdir) + assert install[0].path == '.' assert install[0].method == SETUPTOOLS def test_python_install_allow_empty_list(self): @@ -1386,14 +1385,14 @@ def test_python_install_several_respects_order(self, tmpdir): install = build.python.install assert len(install) == 3 - assert install[0].path == str(tmpdir.join('one')) + assert install[0].path == 'one' assert install[0].method == PIP assert install[0].extra_requirements == [] - assert install[1].path == str(tmpdir.join('two')) + assert install[1].path == 'two' assert install[1].method == SETUPTOOLS - assert install[2].requirements == str(tmpdir.join('three.txt')) + assert install[2].requirements == 'three.txt' def test_python_install_reports_correct_invalid_index(self, tmpdir): apply_fs(tmpdir, { @@ -1531,7 +1530,7 @@ def test_sphinx_configuration_check_valid(self, tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.sphinx.configuration == str(tmpdir.join('conf.py')) + assert build.sphinx.configuration == 'conf.py' def test_sphinx_configuration_check_invalid(self, tmpdir): apply_fs(tmpdir, {'conf.py': ''}) @@ -1574,7 +1573,7 @@ def test_sphinx_configuration_respects_default(self, tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.sphinx.configuration == str(tmpdir.join('conf.py')) + assert build.sphinx.configuration == 'conf.py' def test_sphinx_configuration_default_can_be_none(self, tmpdir): apply_fs(tmpdir, {'conf.py': ''}) @@ -1594,7 +1593,7 @@ def test_sphinx_configuration_priorities_over_default(self, tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.sphinx.configuration == str(tmpdir.join('conf.py')) + assert build.sphinx.configuration == 'conf.py' @pytest.mark.parametrize('value', [[], True, 0, {}]) def test_sphinx_configuration_validate_type(self, value): @@ -1641,7 +1640,7 @@ def test_mkdocs_configuration_check_valid(self, tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.mkdocs.configuration == str(tmpdir.join('mkdocs.yml')) + assert build.mkdocs.configuration == 'mkdocs.yml' assert build.doctype == 'mkdocs' assert build.sphinx is None @@ -1999,7 +1998,7 @@ def test_as_dict(self, tmpdir): 'python': { 'version': 3.6, 'install': [{ - 'requirements': str(tmpdir.join('requirements.txt')), + 'requirements': 'requirements.txt', }], 'use_system_site_packages': False, }, diff --git a/readthedocs/config/tests/test_validation.py b/readthedocs/config/tests/test_validation.py index 8a0e98b98c4..fa5ad793d4e 100644 --- a/readthedocs/config/tests/test_validation.py +++ b/readthedocs/config/tests/test_validation.py @@ -1,6 +1,3 @@ -# -*- coding: utf-8 -*- -import os - from mock import patch from pytest import raises @@ -133,10 +130,10 @@ def test_it_accepts_absolute_path(self, tmpdir): path = str(tmpdir.mkdir('a directory')) validate_path(path, 'does not matter') - def test_it_returns_absolute_path(self, tmpdir): + def test_it_returns_relative_path(self, tmpdir): tmpdir.mkdir('a directory') path = validate_path('a directory', str(tmpdir)) - assert path == os.path.abspath(path) + assert path == 'a directory' def test_it_only_accepts_strings(self): with raises(ValidationError) as excinfo: diff --git a/readthedocs/config/validation.py b/readthedocs/config/validation.py index 5d7651dffd8..b1c5ff507fc 100644 --- a/readthedocs/config/validation.py +++ b/readthedocs/config/validation.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - """Validations for the RTD configuration file.""" import os @@ -90,28 +88,33 @@ def validate_bool(value): def validate_directory(value, base_path): """Check that ``value`` is a directory.""" - path = validate_path(value, base_path) + path = os.path.join( + base_path, + validate_path(value, base_path) + ) if not os.path.isdir(path): raise ValidationError(value, INVALID_DIRECTORY) - return path + return os.path.relpath(path, base_path) def validate_file(value, base_path): """Check that ``value`` is a file.""" - path = validate_path(value, base_path) + path = os.path.join( + base_path, + validate_path(value, base_path) + ) if not os.path.isfile(path): raise ValidationError(value, INVALID_FILE) - return path + return os.path.relpath(path, base_path) def validate_path(value, base_path): """Check that ``value`` is an existent file in ``base_path``.""" string_value = validate_string(value) pathed_value = os.path.join(base_path, string_value) - final_value = os.path.abspath(pathed_value) - if not os.path.exists(final_value): + if not os.path.exists(pathed_value): raise ValidationError(value, INVALID_PATH) - return final_value + return os.path.relpath(pathed_value, base_path) def validate_string(value): diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index c25c88d470a..1960cac2d14 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -78,6 +78,11 @@ def get_yaml_config(self): ) if not os.path.exists(mkdoc_path): return None + else: + mkdoc_path = os.path.join( + self.project.checkout_path(self.version.slug), + self.config.mkdocs.configuration + ) return mkdoc_path def load_yaml_config(self): diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index c5a664835fa..a06fbf3e022 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -43,6 +43,11 @@ def __init__(self, *args, **kwargs): try: if not self.config_file: self.config_file = self.project.conf_file(self.version.slug) + else: + self.config_file = os.path.join( + self.project.checkout_path(self.version.slug), + self.config_file, + ) self.old_artifact_path = os.path.join( os.path.dirname(self.config_file), self.sphinx_build_dir, diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index e117bbcca15..1ad11341dec 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - """An abstraction over virtualenv and Conda environments.""" import copy @@ -9,8 +7,6 @@ import os import shutil -from django.conf import settings - from readthedocs.config import PIP, SETUPTOOLS from readthedocs.config.models import PythonInstall, PythonInstallRequirements from readthedocs.doc_builder.config import load_yaml_config @@ -83,11 +79,10 @@ def install_package(self, install): :param install: A install object from the config module. :type install: readthedocs.config.models.PythonInstall """ - rel_path = os.path.relpath(install.path, self.checkout_path) if install.method == PIP: # Prefix ./ so pip installs from a local path rather than pypi local_path = ( - os.path.join('.', rel_path) if rel_path != '.' else rel_path + os.path.join('.', install.path) if install.path != '.' else install.path ) extra_req_param = '' if install.extra_requirements: @@ -112,7 +107,7 @@ def install_package(self, install): elif install.method == SETUPTOOLS: self.build_env.run( self.venv_bin(filename='python'), - os.path.join(rel_path, 'setup.py'), + os.path.join(install.path, 'setup.py'), 'install', '--force', cwd=self.checkout_path, @@ -346,7 +341,10 @@ def install_requirements_file(self, install): for path, req_file in itertools.product(paths, req_files): test_path = os.path.join(self.checkout_path, path, req_file) if os.path.exists(test_path): - requirements_file_path = test_path + requirements_file_path = os.path.relpath( + test_path, + self.checkout_path, + ) break if requirements_file_path: @@ -363,10 +361,7 @@ def install_requirements_file(self, install): '--cache-dir', self.project.pip_cache_path, '-r', - os.path.relpath( - requirements_file_path, - self.checkout_path - ), + requirements_file_path, ] self.build_env.run( *args, diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index a595b891b89..7d5e8c72429 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -277,7 +277,7 @@ def test_conda_with_cofig(self, checkout_path): ) config = load_yaml_config(self.version) self.assertTrue(config.conda is not None) - self.assertEqual(config.conda.environment, full_conda_file) + self.assertEqual(config.conda.environment, conda_file) @mock.patch('readthedocs.projects.models.Project.checkout_path') def test_conda_without_cofig(self, checkout_path): @@ -303,7 +303,7 @@ def test_requirements_file_from_project_setting(self, checkout_path): self.assertEqual(len(config.python.install), 1) self.assertEqual( config.python.install[0].requirements, - full_requirements_file + requirements_file ) @mock.patch('readthedocs.projects.models.Project.checkout_path') @@ -328,7 +328,7 @@ def test_requirements_file_from_yml(self, checkout_path): self.assertEqual(len(config.python.install), 1) self.assertEqual( config.python.install[0].requirements, - full_requirements_file + requirements_file ) @@ -459,7 +459,6 @@ def test_conda_environment(self, build_failed, checkout_path, tmpdir): update_docs = self.get_update_docs_task() update_docs.run_build(docker=False, record=False) - conda_file = path.join(str(base_path), conda_file) assert update_docs.config.conda.environment == conda_file assert isinstance(update_docs.python_env, Conda) @@ -530,11 +529,10 @@ def test_python_install_requirements(self, run, checkout_path, tmpdir): update_docs.python_env.install_requirements() args, kwargs = run.call_args - full_requirements_file = str(base_path.join(requirements_file)) install = config.python.install assert len(install) == 1 - assert install[0].requirements == full_requirements_file + assert install[0].requirements == requirements_file assert requirements_file in args @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') From 186e2682fff98af8207b03c664acc88fb8d34edc Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 15 Apr 2019 10:43:51 -0500 Subject: [PATCH 2/2] Fix test --- readthedocs/rtd_tests/tests/test_config_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 7d5e8c72429..0d700c47ac5 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -952,7 +952,7 @@ def test_mkdocs_configuration( args, kwargs = run.call_args assert '--config-file' in args - assert path.join(str(tmpdir), 'docx/mkdocs.yml') in args + assert 'docx/mkdocs.yml' in args append_conf.assert_called_once() move.assert_called_once()