VCS: remove unused methods and make new Git pattern the default (#8968)

* VCS: remove unused methods to "update" an existing copy

* VCS backends: minor update to Git

* Lint: pre-commit

* VCS git: keep return code and stdout

* Minor updates

* VCS: make new Git pattern the default

Remove `GIT_CLONE_FETCH_CHECKOUT_PATTERN` feature flag and always use it. We
have been testing this new pattern and it has been working great.

* Remove `set_remote_url` which is not being used anymore

* Update some tests

* VCS: remove `GitPython`

We are not depending on GitPython anymore.
With the introduction of `git ls-remote` we don't require to parse the
repository anymore and GitPython is not required.

This commit also removes the methods `.branches` and `.tags` that are not used
anymore when using Git as VCS backend.

* Remove shallow clone repository test

* More defensive logic and tests updated

* Update common/

* Delete old tests

* Remove `_skip_fetch` that wasn't used

Author: humitos

readthedocs/projects/models.py

@@ -1961,7 +1961,6 @@ def add_features(sender, **kwargs):
     INDEX_FROM_HTML_FILES = 'index_from_html_files'
 
     # Build related features
-    GIT_CLONE_FETCH_CHECKOUT_PATTERN = "git_clone_fetch_checkout_pattern"
     HOSTING_INTEGRATIONS = "hosting_integrations"
     SCALE_IN_PROTECTION = "scale_in_prtection"
 
@@ -2090,12 +2089,6 @@ def add_features(sender, **kwargs):
                 "sources"
             ),
         ),
-        (
-            GIT_CLONE_FETCH_CHECKOUT_PATTERN,
-            _(
-                "Build: Use simplified and optimized git clone + git fetch + git checkout patterns"
-            ),
-        ),
         (
             HOSTING_INTEGRATIONS,
             _(

readthedocs/projects/tasks/mixins.py

@@ -41,18 +41,6 @@ def sync_versions(self, vcs_repository):
         # and just validate them trigger the task. All the other logic should
         # be done by the BuildDirector or the VCS backend. We should not
         # check this here and do not depend on ``vcs_repository``.
-
-        # Do not use ``ls-remote`` if the VCS does not support it or if we
-        # have already cloned the repository locally. The latter happens
-        # when triggering a normal build.
-        # Always use lsremote when we have GIT_CLONE_FETCH_CHECKOUT_PATTERN on
-        # and the repo supports lsremote.
-        # The new pattern does not fetch branch and tag data, so we always
-        # have to do ls-remote.
-        use_lsremote = vcs_repository.supports_lsremote and (
-            self.data.project.has_feature(Feature.GIT_CLONE_FETCH_CHECKOUT_PATTERN)
-            or not vcs_repository.repo_exists()
-        )
         sync_tags = vcs_repository.supports_tags and not self.data.project.has_feature(
             Feature.SKIP_SYNC_TAGS
         )
@@ -62,15 +50,14 @@ def sync_versions(self, vcs_repository):
         )
         tags = []
         branches = []
-        if use_lsremote:
+        if vcs_repository.supports_lsremote:
             branches, tags = vcs_repository.lsremote(
                 include_tags=sync_tags,
                 include_branches=sync_branches,
             )
 
-        # GIT_CLONE_FETCH_CHECKOUT_PATTERN: When the feature flag becomes default, we
-        # can remove this segment since lsremote is always on.
-        # We can even factor out the dependency to gitpython.
+        # Remove this block once we drop support for Bazaar, SVG and Mercurial.
+        # Since we will only support Git, lsremote will be always available.
         else:
             if sync_tags:
                 tags = vcs_repository.tags

readthedocs/projects/tests/mockers.py

@@ -140,25 +140,9 @@ def _mock_git_repository(self):
             return_value=self.project_repository_path,
         )
 
-        def _repo_exists_side_effect(*args, **kwargs):
-            if self._counter == 0:
-                # TODO: create a miniamal git repository nicely or mock `git.Repo` if possible
-                os.system(f'cd {self.project_repository_path} && git init')
-                self._counter += 1
-                return False
-
-            self._counter += 1
-            return True
-
-
         self.patches['git.Backend.make_clean_working_dir'] = mock.patch(
             'readthedocs.vcs_support.backends.git.Backend.make_clean_working_dir',
         )
-        self.patches['git.Backend.repo_exists'] = mock.patch(
-            'readthedocs.vcs_support.backends.git.Backend.repo_exists',
-            side_effect=_repo_exists_side_effect,
-        )
-
 
         # Make a the backend to return 3 submodules when asked
         self.patches['git.Backend.submodules'] = mock.patch(

readthedocs/projects/tests/test_build_tasks.py

@@ -710,8 +710,16 @@ def test_build_commands_executed(
 
         self.mocker.mocks["git.Backend.run"].assert_has_calls(
             [
+                mock.call("git", "clone", "--depth", "1", mock.ANY, "."),
                 mock.call(
-                    "git", "clone", "--no-single-branch", "--depth", "50", mock.ANY, "."
+                    "git",
+                    "fetch",
+                    "origin",
+                    "--force",
+                    "--prune",
+                    "--prune-tags",
+                    "--depth",
+                    "50",
                 ),
                 mock.call(
                     "git",
@@ -724,6 +732,15 @@ def test_build_commands_executed(
                 ),
                 mock.call("git", "checkout", "--force", "origin/a1b2c3"),
                 mock.call("git", "clean", "-d", "-f", "-f"),
+                mock.call(
+                    "git",
+                    "ls-remote",
+                    "--tags",
+                    "--heads",
+                    mock.ANY,
+                    demux=True,
+                    record=False,
+                ),
             ]
         )