From 42a82cabf8df749736c540a64ff5575f61a00567 Mon Sep 17 00:00:00 2001
From: Santos Gallegos <stsewd@proton.me>
Date: Tue, 31 Oct 2023 10:37:16 -0500
Subject: [PATCH] Build: use tag name for checkout (#10879)

* Build: use tag name for checkout

* Comment
---
 readthedocs/doc_builder/director.py           |  7 +-
 .../projects/tests/test_build_tasks.py        | 76 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py
index fcbb4c47c2e..24441fdad98 100644
--- a/readthedocs/doc_builder/director.py
+++ b/readthedocs/doc_builder/director.py
@@ -225,7 +225,12 @@ def checkout(self):
         log.info("Cloning and fetching.")
         self.vcs_repository.update()
 
-        identifier = self.data.build_commit or self.data.version.identifier
+        # NOTE: we use `commit_name` instead of `identifier`,
+        # since identifier can be a ref name or a commit hash,
+        # and we want to use the ref name when doing the checkout when possible
+        # (e.g. `main` instead of `a1b2c3d4`, or `v1.0` instead of `a1b2c3d4`).
+        # See https://github.com/readthedocs/readthedocs.org/issues/10838.
+        identifier = self.data.build_commit or self.data.version.commit_name
         log.info("Checking out.", identifier=identifier)
         self.vcs_repository.checkout(identifier)
 
diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py
index de60cd4d5f3..0bd577827df 100644
--- a/readthedocs/projects/tests/test_build_tasks.py
+++ b/readthedocs/projects/tests/test_build_tasks.py
@@ -11,6 +11,7 @@
     BUILD_STATUS_FAILURE,
     BUILD_STATUS_SUCCESS,
     EXTERNAL,
+    TAG,
 )
 from readthedocs.builds.models import Build
 from readthedocs.config import ALL, ConfigError
@@ -21,6 +22,7 @@
 from readthedocs.projects.models import EnvironmentVariable, Project, WebHookEvent
 from readthedocs.projects.tasks.builds import sync_repository_task, update_docs_task
 from readthedocs.telemetry.models import BuildData
+from readthedocs.vcs_support.backends.git import Backend
 
 from .mockers import BuildEnvironmentMocker
 
@@ -60,17 +62,19 @@ def _get_project(self):
         return fixture.get(
             Project,
             slug="project",
+            repo="https://github.com/readthedocs/readthedocs.org",
             enable_epub_build=True,
             enable_pdf_build=True,
         )
 
-    def _trigger_update_docs_task(self):
+    def _trigger_update_docs_task(self, **kwargs):
         # NOTE: is it possible to replace calling this directly by `trigger_build` instead? :)
+        kwargs.setdefault("build_api_key", "1234")
+        kwargs.setdefault("build_commit", self.build.commit)
         return update_docs_task.delay(
             self.version.pk,
             self.build.pk,
-            build_api_key="1234",
-            build_commit=self.build.commit,
+            **kwargs,
         )
 
 class TestCustomConfigFile(BuildEnvironmentBase):
@@ -690,6 +694,72 @@ def test_failed_build(
         assert revoke_key_request._request.method == "POST"
         assert revoke_key_request.path == "/api/v2/revoke/"
 
+    @mock.patch.object(Backend, "ref_exists")
+    @mock.patch("readthedocs.doc_builder.director.load_yaml_config")
+    def test_checkout_tag_by_name(self, load_yaml_config, ref_exists):
+        ref_exists.return_value = False
+        self.version.type = TAG
+        self.version.identifier = "abc123"
+        self.version.verbose_name = "v1.0"
+        self.version.slug = "v1.0"
+        self.machine = False
+        self.version.save()
+        load_yaml_config.return_value = get_build_config({})
+
+        self._trigger_update_docs_task(build_commit=None)
+
+        self.mocker.mocks["git.Backend.run"].assert_has_calls(
+            [
+                mock.call("git", "clone", "--depth", "1", mock.ANY, "."),
+                mock.call(
+                    "git",
+                    "fetch",
+                    "origin",
+                    "--force",
+                    "--prune",
+                    "--prune-tags",
+                    "--depth",
+                    "50",
+                    "refs/tags/v1.0:refs/tags/v1.0",
+                ),
+                mock.call("git", "checkout", "--force", "v1.0"),
+                mock.call("git", "clean", "-d", "-f", "-f"),
+            ]
+        )
+
+    @mock.patch.object(Backend, "ref_exists")
+    @mock.patch("readthedocs.doc_builder.director.load_yaml_config")
+    def test_checkout_external_version_by_commit(self, load_yaml_config, ref_exists):
+        ref_exists.return_value = False
+        self.version.type = EXTERNAL
+        self.version.identifier = "abc123"
+        self.version.verbose_name = "22"
+        self.version.slug = "22"
+        self.machine = False
+        self.version.save()
+        load_yaml_config.return_value = get_build_config({})
+
+        self._trigger_update_docs_task(build_commit=None)
+
+        self.mocker.mocks["git.Backend.run"].assert_has_calls(
+            [
+                mock.call("git", "clone", "--depth", "1", mock.ANY, "."),
+                mock.call(
+                    "git",
+                    "fetch",
+                    "origin",
+                    "--force",
+                    "--prune",
+                    "--prune-tags",
+                    "--depth",
+                    "50",
+                    "pull/22/head:external-22",
+                ),
+                mock.call("git", "checkout", "--force", "abc123"),
+                mock.call("git", "clean", "-d", "-f", "-f"),
+            ]
+        )
+
     @mock.patch("readthedocs.doc_builder.director.load_yaml_config")
     def test_build_commands_executed(
         self,