Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use relative paths in config module #5377

Merged
merged 4 commits into from
May 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 20 additions & 21 deletions readthedocs/config/tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# -*- coding: utf-8 -*-
import os
import re
import textwrap
Expand Down Expand Up @@ -604,7 +603,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):
Expand Down Expand Up @@ -640,7 +639,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):
Expand All @@ -652,7 +651,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):
Expand Down Expand Up @@ -744,7 +743,7 @@ def test_as_dict(tmpdir):
'python': {
'version': 3.7,
'install': [{
'requirements': str(tmpdir.join('requirements.txt')),
'requirements': 'requirements.txt',
}],
'use_system_site_packages': False,
},
Expand Down Expand Up @@ -859,7 +858,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': ''})
Expand Down Expand Up @@ -1054,7 +1053,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 == []

Expand Down Expand Up @@ -1107,7 +1106,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': ''})
Expand Down Expand Up @@ -1187,7 +1186,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):
Expand Down Expand Up @@ -1237,7 +1236,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):
Expand All @@ -1256,7 +1255,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):
Expand All @@ -1274,7 +1273,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):
Expand All @@ -1301,7 +1300,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):
Expand Down Expand Up @@ -1419,14 +1418,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, {
Expand Down Expand Up @@ -1564,7 +1563,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': ''})
Expand Down Expand Up @@ -1607,7 +1606,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': ''})
Expand All @@ -1627,7 +1626,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):
Expand Down Expand Up @@ -1674,7 +1673,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

Expand Down Expand Up @@ -2032,7 +2031,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,
},
Expand Down
7 changes: 2 additions & 5 deletions readthedocs/config/tests/test_validation.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# -*- coding: utf-8 -*-
import os

from mock import patch
from pytest import raises

Expand Down Expand Up @@ -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:
Expand Down
21 changes: 12 additions & 9 deletions readthedocs/config/validation.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
# -*- coding: utf-8 -*-

"""Validations for the RTD configuration file."""
import os

Expand Down Expand Up @@ -81,28 +79,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):
Expand Down
5 changes: 5 additions & 0 deletions readthedocs/doc_builder/backends/sphinx.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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,
Expand Down
15 changes: 7 additions & 8 deletions readthedocs/doc_builder/python_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,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:
Expand All @@ -111,7 +110,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,
Expand Down Expand Up @@ -364,7 +363,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:
Expand All @@ -381,10 +383,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,
Expand Down
12 changes: 5 additions & 7 deletions readthedocs/rtd_tests/tests/test_config_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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')
Expand All @@ -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
)


Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -954,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()

Expand Down