Don't mutate Environment.environment

Author: stsewd

readthedocs/doc_builder/environments.py

@@ -105,8 +105,8 @@ def __init__(
         self.shell = shell
         self.cwd = cwd or settings.RTD_DOCKER_WORKDIR
         self.user = user or settings.RTD_DOCKER_USER
-        self.environment = environment.copy() if environment else {}
-        if 'PATH' in self.environment:
+        self._environment = environment.copy() if environment else {}
+        if 'PATH' in self._environment:
             raise BuildEnvironmentError('\'PATH\' can\'t be set. Use bin_path')
 
         self.build_env = build_env
@@ -132,7 +132,7 @@ def run(self):
         log.info("Running: '%s' [%s]", self.get_command(), self.cwd)
 
         self.start_time = datetime.utcnow()
-        environment = self.environment.copy()
+        environment = self._environment.copy()
         if 'DJANGO_SETTINGS_MODULE' in environment:
             del environment['DJANGO_SETTINGS_MODULE']
         if 'PYTHONPATH' in environment:
@@ -302,7 +302,7 @@ def run(self):
             exec_cmd = client.exec_create(
                 container=self.build_env.container_id,
                 cmd=self.get_wrapped_command(),
-                environment=self.environment,
+                environment=self._environment,
                 user=self.user,
                 workdir=self.cwd,
                 stdout=True,
@@ -384,7 +384,7 @@ class BaseEnvironment:
     def __init__(self, project, environment=None):
         # TODO: maybe we can remove this Project dependency also
         self.project = project
-        self.environment = environment or {}
+        self._environment = environment or {}
         self.commands = []
 
     def record_command(self, command):
@@ -423,7 +423,7 @@ def run_command_class(
             kwargs.update({'record_as_success': record_as_success})
 
         # Remove PATH from env, and set it to bin_path if it isn't passed in
-        environment = self.environment.copy()
+        environment = self._environment.copy()
         env_path = environment.pop('BIN_PATH', None)
         if 'bin_path' not in kwargs and env_path:
             kwargs['bin_path'] = env_path

readthedocs/projects/tasks.py

@@ -311,6 +311,13 @@ def validate_duplicate_reserved_versions(self, tags_data, branches_data):
                     RepositoryError.DUPLICATED_RESERVED_VERSIONS,
                 )
 
+    def get_vcs_env_vars(self):
+        """Get environment variables to be included in the VCS setup step."""
+        env = self.get_rtd_env_vars()
+        # Don't prompt for username, this requires Git 2.3+
+        env['GIT_TERMINAL_PROMPT'] = '0'
+        return env
+
     def get_rtd_env_vars(self):
         """Get bash environment variables specific to Read the Docs."""
         env = {
@@ -370,7 +377,7 @@ def run(self, version_pk):  # pylint: disable=arguments-differ
                 version=self.version,
                 record=False,
                 update_on_success=False,
-                environment=self.get_rtd_env_vars(),
+                environment=self.get_vcs_env_vars(),
             )
             log.info(
                 'Running sync_repository_task: project=%s version=%s',
@@ -657,7 +664,7 @@ def run_setup(self, record=True):
             build=self.build,
             record=record,
             update_on_success=False,
-            environment=self.get_rtd_env_vars(),
+            environment=self.get_vcs_env_vars(),
         )
         self.build_start_time = environment.start_time
 

readthedocs/rtd_tests/tests/test_doc_building.py

@@ -1024,7 +1024,7 @@ def test_command_env(self):
         env = {'FOOBAR': 'foobar', 'BIN_PATH': 'foobar'}
         cmd = BuildCommand('echo', environment=env)
         for key in list(env.keys()):
-            self.assertEqual(cmd.environment[key], env[key])
+            self.assertEqual(cmd._environment[key], env[key])
 
     def test_result(self):
         """Test result of output using unix true/false commands."""

readthedocs/vcs_support/backends/git.py

@@ -1,7 +1,6 @@
 """Git-related utilities."""
 
 import logging
-import os
 import re
 
 import git
@@ -359,11 +358,3 @@ def ref_exists(self, ref):
         except (BadName, ValueError):
             return False
         return False
-
-    @property
-    def env(self):
-        env = super().env
-        env['GIT_DIR'] = os.path.join(self.working_dir, '.git')
-        # Don't prompt for username, this requires Git 2.3+
-        env['GIT_TERMINAL_PROMPT'] = '0'
-        return env

readthedocs/vcs_support/base.py

@@ -72,9 +72,6 @@ def __init__(
         from readthedocs.doc_builder.environments import LocalBuildEnvironment
         self.environment = environment or LocalBuildEnvironment(record=False)
 
-        # Update the env variables with the proper VCS env variables
-        self.environment.environment.update(self.env)
-
     def check_working_dir(self):
         if not os.path.exists(self.working_dir):
             os.makedirs(self.working_dir)
@@ -84,10 +81,6 @@ def make_clean_working_dir(self):
         shutil.rmtree(self.working_dir, ignore_errors=True)
         self.check_working_dir()
 
-    @property
-    def env(self):
-        return {}
-
     def update(self):
         """
         Update a local copy of the repository in self.working_dir.