Build: pass environment explicitly (#10388)

With #10378
we now need to always pass an environment,
we can't just create a default one.

Author: stsewd

readthedocs/projects/models.py

@@ -947,11 +947,8 @@ def has_htmlzip(self, version_slug=LATEST, version_type=None):
             version_type=version_type
         )
 
-    # NOTE: if `environment=None` everything fails, because it cannot execute
-    # any command.
     def vcs_repo(
-            self, version=LATEST, environment=None,
-            verbose_name=None, version_type=None
+        self, environment, version=LATEST, verbose_name=None, version_type=None
     ):
         """
         Return a Backend object for this project able to handle VCS commands.

readthedocs/rtd_tests/tests/test_backend.py

@@ -12,6 +12,7 @@
 from readthedocs.builds.constants import EXTERNAL
 from readthedocs.builds.models import Version
 from readthedocs.config import ALL
+from readthedocs.doc_builder.environments import LocalBuildEnvironment
 from readthedocs.projects.exceptions import RepositoryError
 from readthedocs.projects.models import Feature, Project
 from readthedocs.rtd_tests.utils import (
@@ -45,6 +46,7 @@ def setUp(self):
         # These are the default values from v1
         self.dummy_conf.submodules.include = ALL
         self.dummy_conf.submodules.exclude = []
+        self.build_environment = LocalBuildEnvironment()
 
     def test_git_lsremote(self):
         repo_path = self.project.repo
@@ -68,7 +70,7 @@ def test_git_lsremote(self):
         create_git_tag(repo_path, 'v02', annotated=True)
         create_git_tag(repo_path, 'release-ünîø∂é')
 
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         # create the working dir if it not exists. It's required to ``cwd`` to
         # execute the command
         repo.check_working_dir()
@@ -91,7 +93,7 @@ def test_git_lsremote_tags_only(self):
         create_git_tag(repo_path, "v02", annotated=True)
         create_git_tag(repo_path, "release-ünîø∂é")
 
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         # create the working dir if it not exists. It's required to ``cwd`` to
         # execute the command
         repo.check_working_dir()
@@ -123,7 +125,7 @@ def test_git_lsremote_branches_only(self):
         for branch in branches:
             create_git_branch(repo_path, branch)
 
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         # create the working dir if it not exists. It's required to ``cwd`` to
         # execute the command
         repo.check_working_dir()
@@ -160,7 +162,7 @@ def test_git_branches(self, checkout_path):
         os.mkdir(local_repo)
         checkout_path.return_value = local_repo
 
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.clone()
 
         self.assertEqual(
@@ -188,7 +190,7 @@ def test_git_branches_unicode(self, checkout_path):
         os.mkdir(local_repo)
         checkout_path.return_value = local_repo
 
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.clone()
 
         self.assertEqual(
@@ -197,7 +199,7 @@ def test_git_branches_unicode(self, checkout_path):
         )
 
     def test_git_update_and_checkout(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         code, _, _ = repo.update()
         self.assertEqual(code, 0)
 
@@ -217,7 +219,8 @@ def test_git_update_with_external_version(self, fetch):
         )
         repo = self.project.vcs_repo(
             verbose_name=version.verbose_name,
-            version_type=version.type
+            version_type=version.type,
+            environment=self.build_environment,
         )
         repo.update()
         fetch.assert_called_once()
@@ -231,14 +234,15 @@ def test_git_fetch_with_external_version(self):
         )
         repo = self.project.vcs_repo(
             verbose_name=version.verbose_name,
-            version_type=version.type
+            version_type=version.type,
+            environment=self.build_environment,
         )
         repo.update()
         code, _, _ = repo.fetch()
         self.assertEqual(code, 0)
 
     def test_git_checkout_invalid_revision(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         version = 'invalid-revision'
         with self.assertRaises(RepositoryError) as e:
@@ -253,7 +257,7 @@ def test_git_tags(self):
         create_git_tag(repo_path, 'v01')
         create_git_tag(repo_path, 'v02', annotated=True)
         create_git_tag(repo_path, 'release-ünîø∂é')
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         # We aren't cloning the repo,
         # so we need to hack the repo path
         repo.working_dir = repo_path
@@ -264,7 +268,7 @@ def test_git_tags(self):
         )
 
     def test_check_for_submodules(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
 
         repo.update()
         self.assertFalse(repo.are_submodules_available(self.dummy_conf))
@@ -274,13 +278,13 @@ def test_check_for_submodules(self):
         self.assertTrue(repo.are_submodules_available(self.dummy_conf))
 
     def test_skip_submodule_checkout(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         repo.checkout('submodule')
         self.assertTrue(repo.are_submodules_available(self.dummy_conf))
 
     def test_use_shallow_clone(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         repo.checkout('submodule')
         self.assertTrue(repo.use_shallow_clone())
@@ -293,14 +297,14 @@ def test_use_shallow_clone(self):
         self.assertFalse(repo.use_shallow_clone())
 
     def test_check_submodule_urls(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         repo.checkout('submodule')
         valid, _ = repo.validate_submodules(self.dummy_conf)
         self.assertTrue(valid)
 
     def test_check_invalid_submodule_urls(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         repo.checkout('invalidsubmodule')
         with self.assertRaises(RepositoryError) as e:
@@ -313,7 +317,7 @@ def test_check_invalid_submodule_urls(self):
         )
 
     def test_invalid_submodule_is_ignored(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         repo.checkout('submodule')
         gitmodules_path = os.path.join(repo.working_dir, '.gitmodules')
@@ -341,7 +345,7 @@ def test_fetch_clean_tags_and_branches(self, checkout_path):
         os.mkdir(local_repo)
         checkout_path.return_value = local_repo
 
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.clone()
 
         delete_git_tag(upstream_repo, 'v02')
@@ -391,20 +395,25 @@ def setUp(self):
             repo=hg_repo,
         )
         self.project.users.add(self.eric)
+        self.build_environment = LocalBuildEnvironment()
 
     def test_parse_branches(self):
         data = """\
         stable
         default
         """
 
-        expected_ids = ['stable', 'default']
-        given_ids = [x.identifier for x in
-                     self.project.vcs_repo().parse_branches(data)]
+        expected_ids = ["stable", "default"]
+        given_ids = [
+            x.identifier
+            for x in self.project.vcs_repo(
+                environment=self.build_environment
+            ).parse_branches(data)
+        ]
         self.assertEqual(expected_ids, given_ids)
 
     def test_update_and_checkout(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.make_clean_working_dir()
         code, _, _ = repo.update()
         self.assertEqual(code, 0)
@@ -413,7 +422,7 @@ def test_update_and_checkout(self):
         self.assertTrue(exists(repo.working_dir))
 
     def test_checkout_invalid_revision(self):
-        repo = self.project.vcs_repo()
+        repo = self.project.vcs_repo(environment=self.build_environment)
         repo.update()
         version = 'invalid-revision'
         with self.assertRaises(RepositoryError) as e:
@@ -436,6 +445,10 @@ def test_parse_tags(self):
             ('2b2155623ee2', '1.7.5'),
         ]
 
-        given_ids = [(x.identifier, x.verbose_name) for x in
-                     self.project.vcs_repo().parse_tags(data)]
+        given_ids = [
+            (x.identifier, x.verbose_name)
+            for x in self.project.vcs_repo(
+                environment=self.build_environment
+            ).parse_tags(data)
+        ]
         self.assertEqual(expected_tags, given_ids)

readthedocs/rtd_tests/tests/test_backend_svn.py

@@ -4,6 +4,7 @@
 from django_dynamic_fixture import get
 
 from readthedocs.builds.models import Version
+from readthedocs.doc_builder.environments import LocalBuildEnvironment
 from readthedocs.projects.models import Project
 from readthedocs.vcs_support.backends.svn import Backend as SvnBackend
 
@@ -13,7 +14,8 @@ class TestSvnBackend(TestCase):
     def test_get_url(self):
         project = get(Project)
         version = get(Version, project=project)
-        backend_obj = SvnBackend(project, version.slug)
+        environment = LocalBuildEnvironment()
+        backend_obj = SvnBackend(project, version.slug, environment=environment)
 
         base = 'http://example.com/'
         tag = 'xyz/'

readthedocs/rtd_tests/tests/test_version.py

@@ -70,7 +70,7 @@ def test_vcs_url_for_external_version_gitlab(self):
         self.assertEqual(self.external_version.vcs_url, expected_url)
 
     def test_vcs_url_for_latest_version(self):
-        slug = self.pip.default_branch or self.pip.vcs_repo().fallback_branch
+        slug = self.pip.default_branch or self.pip.vcs_class().fallback_branch
         expected_url = f'https://github.com/pypa/pip/tree/{slug}/'
         self.assertEqual(self.tag_version.vcs_url, expected_url)
 

readthedocs/vcs_support/base.py

@@ -55,8 +55,13 @@ class BaseVCS:
     # Defining a base API, so we'll have unused args
     # pylint: disable=unused-argument
     def __init__(
-            self, project, version_slug, environment=None,
-            verbose_name=None, version_type=None, **kwargs
+        self,
+        project,
+        version_slug,
+        environment,
+        verbose_name=None,
+        version_type=None,
+        **kwargs
     ):
         self.default_branch = project.default_branch
         self.project = project
@@ -67,13 +72,7 @@ def __init__(
         self.verbose_name = verbose_name
         self.version_type = version_type
 
-        # TODO: always pass an explicit environment
-        # This is only used in tests #6546
-        #
-        # TODO: we should not allow ``environment=None`` and always use the
-        # environment defined by the settings
-        from readthedocs.doc_builder.environments import LocalBuildEnvironment
-        self.environment = environment or LocalBuildEnvironment()
+        self.environment = environment
 
     def check_working_dir(self):
         if not os.path.exists(self.working_dir):