Do a small builder refactor.

readthedocs/doc_builder/config.py

@@ -108,8 +108,8 @@ def load_yaml_config(version):
                 'name': version.slug,
             },
         )[0]
-    except InvalidConfig as e:  # This is a subclass of ConfigError, so has to come first
-        raise ProjectImportError(e.message)
+    except InvalidConfig:  # This is a subclass of ConfigError, so has to come first
+        raise
     except ConfigError:
         config = BuildConfig(
             env_config={},

readthedocs/doc_builder/environments.py

@@ -36,6 +36,7 @@
 
 
 class BuildCommand(BuildCommandResultMixin):
+
     '''Wrap command execution for execution in build environments
 
     This wraps subprocess commands with some logic to handle exceptions,
@@ -177,6 +178,7 @@ def save(self):
 
 
 class DockerBuildCommand(BuildCommand):
+
     '''Create a docker container and run a command inside the container
 
     Build command to execute in docker container
@@ -234,11 +236,9 @@ def get_wrapped_command(self):
         """
         bash_escape_re = re.compile(r"([\t\ \!\"\#\$\&\'\(\)\*\:\;\<\>\?\@"
                                     r"\[\\\]\^\`\{\|\}\~])")
-        prefix = 'READTHEDOCS=True '
+        prefix = ''
         if self.bin_path:
             prefix += 'PATH={0}:$PATH '.format(self.bin_path)
-        if 'CONDA_ENVS_PATH' in self.environment:
-            prefix += 'CONDA_ENVS_PATH={0} '.format(self.environment['CONDA_ENVS_PATH'])
         return ("/bin/sh -c 'cd {cwd} && {prefix}{cmd}'"
                 .format(
                     cwd=self.cwd,
@@ -248,6 +248,7 @@ def get_wrapped_command(self):
 
 
 class BuildEnvironment(object):
+
     '''
     Base build environment
 
@@ -259,11 +260,13 @@ class BuildEnvironment(object):
     :param record: Record status of build object
     '''
 
-    def __init__(self, project=None, version=None, build=None, record=True):
+    def __init__(self, project=None, version=None, build=None, record=True, environment=None):
         self.project = project
         self.version = version
         self.build = build
         self.record = record
+        self.environment = environment or {}
+
         self.commands = []
         self.failure = None
         self.start_time = datetime.utcnow()
@@ -317,6 +320,12 @@ def run_command_class(self, cls, cmd, **kwargs):
         :param warn_only: Don't raise an exception on command failure
         '''
         warn_only = kwargs.pop('warn_only', False)
+        # Remove PATH from env, and set it to bin_path if it isn't passed in
+        env_path = self.environment.pop('PATH', None)
+        if 'bin_path' not in kwargs and env_path:
+            kwargs['bin_path'] = env_path
+        assert 'environment' not in kwargs, "environment can't be passed in via commands."
+        kwargs['environment'] = self.environment
         kwargs['build_env'] = self
         build_cmd = cls(cmd, **kwargs)
         self.commands.append(build_cmd)
@@ -408,11 +417,13 @@ def update_build(self, state=None):
 
 
 class LocalEnvironment(BuildEnvironment):
+
     '''Local execution environment'''
     command_class = BuildCommand
 
 
 class DockerEnvironment(BuildEnvironment):
+
     '''
     Docker build environment, uses docker to contain builds
 
@@ -597,8 +608,7 @@ def create_container(self):
                     },
                 }),
                 detach=True,
-                environment={'READTHEDOCS_VERSION': self.version.slug,
-                             'READTHEDOCS_PROJECT': self.project.slug},
+                environment=self.environment,
                 mem_limit=self.container_mem_limit,
             )
             client.start(container=self.container_id)

readthedocs/doc_builder/python_environments.py

@@ -180,7 +180,6 @@ def setup_base(self):
             '--name',
             self.version.slug,
             'python={python_version}'.format(python_version=self.config.python_version),
-            environment={'CONDA_ENVS_PATH': conda_env_path}
         )
 
     def install_core_requirements(self):
@@ -206,8 +205,7 @@ def install_core_requirements(self):
         ]
         cmd.extend(requirements)
         self.build_env.run(
-            *cmd,
-            environment={'CONDA_ENVS_PATH': conda_env_path}
+            *cmd
         )
 
         # Install pip-only things.
@@ -242,5 +240,4 @@ def install_user_requirements(self):
             self.version.slug,
             '--file',
             self.config.conda_file,
-            environment={'CONDA_ENVS_PATH': conda_env_path}
         )

readthedocs/projects/tasks.py

@@ -108,12 +108,11 @@ def run(self, pk, version_pk=None, build_pk=None, record=True, docker=False,
         self.build_force = force
 
         env_cls = LocalEnvironment
-        if docker or settings.DOCKER_ENABLE:
-            env_cls = DockerEnvironment
-        self.build_env = env_cls(project=self.project, version=self.version,
+        self.setup_env = env_cls(project=self.project, version=self.version,
                                  build=self.build, record=record)
 
-        with self.build_env:
+        # Environment used for code checkout & initial configuration reading
+        with self.setup_env:
             if self.project.skip:
                 raise BuildEnvironmentError(
                     _('Builds for this project are temporarily disabled'))
@@ -126,12 +125,20 @@ def run(self, pk, version_pk=None, build_pk=None, record=True, docker=False,
                     status_code=423
                 )
 
+            self.config = load_yaml_config(version=self.version)
+
+        bash_env = self.get_bash_env()
+        if docker or settings.DOCKER_ENABLE:
+            env_cls = DockerEnvironment
+        self.build_env = env_cls(project=self.project, version=self.version,
+                                 build=self.build, record=record, environment=bash_env)
+
+        # Environment used for building code, usually with Docker
+        with self.build_env:
+
             if self.project.documentation_type == 'auto':
                 self.update_documentation_type()
 
-            # Read YAML config after code checkout has been run
-            self.config = load_yaml_config(version=self.version)
-
             python_env_cls = Virtualenv
             if self.config.use_conda:
                 self._log('Using conda')
@@ -195,20 +202,6 @@ def get_build(build_pk):
                     if key not in ['project', 'version', 'resource_uri',
                                    'absolute_uri'])
 
-    def update_documentation_type(self):
-        """
-        Force Sphinx for 'auto' documentation type
-
-        This used to determine the type and automatically set the documentation
-        type to Sphinx for rST and Mkdocs for markdown. It now just forces
-        Sphinx, due to markdown support.
-        """
-        ret = 'sphinx'
-        project_data = api_v2.project(self.project.pk).get()
-        project_data['documentation_type'] = ret
-        api_v2.project(self.project.pk).put(project_data)
-        self.project.documentation_type = ret
-
     def setup_vcs(self):
         """
         Update the checkout of the repo to make sure it's the latest.
@@ -217,7 +210,7 @@ def setup_vcs(self):
 
         :param build_env: Build environment
         """
-        self.build_env.update_build(state=BUILD_STATE_CLONING)
+        self.setup_env.update_build(state=BUILD_STATE_CLONING)
 
         self._log(msg='Updating docs from VCS')
         try:
@@ -229,6 +222,43 @@ def setup_vcs(self):
             raise BuildEnvironmentError('Failed to import project',
                                         status_code=404)
 
+    def get_bash_env(self):
+        """
+        Get bash environment variables used for all builder commands.
+        """
+        env = {
+            'READTHEDOCS': True,
+            'READTHEDOCS_VERSION': self.version.slug,
+            'READTHEDOCS_PROJECT': self.project.slug
+        }
+
+        if self.config.use_conda:
+            env.update({
+                'CONDA_ENVS_PATH': os.path.join(self.project.doc_path, 'conda'),
+                'CONDA_DEFAULT_ENV': self.version.slug,
+                'PATH': os.path.join(self.project.doc_path, 'conda', self.version.slug, 'bin')
+            })
+        else:
+            env.update({
+                'PATH': os.path.join(self.project.doc_path, 'envs', self.version.slug, 'bin')
+            })
+
+        return env
+
+    def update_documentation_type(self):
+        """
+        Force Sphinx for 'auto' documentation type
+
+        This used to determine the type and automatically set the documentation
+        type to Sphinx for rST and Mkdocs for markdown. It now just forces
+        Sphinx, due to markdown support.
+        """
+        ret = 'sphinx'
+        project_data = api_v2.project(self.project.pk).get()
+        project_data['documentation_type'] = ret
+        api_v2.project(self.project.pk).put(project_data)
+        self.project.documentation_type = ret
+
     def setup_environment(self):
         """
         Build the virtualenv and install the project into it.

readthedocs/rtd_tests/tests/test_doc_building.py

@@ -169,7 +169,7 @@ def test_command_execution(self):
 
         self.mocks.docker_client.exec_create.assert_called_with(
             container='version-foobar-of-pip-20',
-            cmd="/bin/sh -c 'cd /tmp && READTHEDOCS=True echo\\ test'",
+            cmd="/bin/sh -c 'cd /tmp && echo\\ test'",
             stderr=True,
             stdout=True
         )
@@ -344,15 +344,15 @@ def test_wrapped_command(self):
             cmd.get_wrapped_command(),
             ("/bin/sh -c "
              "'cd /tmp/foobar && "
-             "READTHEDOCS=True pip install requests'"))
+             "pip install requests'"))
         cmd = DockerBuildCommand(['python', '/tmp/foo/pip', 'install',
                                   'Django>1.7'],
                                  cwd='/tmp/foobar',
                                  bin_path='/tmp/foo')
         self.assertEqual(
             cmd.get_wrapped_command(),
             ("/bin/sh -c "
-             "'cd /tmp/foobar && READTHEDOCS=True PATH=/tmp/foo:$PATH "
+             "'cd /tmp/foobar && PATH=/tmp/foo:$PATH "
              "python /tmp/foo/pip install Django\>1.7'"))
 
     def test_unicode_output(self):