diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 36da6188089..222e1bdc247 100644 --- a/readthedocs/projects/models.py +++ b/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, _( diff --git a/readthedocs/projects/tasks/mixins.py b/readthedocs/projects/tasks/mixins.py index 5d831e1aee1..3052bb9cc90 100644 --- a/readthedocs/projects/tasks/mixins.py +++ b/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 diff --git a/readthedocs/projects/tests/mockers.py b/readthedocs/projects/tests/mockers.py index 6ad92ea9db9..b0f71a64c36 100644 --- a/readthedocs/projects/tests/mockers.py +++ b/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( diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index cbe342b4f3a..f0437a00496 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/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, + ), ] ) diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index f581d5008a3..0da7916ec2f 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -1,7 +1,6 @@ import os import textwrap from os.path import exists -from tempfile import mkdtemp from unittest import mock from unittest.mock import Mock, patch @@ -14,12 +13,10 @@ 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.projects.models import Project from readthedocs.rtd_tests.utils import ( create_git_branch, create_git_tag, - delete_git_branch, - delete_git_tag, get_current_commit, get_git_latest_commit_hash, make_test_git, @@ -145,65 +142,6 @@ def test_git_lsremote_branches_only(self): {branch.verbose_name: branch.identifier for branch in repo_branches}, ) - @patch('readthedocs.projects.models.Project.checkout_path') - def test_git_branches(self, checkout_path): - repo_path = self.project.repo - default_branches = [ - # comes from ``make_test_git`` function - 'submodule', - 'invalidsubmodule', - ] - branches = [ - 'develop', - 'master', - '2.0.X', - 'release/2.0.0', - 'release/foo/bar', - ] - for branch in branches: - create_git_branch(repo_path, branch) - - # Create dir where to clone the repo - local_repo = os.path.join(mkdtemp(), 'local') - os.mkdir(local_repo) - checkout_path.return_value = local_repo - - repo = self.project.vcs_repo(environment=self.build_environment) - repo.clone() - - self.assertEqual( - {branch: branch for branch in default_branches + branches}, - {branch.verbose_name: branch.identifier for branch in repo.branches}, - ) - - @patch('readthedocs.projects.models.Project.checkout_path') - def test_git_branches_unicode(self, checkout_path): - repo_path = self.project.repo - default_branches = [ - # comes from ``make_test_git`` function - 'submodule', - 'invalidsubmodule', - ] - branches = [ - 'master', - 'release-ünîø∂é', - ] - for branch in branches: - create_git_branch(repo_path, branch) - - # Create dir where to clone the repo - local_repo = os.path.join(mkdtemp(), 'local') - os.mkdir(local_repo) - checkout_path.return_value = local_repo - - repo = self.project.vcs_repo(environment=self.build_environment) - repo.clone() - - self.assertEqual( - set(branches + default_branches), - {branch.verbose_name for branch in repo.branches}, - ) - def test_git_update_and_checkout(self): repo = self.project.vcs_repo(environment=self.build_environment) code, _, _ = repo.update() @@ -215,38 +153,6 @@ def test_git_update_and_checkout(self): self.assertTrue(exists(repo.working_dir)) - @patch('readthedocs.vcs_support.backends.git.Backend.fetch') - def test_git_update_with_external_version(self, fetch): - 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, - environment=self.build_environment, - ) - repo.update() - fetch.assert_called_once() - - def test_git_fetch_with_external_version(self): - 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, - 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(environment=self.build_environment) repo.update() @@ -258,159 +164,6 @@ def test_git_checkout_invalid_revision(self): RepositoryError.FAILED_TO_CHECKOUT.format(version), ) - def test_git_tags(self): - repo_path = self.project.repo - 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(environment=self.build_environment) - # We aren't cloning the repo, - # so we need to hack the repo path - repo.working_dir = repo_path - commit = get_current_commit(repo_path) - self.assertEqual( - {"v01": commit, "v02": commit, "release-ünîø∂é": commit}, - {tag.verbose_name: tag.identifier for tag in repo.tags}, - ) - - def test_check_for_submodules(self): - repo = self.project.vcs_repo(environment=self.build_environment) - - repo.update() - self.assertFalse(repo.are_submodules_available(self.dummy_conf)) - - # The submodule branch contains one submodule - repo.checkout('submodule') - self.assertTrue(repo.are_submodules_available(self.dummy_conf)) - - def test_skip_submodule_checkout(self): - 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(environment=self.build_environment) - repo.update() - repo.checkout('submodule') - self.assertTrue(repo.use_shallow_clone()) - fixture.get( - Feature, - projects=[self.project], - feature_id=Feature.DONT_SHALLOW_CLONE, - ) - self.assertTrue(self.project.has_feature(Feature.DONT_SHALLOW_CLONE)) - self.assertFalse(repo.use_shallow_clone()) - - def test_check_submodule_urls(self): - 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(environment=self.build_environment) - repo.update() - repo.checkout('invalidsubmodule') - with self.assertRaises(RepositoryError) as e: - repo.update_submodules(self.dummy_conf) - # `invalid` is created in `make_test_git` - # it's a url in ssh form. - self.assertEqual( - str(e.exception), - RepositoryError.INVALID_SUBMODULES.format(['invalid']), - ) - - def test_invalid_submodule_is_ignored(self): - repo = self.project.vcs_repo(environment=self.build_environment) - repo.update() - repo.checkout('submodule') - gitmodules_path = os.path.join(repo.working_dir, '.gitmodules') - - with open(gitmodules_path, "a") as f: - content = textwrap.dedent( - """ - [submodule "not-valid-path"] - path = not-valid-path - url = - """ - ) - f.write(content) - - valid, submodules = repo.validate_submodules(self.dummy_conf) - self.assertTrue(valid) - self.assertEqual(list(submodules), ['foobar']) - - @patch('readthedocs.projects.models.Project.checkout_path') - def test_fetch_clean_tags_and_branches(self, checkout_path): - upstream_repo = self.project.repo - create_git_tag(upstream_repo, 'v01') - create_git_tag(upstream_repo, 'v02') - create_git_branch(upstream_repo, 'newbranch') - - local_repo = os.path.join(mkdtemp(), 'local') - os.mkdir(local_repo) - checkout_path.return_value = local_repo - - repo = self.project.vcs_repo(environment=self.build_environment) - repo.clone() - - delete_git_tag(upstream_repo, 'v02') - delete_git_branch(upstream_repo, 'newbranch') - - # We still have all branches and tags in the local repo - self.assertEqual( - {'v01', 'v02'}, - {vcs.verbose_name for vcs in repo.tags}, - ) - self.assertEqual( - { - 'invalidsubmodule', 'master', 'submodule', 'newbranch', - }, - {vcs.verbose_name for vcs in repo.branches}, - ) - - repo.update() - - # We don't have the eliminated branches and tags in the local repo - self.assertEqual( - {'v01'}, - {vcs.verbose_name for vcs in repo.tags}, - ) - self.assertEqual( - { - 'invalidsubmodule', 'master', 'submodule', - }, - {vcs.verbose_name for vcs in repo.branches}, - ) - - -@mock.patch("readthedocs.doc_builder.environments.BuildCommand.save", mock.MagicMock()) -class TestGitBackendNew(TestGitBackend): - """ - Test the entire Git backend (with the GIT_CLONE_FETCH_CHECKOUT_PATTERN feature flag). - - This test class is intended to maintain all backwards compatibility when introducing new - git clone+fetch commands, hence inheriting from the former test class. - - Methods have been copied and adapted. - Once the feature ``GIT_CLONE_FETCH_CHECKOUT_PATTERN`` has become the default for all projects, - we can discard of TestGitBackend by moving the methods that aren't overwritten into this class - and renaming it. - """ - - def setUp(self): - super().setUp() - fixture.get( - Feature, - projects=[self.project], - feature_id=Feature.GIT_CLONE_FETCH_CHECKOUT_PATTERN, - ) - self.assertTrue( - self.project.has_feature(Feature.GIT_CLONE_FETCH_CHECKOUT_PATTERN) - ) - def test_check_for_submodules(self): """ Test that we can get a branch called 'submodule' containing a valid submodule. @@ -458,7 +211,7 @@ def test_check_submodule_urls(self): valid, _ = repo.validate_submodules(self.dummy_conf) self.assertTrue(valid) - @patch("readthedocs.vcs_support.backends.git.Backend.fetch_ng") + @patch("readthedocs.vcs_support.backends.git.Backend.fetch") def test_git_update_with_external_version(self, fetch): """Test that an external Version (PR) is correctly cloned and fetched.""" version = fixture.get( @@ -560,11 +313,6 @@ def test_skip_submodule_checkout(self): repo.checkout("submodule") self.assertTrue(repo.are_submodules_available(self.dummy_conf)) - def test_use_shallow_clone(self): - """A test that should be removed because shallow clones are the new default.""" - # We should not be calling test_use_shallow_clone - return True - 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) @@ -574,64 +322,9 @@ def test_git_fetch_with_external_version(self): environment=self.build_environment, ) repo.update() - code, _, _ = repo.fetch_ng() + code, _, _ = repo.fetch() self.assertEqual(code, 0) - def test_git_branches(self): - """ - Test a source repository with multiple branches, can be cloned and fetched. - - For each branch, we clone and fetch and check that we get exactly what we expect. - """ - repo_path = self.project.repo - branches = [ - "develop", - "master", - "2.0.X", - "release/2.0.0", - "release/foo/bar", - ] - for branch in branches: - create_git_branch(repo_path, branch) - - for branch in branches: - # Create a repo that we want to clone and fetch a specific branch for - repo = self.project.vcs_repo( - environment=self.build_environment, - version_identifier=branch, - version_type=BRANCH, - ) - # Because current behavior is to reuse already cloned destinations, we should - # clear the working dir instead of reusing it. - repo.make_clean_working_dir() - - repo.update() - - self.assertEqual( - set([branch, "master"]), - {branch.verbose_name for branch in repo.branches}, - ) - - def test_git_branches_unicode(self): - """Test to verify that we can clone+fetch a unicode branch name.""" - - # Add a branch to the repo. - # Note: It's assumed that the source repo is re-created in setUp() - create_git_branch(self.project.repo, "release-ünîø∂é") - - repo = self.project.vcs_repo( - environment=self.build_environment, - version_identifier="release-ünîø∂é", - version_type=BRANCH, - ) - repo.update() - - # Note here that the original default branch 'master' got created during the clone - self.assertEqual( - set(["release-ünîø∂é", "master"]), - {branch.verbose_name for branch in repo.branches}, - ) - def test_update_without_branch_name(self): """ Test that we get expected default branch when not specifying a branch name. diff --git a/readthedocs/vcs_support/backends/__init__.py b/readthedocs/vcs_support/backends/__init__.py index ff88fa587ec..1e4d033a56f 100644 --- a/readthedocs/vcs_support/backends/__init__.py +++ b/readthedocs/vcs_support/backends/__init__.py @@ -1,6 +1,5 @@ """Listing of all the VCS backends.""" -from __future__ import absolute_import -from . import bzr, hg, git, svn +from . import bzr, git, hg, svn backend_cls = { 'bzr': bzr.Backend, diff --git a/readthedocs/vcs_support/backends/bzr.py b/readthedocs/vcs_support/backends/bzr.py index e8600030a1b..04d214ad37b 100644 --- a/readthedocs/vcs_support/backends/bzr.py +++ b/readthedocs/vcs_support/backends/bzr.py @@ -1,4 +1,3 @@ - """Bazaar-related utilities.""" import csv @@ -16,23 +15,6 @@ class Backend(BaseVCS): supports_tags = True fallback_branch = '' - def update(self): - super().update() - if self.repo_exists(): - return self.up() - return self.clone() - - def repo_exists(self): - try: - code, _, _ = self.run('bzr', 'status', record=False) - return code == 0 - except RepositoryError: - return False - - def up(self): - self.run('bzr', 'revert') - return self.run('bzr', 'up') - def clone(self): self.make_clean_working_dir() try: diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 55328303c91..f44533b8557 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -4,10 +4,8 @@ from dataclasses import dataclass from typing import Iterable -import git import structlog from django.core.exceptions import ValidationError -from gitdb.util import hex_to_bin from readthedocs.builds.constants import BRANCH, EXTERNAL, TAG from readthedocs.config import ALL @@ -53,9 +51,6 @@ def __init__(self, *args, **kwargs): self.token = kwargs.get('token') self.repo_url = self._get_clone_url() - # While cloning, we can decide that we are not going to fetch anything - self._skip_fetch = False - def _get_clone_url(self): if '://' in self.repo_url: hacked_url = self.repo_url.split('://')[1] @@ -69,34 +64,14 @@ def _get_clone_url(self): # clone_url = 'git://%s' % (hacked_url) return self.repo_url - # TODO: Remove when removing GIT_CLONE_FETCH_CHECKOUT_PATTERN - def set_remote_url(self, url): - return self.run('git', 'remote', 'set-url', 'origin', url) - def update(self): """Clone and/or fetch remote repository.""" super().update() - from readthedocs.projects.models import Feature - if self.project.has_feature(Feature.GIT_CLONE_FETCH_CHECKOUT_PATTERN): - # New behavior: Clone is responsible for calling .repo_exists() and - # .make_clean_working_dir() - self.clone_ng() - - # TODO: We are still using return values in this function that are legacy. - # This should be either explained or removed. - return self.fetch_ng() - - # Old behavior - if self.repo_exists(): - self.set_remote_url(self.repo_url) - return self.fetch() - self.make_clean_working_dir() - # A fetch is always required to get external versions properly - if self.version_type == EXTERNAL: - self.clone() - return self.fetch() - return self.clone() + self.clone() + # TODO: We are still using return values in this function that are legacy. + # This should be either explained or removed. + return self.fetch() def get_remote_fetch_refspec(self): """ @@ -172,26 +147,7 @@ def get_remote_fetch_refspec(self): project_slug=self.project.slug, ) - def clone_ng(self): - """ - Performs the next-generation (ng) git clone operation. - - This method is used when GIT_CLONE_FETCH_CHECKOUT_PATTERN is on. - """ - # TODO: This seems to be legacy that can be removed. - # If the repository is already cloned, we don't do anything. - # It seems to originate from when a cloned repository was cached on disk, - # and so we can call call .update() several times in the same build. - if self.repo_exists(): - return - - # TODO: This seems to be legacy that can be removed. - # There shouldn't be cases where we are asked to - # clone the repo in a non-clean working directory. - # The prior call to repo_exists() will return if a repo already exist with - # unclear guarantees about whether that even needs to be a fully consistent clone. - self.make_clean_working_dir() - + def clone(self): # TODO: We should add "--no-checkout" in all git clone operations, except: # There exists a case of version_type=BRANCH without a branch name. # This case is relevant for building projects for the first time without knowing the name @@ -207,25 +163,7 @@ def clone_ng(self): except RepositoryError as exc: raise RepositoryError(RepositoryError.CLONE_ERROR()) from exc - def fetch_ng(self): - """ - Performs the next-generation (ng) git fetch operation. - - This method is used when GIT_CLONE_FETCH_CHECKOUT_PATTERN is on. - """ - - # When git clone does NOT add --no-checkout, it's because we are going - # to use the remote HEAD, so we don't have to fetch nor check out. - if self._skip_fetch: - log.info( - "Skipping git fetch", - version_identifier=self.version_identifier, - version_machine=self.version_machine, - version_verbose_name=self.verbose_name, - version_type=self.version_type, - ) - return - + def fetch(self): # --force: Likely legacy, it seems to be irrelevant to this usage # --prune: Likely legacy, we don't expect a previous fetch command to have run # --prune-tags: Likely legacy, we don't expect a previous fetch command to have run @@ -268,20 +206,6 @@ def fetch_ng(self): code, stdout, stderr = self.run(*cmd) return code, stdout, stderr - def repo_exists(self): - """Test if the current working directory is a top-level git repository.""" - # If we are at the top-level of a git repository, - # ``--show-prefix`` will return an empty string. - exit_code, stdout, _ = self.run( - "git", "rev-parse", "--show-prefix", record=False - ) - return exit_code == 0 and stdout.strip() == "" - - @property - def _repo(self): - """Get a `git.Repo` instance from the current `self.working_dir`.""" - return git.Repo(self.working_dir, expand_vars=False) - def are_submodules_available(self, config): """Test whether git submodule checkout step should be performed.""" submodules_in_config = ( @@ -357,30 +281,6 @@ def use_shallow_clone(self): from readthedocs.projects.models import Feature return not self.project.has_feature(Feature.DONT_SHALLOW_CLONE) - def fetch(self): - # --force lets us checkout branches that are not fast-forwarded - # https://github.com/readthedocs/readthedocs.org/issues/6097 - cmd = ['git', 'fetch', 'origin', - '--force', '--tags', '--prune', '--prune-tags'] - - if self.use_shallow_clone(): - cmd.extend(['--depth', str(self.repo_depth)]) - - if self.verbose_name and self.version_type == EXTERNAL: - - if self.project.git_provider_name == GITHUB_BRAND: - cmd.append( - GITHUB_PR_PULL_PATTERN.format(id=self.verbose_name) - ) - - if self.project.git_provider_name == GITLAB_BRAND: - cmd.append( - GITLAB_MR_PULL_PATTERN.format(id=self.verbose_name) - ) - - code, stdout, stderr = self.run(*cmd) - return code, stdout, stderr - def checkout_revision(self, revision): try: code, out, err = self.run('git', 'checkout', '--force', revision) @@ -390,39 +290,6 @@ def checkout_revision(self, revision): RepositoryError.FAILED_TO_CHECKOUT.format(revision), ) from exc - def clone(self): - """Clones the repository.""" - cmd = ['git', 'clone', '--no-single-branch'] - - if self.use_shallow_clone(): - cmd.extend(['--depth', str(self.repo_depth)]) - - cmd.extend([self.repo_url, '.']) - - try: - code, stdout, stderr = self.run(*cmd) - - # TODO: for those VCS providers that don't tell us the `default_branch` - # of the repository in the incoming webhook, - # we need to get it from the cloned repository itself. - # - # cmd = ['git', 'symbolic-ref', 'refs/remotes/origin/HEAD'] - # _, default_branch, _ = self.run(*cmd) - # default_branch = default_branch.replace('refs/remotes/origin/', '') - # - # The idea is to hit the APIv2 here to update the `latest` version with - # the `default_branch` we just got from the repository itself, - # after clonning it. - # However, we don't know the PK for the version we want to update. - # - # api_v2.version(pk).patch( - # {'default_branch': default_branch} - # ) - - return code, stdout, stderr - except RepositoryError as exc: - raise RepositoryError(RepositoryError.CLONE_ERROR()) from exc - def lsremote(self, include_tags=True, include_branches=True): """ Use ``git ls-remote`` to list branches and tags without cloning the repository. @@ -449,7 +316,11 @@ def lsremote(self, include_tags=True, include_branches=True): all_tags = {} light_tags = {} for line in stdout.splitlines(): - commit, ref = line.split(maxsplit=1) + try: + commit, ref = line.split(maxsplit=1) + except ValueError: + # Skip this line if we have a problem splitting the line + continue if ref.startswith("refs/heads/"): branch = ref.replace("refs/heads/", "", 1) branches.append(VCSVersion(self, branch, branch)) @@ -470,71 +341,10 @@ def lsremote(self, include_tags=True, include_branches=True): return branches, tags - @property - def tags(self): - versions = [] - repo = self._repo - - # Build a cache of tag -> commit - # GitPython is not very optimized for reading large numbers of tags - ref_cache = {} # 'ref/tags/' -> hexsha - # This code is the same that is executed for each tag in gitpython, - # we execute it only once for all tags. - for hexsha, ref in git.TagReference._iter_packed_refs(repo): - gitobject = git.Object.new_from_sha(repo, hex_to_bin(hexsha)) - if gitobject.type == 'commit': - ref_cache[ref] = str(gitobject) - elif gitobject.type == 'tag' and gitobject.object.type == 'commit': - ref_cache[ref] = str(gitobject.object) - - for tag in repo.tags: - if tag.path in ref_cache: - hexsha = ref_cache[tag.path] - else: - try: - hexsha = str(tag.commit) - except ValueError: - # ValueError: Cannot resolve commit as tag TAGNAME points to a - # blob object - use the `.object` property instead to access it - # This is not a real tag for us, so we skip it - # https://github.com/rtfd/readthedocs.org/issues/4440 - log.warning('Git tag skipped.', tag=tag, exc_info=True) - continue - - versions.append(VCSVersion(self, hexsha, str(tag))) - return versions - - @property - def branches(self): - repo = self._repo - versions = [] - branches = [] - - # ``repo.remotes.origin.refs`` returns remote branches - if repo.remotes: - branches += repo.remotes.origin.refs - - for branch in branches: - verbose_name = branch.name - if verbose_name.startswith("origin/"): - verbose_name = verbose_name.replace("origin/", "", 1) - if verbose_name == "HEAD": - continue - versions.append( - VCSVersion( - repository=self, - identifier=verbose_name, - verbose_name=verbose_name, - ) - ) - return versions - @property def commit(self): - if self.repo_exists(): - _, stdout, _ = self.run('git', 'rev-parse', 'HEAD', record=False) - return stdout.strip() - return None + _, stdout, _ = self.run("git", "rev-parse", "HEAD", record=False) + return stdout.strip() @property def submodules(self) -> Iterable[GitSubmodule]: diff --git a/readthedocs/vcs_support/backends/hg.py b/readthedocs/vcs_support/backends/hg.py index 1880ee44f16..44adaeef4d7 100644 --- a/readthedocs/vcs_support/backends/hg.py +++ b/readthedocs/vcs_support/backends/hg.py @@ -14,22 +14,8 @@ class Backend(BaseVCS): def update(self): super().update() - if self.repo_exists(): - return self.pull() return self.clone() - def repo_exists(self): - try: - code, _, _ = self.run('hg', 'status', record=False) - return code == 0 - except RepositoryError: - return False - - def pull(self): - self.run('hg', 'pull') - code, stdout, stderr = self.run('hg', 'update', '--clean') - return code, stdout, stderr - def clone(self): self.make_clean_working_dir() try: diff --git a/readthedocs/vcs_support/backends/svn.py b/readthedocs/vcs_support/backends/svn.py index 2d203659bd6..178d887266f 100644 --- a/readthedocs/vcs_support/backends/svn.py +++ b/readthedocs/vcs_support/backends/svn.py @@ -1,4 +1,3 @@ - """Subversion-related utilities.""" import csv @@ -28,32 +27,8 @@ def __init__(self, *args, **kwargs): def update(self): super().update() - if self.repo_exists(): - return self.up() return self.co() - def repo_exists(self): - # For some reason `svn status` gives me retcode 0 in non-svn - # directories that's why I use `svn info` here. - retcode, *_ = self.run('svn', 'info', record=False) - return retcode == 0 - - def up(self): - retcode = self.run('svn', 'revert', '--recursive', '.')[0] - if retcode != 0: - raise RepositoryError - retcode, out, err = self.run( - 'svn', - 'up', - '--accept', - 'theirs-full', - '--trust-server-cert', - '--non-interactive', - ) - if retcode != 0: - raise RepositoryError - return retcode, out, err - def co(self, identifier=None): self.make_clean_working_dir() if identifier: diff --git a/readthedocs/vcs_support/base.py b/readthedocs/vcs_support/base.py index 16a2e1b8417..fc56db002f9 100644 --- a/readthedocs/vcs_support/base.py +++ b/readthedocs/vcs_support/base.py @@ -43,7 +43,7 @@ class BaseVCS: supports_tags = False # Whether this VCS supports tags or not. supports_branches = False # Whether this VCS supports branches or not. - supports_submodules = False + supports_submodules = False # Whether this VCS supports submodules or not. # Whether this VCS supports listing remotes (branches, tags) without cloning supports_lsremote = False @@ -161,6 +161,3 @@ def update_submodules(self, config): :type config: readthedocs.config.BuildConfigBase """ raise NotImplementedError - - def repo_exists(self): - raise NotImplementedError diff --git a/requirements/deploy.txt b/requirements/deploy.txt index 0759a32f0ff..1efaf7e2dc8 100644 --- a/requirements/deploy.txt +++ b/requirements/deploy.txt @@ -189,12 +189,6 @@ funcy==1.18 # via # -r requirements/pip.txt # django-cacheops -gitdb==4.0.10 - # via - # -r requirements/pip.txt - # gitpython -gitpython==3.1.32 - # via -r requirements/pip.txt gunicorn==21.2.0 # via -r requirements/pip.txt idna==3.4 @@ -331,10 +325,6 @@ six==1.16.0 # unicode-slugify slumber==0.7.1 # via -r requirements/pip.txt -smmap==5.0.0 - # via - # -r requirements/pip.txt - # gitdb sqlparse==0.4.4 # via # -r requirements/pip.txt diff --git a/requirements/docker.txt b/requirements/docker.txt index 4785ef58942..a7a6c34954e 100644 --- a/requirements/docker.txt +++ b/requirements/docker.txt @@ -201,12 +201,6 @@ funcy==1.18 # via # -r requirements/pip.txt # django-cacheops -gitdb==4.0.10 - # via - # -r requirements/pip.txt - # gitpython -gitpython==3.1.32 - # via -r requirements/pip.txt gunicorn==21.2.0 # via -r requirements/pip.txt idna==3.4 @@ -360,10 +354,6 @@ six==1.16.0 # unicode-slugify slumber==0.7.1 # via -r requirements/pip.txt -smmap==5.0.0 - # via - # -r requirements/pip.txt - # gitdb sqlparse==0.4.4 # via # -r requirements/pip.txt diff --git a/requirements/pip.in b/requirements/pip.in index 66df6831a7d..cef10db3e24 100644 --- a/requirements/pip.in +++ b/requirements/pip.in @@ -81,8 +81,6 @@ django-allauth==0.51.0 requests-oauthlib -GitPython - # Search elasticsearch<8.0 elasticsearch-dsl<8.0 diff --git a/requirements/pip.txt b/requirements/pip.txt index 8f1dfada2ad..32299fba7e4 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -143,10 +143,6 @@ filelock==3.12.2 # via virtualenv funcy==1.18 # via django-cacheops -gitdb==4.0.10 - # via gitpython -gitpython==3.1.32 - # via -r requirements/pip.in gunicorn==21.2.0 # via -r requirements/pip.in idna==3.4 @@ -235,8 +231,6 @@ six==1.16.0 # unicode-slugify slumber==0.7.1 # via -r requirements/pip.in -smmap==5.0.0 - # via gitdb sqlparse==0.4.4 # via # django diff --git a/requirements/testing.txt b/requirements/testing.txt index 51381744c12..24fcf230705 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -192,12 +192,6 @@ funcy==1.18 # via # -r requirements/pip.txt # django-cacheops -gitdb==4.0.10 - # via - # -r requirements/pip.txt - # gitpython -gitpython==3.1.32 - # via -r requirements/pip.txt gunicorn==21.2.0 # via -r requirements/pip.txt idna==3.4 @@ -347,10 +341,6 @@ six==1.16.0 # unicode-slugify slumber==0.7.1 # via -r requirements/pip.txt -smmap==5.0.0 - # via - # -r requirements/pip.txt - # gitdb snowballstemmer==2.2.0 # via sphinx sphinx==7.1.2