VCS: pass a version object to git provider (#12101)

stsewd · web-flow · commit 08284af69079 · 2025-04-14T10:35:22.000-05:00
Closes #11955
diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py
@@ -89,12 +89,8 @@ def setup_vcs(self):
         # Create the VCS repository where all the commands are going to be
         # executed for a particular VCS type
         self.vcs_repository = self.data.project.vcs_repo(
-            version=self.data.version.slug,
+            version=self.data.version,
             environment=self.vcs_environment,
-            verbose_name=self.data.version.verbose_name,
-            version_type=self.data.version.type,
-            version_identifier=self.data.version.identifier,
-            version_machine=self.data.version.machine,
         )
 
         # We can't do too much on ``pre_checkout`` because we haven't
diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py
@@ -964,39 +964,22 @@ def has_good_build(self):
             return self._good_build
         return self.builds(manager=INTERNAL).filter(success=True).exists()
 
-    def vcs_repo(
-        self,
-        environment,
-        version=LATEST,
-        verbose_name=None,
-        version_type=None,
-        version_identifier=None,
-        version_machine=None,
-    ):
+    def vcs_repo(self, environment, version):
         """
         Return a Backend object for this project able to handle VCS commands.
 
         :param environment: environment to run the commands
         :type environment: doc_builder.environments.BuildEnvironment
-        :param version: version slug for the backend (``LATEST`` by default)
-        :type version: str
+        :param version: Version for the backend.
         """
-        # TODO: this seems to be the only method that receives a
-        # ``version.slug`` instead of a ``Version`` instance (I prefer an
-        # instance here)
-
         backend = self.vcs_class()
         if not backend:
             repo = None
         else:
             repo = backend(
                 self,
-                version,
+                version=version,
                 environment=environment,
-                verbose_name=verbose_name,
-                version_type=version_type,
-                version_identifier=version_identifier,
-                version_machine=version_machine,
             )
         return repo
 
diff --git a/readthedocs/projects/tasks/builds.py b/readthedocs/projects/tasks/builds.py
@@ -220,10 +220,8 @@ def execute(self):
             )
 
             vcs_repository = self.data.project.vcs_repo(
-                version=self.data.version.slug,
+                version=self.data.version,
                 environment=environment,
-                verbose_name=self.data.version.verbose_name,
-                version_type=self.data.version.type,
             )
             log.info("Syncing repository via remote listing.")
             self.sync_versions(vcs_repository)
diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py
@@ -1,14 +1,16 @@
 import os
 import textwrap
+from readthedocs.core.utils.filesystem import safe_rmtree
 from os.path import exists
 from unittest import mock
 from unittest.mock import Mock, patch
 
 import django_dynamic_fixture as fixture
+from django_dynamic_fixture import get
 from django.contrib.auth.models import User
 from django.test import TestCase
 
-from readthedocs.builds.constants import BRANCH, EXTERNAL, TAG
+from readthedocs.builds.constants import BRANCH, EXTERNAL, STABLE, TAG
 from readthedocs.builds.models import Version
 from readthedocs.config import ALL
 from readthedocs.doc_builder.environments import LocalBuildEnvironment
@@ -46,8 +48,8 @@ def setUp(self):
         self.build_environment = LocalBuildEnvironment(api_client=mock.MagicMock())
 
     def tearDown(self):
-        repo = self.project.vcs_repo(environment=self.build_environment)
-        repo.make_clean_working_dir()
+        # Remove all the files created by the test for this project.
+        safe_rmtree(self.project.doc_path)
         super().tearDown()
 
     def test_git_lsremote(self):
@@ -72,7 +74,8 @@ 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(environment=self.build_environment)
+        version = self.project.versions.first()
+        repo = self.project.vcs_repo(environment=self.build_environment, version=version)
         # create the working dir if it not exists. It's required to ``cwd`` to
         # execute the command
         repo.check_working_dir()
@@ -95,7 +98,8 @@ 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(environment=self.build_environment)
+        version = self.project.versions.first()
+        repo = self.project.vcs_repo(environment=self.build_environment, version=version)
         # create the working dir if it not exists. It's required to ``cwd`` to
         # execute the command
         repo.check_working_dir()
@@ -127,7 +131,8 @@ def test_git_lsremote_branches_only(self):
         for branch in branches:
             create_git_branch(repo_path, branch)
 
-        repo = self.project.vcs_repo(environment=self.build_environment)
+        version = self.project.versions.first()
+        repo = self.project.vcs_repo(environment=self.build_environment, version=version)
         # create the working dir if it not exists. It's required to ``cwd`` to
         # execute the command
         repo.check_working_dir()
@@ -142,7 +147,8 @@ def test_git_lsremote_branches_only(self):
         )
 
     def test_git_update_and_checkout(self):
-        repo = self.project.vcs_repo(environment=self.build_environment)
+        version = self.project.versions.first()
+        repo = self.project.vcs_repo(environment=self.build_environment, version=version)
         code, _, _ = repo.update()
         self.assertEqual(code, 0)
 
@@ -153,11 +159,12 @@ def test_git_update_and_checkout(self):
         self.assertTrue(exists(repo.working_dir))
 
     def test_git_checkout_invalid_revision(self):
-        repo = self.project.vcs_repo(environment=self.build_environment)
+        version = self.project.versions.first()
+        repo = self.project.vcs_repo(environment=self.build_environment, version=version)
         repo.update()
-        version = "invalid-revision"
+        branch = "invalid-revision"
         with self.assertRaises(RepositoryError) as e:
-            repo.checkout(version)
+            repo.checkout(branch)
         self.assertEqual(
             e.exception.message_id,
             RepositoryError.FAILED_TO_CHECKOUT,
@@ -167,10 +174,17 @@ def test_check_for_submodules(self):
         """
         Test that we can get a branch called 'submodule' containing a valid submodule.
         """
+        version = get(
+            Version,
+            project=self.project,
+            type=BRANCH,
+            identifier="submodule",
+            verbose_name="submodule",
+        )
+
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=BRANCH,
-            version_identifier="submodule",
+            version=version,
         )
 
         repo.update()
@@ -182,10 +196,16 @@ def test_check_for_submodules(self):
 
     def test_check_submodule_urls(self):
         """Test that a valid submodule is found in the 'submodule' branch."""
+        version = get(
+            Version,
+            project=self.project,
+            type=BRANCH,
+            identifier="submodule",
+            verbose_name="submodule",
+        )
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=BRANCH,
-            version_identifier="submodule",
+            version=version,
         )
         repo.update()
         repo.checkout("submodule")
@@ -204,20 +224,24 @@ def test_git_update_with_external_version(self, fetch):
             verbose_name="1234",  # pr number
         )
         repo = self.project.vcs_repo(
-            verbose_name=version.verbose_name,
-            version_type=version.type,
-            version_identifier=version.identifier,
+            version=version,
             environment=self.build_environment,
         )
         repo.update()
         fetch.assert_called_once()
 
     def test_submodule_without_url_is_included(self):
         """Test that an invalid submodule isn't listed."""
+        version = get(
+            Version,
+            project=self.project,
+            type=BRANCH,
+            identifier="submodule",
+            verbose_name="submodule",
+        )
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=BRANCH,
-            version_identifier="submodule",
+            version=version,
         )
         repo.update()
         repo.checkout("submodule")
@@ -237,10 +261,16 @@ def test_submodule_without_url_is_included(self):
         self.assertEqual(submodules, ["foobar", "not-valid-path"])
 
     def test_parse_submodules(self):
+        version = get(
+            Version,
+            project=self.project,
+            type=BRANCH,
+            identifier="submodule",
+            verbose_name="submodule",
+        )
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=BRANCH,
-            version_identifier="submodule",
+            version=version,
         )
         repo.update()
         repo.checkout("submodule")
@@ -290,10 +320,16 @@ def test_parse_submodules(self):
 
     def test_skip_submodule_checkout(self):
         """Test that a submodule is listed as available."""
+        version = get(
+            Version,
+            project=self.project,
+            type=BRANCH,
+            identifier="submodule",
+            verbose_name="submodule",
+        )
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=BRANCH,
-            version_identifier="submodule",
+            version=version,
         )
         repo.update()
         repo.checkout("submodule")
@@ -303,8 +339,7 @@ def test_git_fetch_with_external_version(self):
         """Test that fetching an external build (PR branch) correctly executes."""
         version = fixture.get(Version, project=self.project, type=EXTERNAL, active=True)
         repo = self.project.vcs_repo(
-            verbose_name=version.verbose_name,
-            version_type=version.type,
+            version=version,
             environment=self.build_environment,
         )
         repo.update()
@@ -323,9 +358,17 @@ def test_update_without_branch_name(self):
         repo_path = self.project.repo
         create_git_branch(repo_path, "develop")
 
+        version = get(
+            Version,
+            project=self.project,
+            type=BRANCH,
+            identifier="develop",
+            verbose_name="develop",
+        )
+
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=BRANCH,
+            version=version,
         )
         repo.update()
 
@@ -345,11 +388,19 @@ def test_special_tag_stable(self):
         repo_path = self.project.repo
         latest_actual_commit_hash = get_git_latest_commit_hash(repo_path, "master")
 
+        version = get(
+            Version,
+            project=self.project,
+            type=TAG,
+            identifier=latest_actual_commit_hash,
+            verbose_name="stable",
+            slug=STABLE,
+            machine=True,
+        )
+
         repo = self.project.vcs_repo(
             environment=self.build_environment,
-            version_type=TAG,
-            version_machine=True,
-            version_identifier=latest_actual_commit_hash,
+            version=version,
         )
         repo.update()
 
diff --git a/readthedocs/rtd_tests/utils.py b/readthedocs/rtd_tests/utils.py
@@ -197,7 +197,7 @@ def get_git_latest_commit_hash(directory, branch):
     chdir(directory)
 
     command = ["git", "rev-parse", branch]
-    return check_output(command, env=env).strip()
+    return check_output(command, env=env).decode().strip()
 
 
 @restoring_chdir
diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py
diff --git a/readthedocs/vcs_support/base.py b/readthedocs/vcs_support/base.py

Original file line number	Diff line number	Diff line change
`@@ -220,10 +220,8 @@ def execute(self):`
`220`	`220`	`)`
`221`	`221`
`222`	`222`	`vcs_repository = self.data.project.vcs_repo(`
`223`		`- version=self.data.version.slug,`
	`223`	`+ version=self.data.version,`
`224`	`224`	`environment=environment,`
`225`		`- verbose_name=self.data.version.verbose_name,`
`226`		`- version_type=self.data.version.type,`
`227`	`225`	`)`
`228`	`226`	`log.info("Syncing repository via remote listing.")`
`229`	`227`	`self.sync_versions(vcs_repository)`