diff --git a/readthedocs/doc_builder/config.py b/readthedocs/doc_builder/config.py index 2ce1db0ad9c..ae0fd84b99e 100644 --- a/readthedocs/doc_builder/config.py +++ b/readthedocs/doc_builder/config.py @@ -51,16 +51,7 @@ def extra_requirements(self): @property def python_interpreter(self): - ver = self.python_version - if ver in [2, 3]: - # Get the highest version of the major series version if user only - # gave us a version of '2', or '3' - ver = max( - list( - filter( - lambda x: x < ver + 1, - self._yaml_config.get_valid_python_versions(), - ))) + ver = self.python_full_version return 'python{0}'.format(ver) @property @@ -75,6 +66,20 @@ def python_version(self): version = 3 return version + @property + def python_full_version(self): + ver = self.python_version + if ver in [2, 3]: + # Get the highest version of the major series version if user only + # gave us a version of '2', or '3' + ver = max( + list( + filter( + lambda x: x < ver + 1, + self._yaml_config.get_valid_python_versions(), + ))) + return ver + @property def use_system_site_packages(self): if 'use_system_site_packages' in self._yaml_config.get('python', {}): diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 74879ac4ce7..b64c8bbce35 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -1,14 +1,19 @@ +# -*- coding: utf-8 -*- """An abstraction over virtualenv and Conda environments.""" -from __future__ import absolute_import -from builtins import object +from __future__ import ( + absolute_import, division, print_function, unicode_literals) + +import json import logging import os import shutil +from builtins import object, open from django.conf import settings from readthedocs.doc_builder.config import ConfigWrapper +from readthedocs.doc_builder.constants import DOCKER_IMAGE from readthedocs.doc_builder.loader import get_builder_class from readthedocs.projects.constants import LOG_TEMPLATE from readthedocs.projects.models import Feature @@ -38,7 +43,6 @@ def _log(self, msg): msg=msg)) def delete_existing_build_dir(self): - # Handle deleting old build dir build_dir = os.path.join( self.venv_path(), @@ -47,6 +51,13 @@ def delete_existing_build_dir(self): self._log('Removing existing build directory') shutil.rmtree(build_dir) + def delete_existing_venv_dir(self): + venv_dir = self.venv_path() + # Handle deleting old venv dir + if os.path.exists(venv_dir): + self._log('Removing existing venv directory') + shutil.rmtree(venv_dir) + def install_package(self): if self.config.install_project: if self.config.pip_install or getattr(settings, 'USE_PIP_INSTALL', False): @@ -87,6 +98,73 @@ def venv_bin(self, filename=None): parts.append(filename) return os.path.join(*parts) + def environment_json_path(self): + """Return the path to the ``readthedocs-environment.json`` file.""" + return os.path.join( + self.venv_path(), + 'readthedocs-environment.json', + ) + + @property + def is_obsolete(self): + """ + Determine if the Python version of the venv obsolete. + + It checks the the data stored at ``readthedocs-environment.json`` and + compares it against the Python version in the project version to be + built and the Docker image used to create the venv against the one in + the project version config. + + :returns: ``True`` when it's obsolete and ``False`` otherwise + + :rtype: bool + """ + # Always returns False if we don't have information about what Python + # version/Docker image was used to create the venv as backward + # compatibility. + if not os.path.exists(self.environment_json_path()): + return False + + try: + with open(self.environment_json_path(), 'r') as fpath: + environment_conf = json.load(fpath) + env_python_version = environment_conf['python']['version'] + env_build_image = environment_conf['build']['image'] + except (IOError, TypeError, KeyError, ValueError): + log.error('Unable to read/parse readthedocs-environment.json file') + return False + + # TODO: remove getattr when https://github.com/rtfd/readthedocs.org/pull/3339 got merged + build_image = getattr(self.config, 'build_image', self.version.project.container_image) or DOCKER_IMAGE # noqa + + # If the user define the Python version just as a major version + # (e.g. ``2`` or ``3``) we won't know exactly which exact version was + # used to create the venv but we can still compare it against the new + # one coming from the project version config. + return any([ + env_python_version != self.config.python_full_version, + env_build_image != build_image, + ]) + + def save_environment_json(self): + """Save on disk Python and build image versions used to create the venv.""" + # TODO: remove getattr when https://github.com/rtfd/readthedocs.org/pull/3339 got merged + build_image = getattr(self.config, 'build_image', self.version.project.container_image) or DOCKER_IMAGE # noqa + + data = { + 'python': { + 'version': self.config.python_full_version, + }, + 'build': { + 'image': build_image, + }, + } + + with open(self.environment_json_path(), 'w') as fpath: + # Compatibility for Py2 and Py3. ``io.TextIOWrapper`` expects + # unicode but ``json.dumps`` returns str in Py2. + fpath.write(unicode(json.dumps(data))) + class Virtualenv(PythonEnvironment): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 0bf82c85500..a376e30ff38 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -435,8 +435,16 @@ def setup_environment(self): version=self.version, max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)): - self.python_env.delete_existing_build_dir() + # Check if the python version/build image in the current venv is the + # same to be used in this build and if it differs, wipe the venv to + # avoid conflicts. + if self.python_env.is_obsolete: + self.python_env.delete_existing_venv_dir() + else: + self.python_env.delete_existing_build_dir() + self.python_env.setup_base() + self.python_env.save_environment_json() self.python_env.install_core_requirements() self.python_env.install_user_requirements() self.python_env.install_package() diff --git a/readthedocs/rtd_tests/tests/test_doc_building.py b/readthedocs/rtd_tests/tests/test_doc_building.py index 818acaeb75a..8e928d27dae 100644 --- a/readthedocs/rtd_tests/tests/test_doc_building.py +++ b/readthedocs/rtd_tests/tests/test_doc_building.py @@ -14,18 +14,22 @@ from builtins import str import mock +import pytest from django.test import TestCase from docker.errors import APIError as DockerAPIError from docker.errors import DockerException -from mock import Mock, PropertyMock, patch +from mock import Mock, PropertyMock, mock_open, patch from readthedocs.builds.constants import BUILD_STATE_CLONING from readthedocs.builds.models import Version +from readthedocs.doc_builder.config import ConfigWrapper from readthedocs.doc_builder.environments import ( BuildCommand, DockerBuildCommand, DockerEnvironment, LocalEnvironment) from readthedocs.doc_builder.exceptions import BuildEnvironmentError +from readthedocs.doc_builder.python_environments import Virtualenv from readthedocs.projects.models import Project from readthedocs.rtd_tests.mocks.environment import EnvironmentMockGroup +from readthedocs.rtd_tests.tests.test_config_wrapper import create_load DUMMY_BUILD_ID = 123 SAMPLE_UNICODE = u'HérÉ îß sömê ünïçó∂é' @@ -831,3 +835,134 @@ def test_command_oom_kill(self): self.assertEqual( str(cmd.output), u'Command killed due to excessive memory consumption\n') + + + + +class TestAutoWipeEnvironment(TestCase): + fixtures = ['test_data'] + + def setUp(self): + self.pip = Project.objects.get(slug='pip') + self.version = self.pip.versions.get(slug='0.8') + + def test_is_obsolete_without_env_json_file(self): + yaml_config = create_load()()[0] + config = ConfigWrapper(version=self.version, yaml_config=yaml_config) + + with patch('os.path.exists') as exists: + exists.return_value = False + python_env = Virtualenv( + version=self.version, + build_env=None, + config=config, + ) + + self.assertFalse(python_env.is_obsolete) + + def test_is_obsolete_with_invalid_env_json_file(self): + yaml_config = create_load()()[0] + config = ConfigWrapper(version=self.version, yaml_config=yaml_config) + + with patch('os.path.exists') as exists: + exists.return_value = True + python_env = Virtualenv( + version=self.version, + build_env=None, + config=config, + ) + + self.assertFalse(python_env.is_obsolete) + + def test_is_obsolete_with_json_different_python_version(self): + config_data = { + 'build': { + 'image': '2.0', + }, + 'python': { + 'version': 2.7, + }, + } + yaml_config = create_load(config_data)()[0] + config = ConfigWrapper(version=self.version, yaml_config=yaml_config) + + python_env = Virtualenv( + version=self.version, + build_env=None, + config=config, + ) + env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 3.5}}' + with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa + exists.return_value = True + self.assertTrue(python_env.is_obsolete) + + @pytest.mark.xfail(reason='build.image is not being considered yet') + def test_is_obsolete_with_json_different_build_image(self): + config_data = { + 'build': { + 'image': 'latest', + }, + 'python': { + 'version': 2.7, + }, + } + yaml_config = create_load(config_data)()[0] + config = ConfigWrapper(version=self.version, yaml_config=yaml_config) + + python_env = Virtualenv( + version=self.version, + build_env=None, + config=config, + ) + env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 2.7}}' + with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa + exists.return_value = True + self.assertTrue(python_env.is_obsolete) + + def test_is_obsolete_with_project_different_build_image(self): + config_data = { + 'build': { + 'image': '2.0', + }, + 'python': { + 'version': 2.7, + }, + } + yaml_config = create_load(config_data)()[0] + config = ConfigWrapper(version=self.version, yaml_config=yaml_config) + + # Set container_image manually + self.pip.container_image = 'readthedocs/build:latest' + self.pip.save() + + python_env = Virtualenv( + version=self.version, + build_env=None, + config=config, + ) + env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 2.7}}' + with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa + exists.return_value = True + self.assertTrue(python_env.is_obsolete) + + def test_is_obsolete_with_json_same_data_as_version(self): + config_data = { + 'build': { + 'image': '2.0', + }, + 'python': { + 'version': 3.5, + }, + } + yaml_config = create_load(config_data)()[0] + config = ConfigWrapper(version=self.version, yaml_config=yaml_config) + + python_env = Virtualenv( + version=self.version, + build_env=None, + config=config, + ) + env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 3.5}}' + with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa + exists.return_value = True + self.assertFalse(python_env.is_obsolete)