Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common
7 changes: 0 additions & 7 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1947,7 +1947,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"

Expand Down Expand Up @@ -2076,12 +2075,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,
_(
Expand Down
19 changes: 3 additions & 16 deletions readthedocs/projects/tasks/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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
Expand Down
16 changes: 0 additions & 16 deletions readthedocs/projects/tests/mockers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
19 changes: 18 additions & 1 deletion readthedocs/projects/tests/test_build_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
),
]
)

Expand Down
Loading