From a6208e1f1a896ac11047a8e97d0e99e212a95191 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 1 Aug 2023 12:36:06 +0200 Subject: [PATCH 1/6] Feature flag: remove `DONT_SHALLOW_CLONE` This can be done now by using `build.jobs.post_checkout`. See an example at https://docs.readthedocs.io/en/latest/build-customization.html#unshallow-git-clone --- readthedocs/projects/models.py | 5 --- readthedocs/rtd_tests/tests/test_backend.py | 13 -------- readthedocs/vcs_support/backends/git.py | 34 ++++++++------------- 3 files changed, 12 insertions(+), 40 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 834f55fa4b5..9a070e3fe5e 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1914,7 +1914,6 @@ def add_features(sender, **kwargs): SKIP_SPHINX_HTML_THEME_PATH = "skip_sphinx_html_theme_path" MKDOCS_THEME_RTD = "mkdocs_theme_rtd" API_LARGE_DATA = "api_large_data" - DONT_SHALLOW_CLONE = "dont_shallow_clone" UPDATE_CONDA_STARTUP = "update_conda_startup" CONDA_APPEND_CORE_REQUIREMENTS = "conda_append_core_requirements" ALL_VERSIONS_IN_HTML_CONTEXT = "all_versions_in_html_context" @@ -1963,10 +1962,6 @@ def add_features(sender, **kwargs): MKDOCS_THEME_RTD, _("MkDocs: Use Read the Docs theme for MkDocs as default theme"), ), - ( - DONT_SHALLOW_CLONE, - _("Build: Do not shallow clone when cloning git repos"), - ), ( API_LARGE_DATA, _("Build: Try alternative method of posting large data"), diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 9a124378a5c..d19443018de 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -289,19 +289,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): - 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() diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index ad86c1615dc..f991f716074 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -330,27 +330,20 @@ def validate_submodules(self, config): return False, invalid_submodules return True, submodules.keys() - def use_shallow_clone(self): - """ - Test whether shallow clone should be performed. - - .. note:: - - Temporarily, we support skipping this option as builds that rely on - git history can fail if using shallow clones. This should - eventually be configurable via the web UI. - """ - 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)]) + cmd = [ + "git", + "fetch", + "origin", + "--force", + "--tags", + "--prune", + "--prune-tags", + "--depth", + str(self.repo_depth), + ] if self.verbose_name and self.version_type == EXTERNAL: @@ -378,10 +371,7 @@ def checkout_revision(self, revision): 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 = ["git", "clone", "--no-single-branch", "--depth", str(self.repo_depth)] cmd.extend([self.repo_url, '.']) From 19d5a18c8695fd3ebe859601712acc4f18324631 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 2 Aug 2023 10:31:22 +0200 Subject: [PATCH 2/6] Use a different `start_time` to make the API call valid --- readthedocs/rtd_tests/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index f14e2c9ad00..e53921d0b72 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -405,7 +405,7 @@ def test_make_build_commands(self): "command": "$READTHEDOCS_VIRTUALENV_PATH/bin/python -m sphinx", "description": "Python and Sphinx command", "exit_code": 0, - "start_time": start_time, + "start_time": start_time + datetime.timedelta(seconds=1), "end_time": end_time, }, format='json', From f2c7a61e5165b1f7cfab9565cdb207b5e6205753 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 22 Aug 2023 19:50:49 +0200 Subject: [PATCH 3/6] Resolve merge conflict --- readthedocs/vcs_support/backends/git.py | 29 +++++++------------------ 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 93dfe998172..79d6737a891 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -264,30 +264,17 @@ def checkout_revision(self, revision): def clone(self): """Clones the repository.""" - cmd = ["git", "clone", "--no-single-branch", "--depth", str(self.repo_depth)] - - cmd.extend([self.repo_url, '.']) + # 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 + # of the default branch. Once this case has been made redundant, we can have + # --no-checkout for all clones. + # --depth 1: Shallow clone, fetch as little data as possible. + cmd = ["git", "clone", "--depth", "1", self.repo_url, "."] try: + # TODO: Explain or remove the return value 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 From dc4f83a3adf37afb58313208741dec3c9c8212b8 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 22 Aug 2023 19:51:54 +0200 Subject: [PATCH 4/6] Solve merge conflict --- readthedocs/rtd_tests/tests/test_backend.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index e73333e5f3b..0da7916ec2f 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -306,7 +306,6 @@ def test_skip_submodule_checkout(self): """Test that a submodule is listed as available.""" repo = self.project.vcs_repo( environment=self.build_environment, - version_identifier="release-ünîø∂é", version_type=BRANCH, version_identifier="submodule", ) From 99662107e217a723636891a4ce347afea70f7d71 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 22 Aug 2023 19:52:32 +0200 Subject: [PATCH 5/6] Move code to simplify diff --- readthedocs/vcs_support/backends/git.py | 34 ++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 79d6737a891..273e2f4994b 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -148,6 +148,23 @@ def get_remote_fetch_refspec(self): ) + def clone(self): + """Clones the repository.""" + # 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 + # of the default branch. Once this case has been made redundant, we can have + # --no-checkout for all clones. + # --depth 1: Shallow clone, fetch as little data as possible. + cmd = ["git", "clone", "--depth", "1", self.repo_url, "."] + + try: + # TODO: Explain or remove the return value + code, stdout, stderr = self.run(*cmd) + return code, stdout, stderr + except RepositoryError as exc: + raise RepositoryError(RepositoryError.CLONE_ERROR()) from exc + 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 @@ -262,23 +279,6 @@ def checkout_revision(self, revision): RepositoryError.FAILED_TO_CHECKOUT.format(revision), ) from exc - def clone(self): - """Clones the repository.""" - # 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 - # of the default branch. Once this case has been made redundant, we can have - # --no-checkout for all clones. - # --depth 1: Shallow clone, fetch as little data as possible. - cmd = ["git", "clone", "--depth", "1", self.repo_url, "."] - - try: - # TODO: Explain or remove the return value - code, stdout, stderr = self.run(*cmd) - 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. From 0213cca4dcb545b9d539e768d110cf83667f943e Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 22 Aug 2023 19:53:06 +0200 Subject: [PATCH 6/6] Remove double space --- readthedocs/vcs_support/backends/git.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 273e2f4994b..9893eb5468b 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -147,7 +147,6 @@ def get_remote_fetch_refspec(self): project_slug=self.project.slug, ) - def clone(self): """Clones the repository.""" # TODO: We should add "--no-checkout" in all git clone operations, except: