From dca51f3dea69b0b5163410d4442b78b054c27c29 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Mon, 19 Mar 2018 19:52:03 -0600 Subject: [PATCH 1/3] Add temporary method for skipping submodule checkout This adds a project feature that allows for a project to specify that they would like to skip submodule installation. Currently we are forcing all submodules to be checked out, so this fails on private submodules. Refs https://github.com/rtfd/readthedocs-build/issues/30 --- readthedocs/projects/models.py | 2 ++ readthedocs/rtd_tests/tests/test_backend.py | 19 ++++++++++++++++--- readthedocs/vcs_support/backends/git.py | 18 +++++++++++++++--- readthedocs/vcs_support/base.py | 1 + 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 9d30b37788a..004e93bb795 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1067,12 +1067,14 @@ def add_features(sender, **kwargs): USE_SETUPTOOLS_LATEST = 'use_setuptools_latest' ALLOW_DEPRECATED_WEBHOOKS = 'allow_deprecated_webhooks' PIP_ALWAYS_UPGRADE = 'pip_always_upgrade' + SKIP_SUBMODULES = 'skip_submodules' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), (USE_SETUPTOOLS_LATEST, _('Use latest version of setuptools')), (ALLOW_DEPRECATED_WEBHOOKS, _('Allow deprecated webhook views')), (PIP_ALWAYS_UPGRADE, _('Always run pip install --upgrade')), + (SKIP_SUBMODULES, _('Skip git submodule checkout')), ) projects = models.ManyToManyField( diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 724c2d56b29..4c020b2d78a 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -2,8 +2,9 @@ from os.path import exists from django.contrib.auth.models import User +import django_dynamic_fixture as fixture -from readthedocs.projects.models import Project +from readthedocs.projects.models import Project, Feature from readthedocs.rtd_tests.base import RTDTestCase from readthedocs.rtd_tests.utils import make_test_git, make_test_hg @@ -82,11 +83,23 @@ def test_check_for_submodules(self): repo = self.project.vcs_repo() repo.checkout() - self.assertFalse(repo.submodules_exists()) + self.assertFalse(repo.are_submodules_available()) # The submodule branch contains one submodule repo.checkout('submodule') - self.assertTrue(repo.submodules_exists()) + self.assertTrue(repo.are_submodules_available()) + + def test_skip_submodule_checkout(self): + repo = self.project.vcs_repo() + repo.checkout('submodule') + self.assertTrue(repo.are_submodules_available()) + feature = fixture.get( + Feature, + projects=[self.project], + feature_id=Feature.SKIP_SUBMODULES, + ) + self.assertTrue(self.project.has_feature(Feature.SKIP_SUBMODULES)) + self.assertFalse(repo.are_submodules_available()) class TestHgBackend(RTDTestCase): diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 5e8a9242d7c..91eda68907b 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -54,7 +54,18 @@ def repo_exists(self): code, _, _ = self.run('git', 'status', record=False) return code == 0 - def submodules_exists(self): + def are_submodules_available(self): + """Test whether git submodule checkout step should be performed + + .. note:: + Temporarily, we support skipping these steps as submodule step can + fail if using private submodules. This will eventually be + configureable with our YAML config. + """ + # TODO remove with https://github.com/rtfd/readthedocs-build/issues/30 + from readthedocs.projects.models import Feature + if self.project.has_feature(Feature.SKIP_SUBMODULES): + return False code, out, _ = self.run('git', 'submodule', 'status', record=False) return code == 0 and bool(out) @@ -199,8 +210,9 @@ def checkout(self, identifier=None): # Clean any remains of previous checkouts self.run('git', 'clean', '-d', '-f', '-f') - # Update submodules - if self.submodules_exists(): + # Update submodules, temporarily allow for skipping submodule checkout + # step for projects need more submodule configuration. + if self.are_submodules_available(): self.run('git', 'submodule', 'sync') self.run( 'git', diff --git a/readthedocs/vcs_support/base.py b/readthedocs/vcs_support/base.py index e7acc1b8e2f..b3e2c83e3cf 100644 --- a/readthedocs/vcs_support/base.py +++ b/readthedocs/vcs_support/base.py @@ -52,6 +52,7 @@ class BaseVCS(object): # pylint: disable=unused-argument def __init__(self, project, version_slug, environment=None, **kwargs): self.default_branch = project.default_branch + self.project = project self.name = project.name self.repo_url = project.clean_repo self.working_dir = project.checkout_path(version_slug) From 89ba11f582b6b096bb49cc3adb82b4724fe2ff95 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 22 Mar 2018 14:58:08 -0600 Subject: [PATCH 2/3] Also protect git clone --- readthedocs/vcs_support/backends/git.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 91eda68907b..45d336429d5 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -85,7 +85,20 @@ def checkout_revision(self, revision=None): return [code, out, err] def clone(self): - code, _, _ = self.run('git', 'clone', '--recursive', self.repo_url, '.') + """Clone the repository + + .. note:: + Temporarily, we support skipping submodule recursive clone via a + feature flag. This will eventually be configureable with our YAML + config. + """ + # TODO remove with https://github.com/rtfd/readthedocs-build/issues/30 + from readthedocs.projects.models import Feature + cmd = ['git', 'clone'] + if not self.project.has_feature(Feature.SKIP_SUBMODULES): + cmd.append('--recursive') + cmd.extend([self.repo_url, '.']) + code, _, _ = self.run(*cmd) if code != 0: raise RepositoryError From 5a8a24b0b0135fed5b4fd2d39197617e76480383 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 22 Mar 2018 15:08:10 -0600 Subject: [PATCH 3/3] Lint fixes --- readthedocs/vcs_support/backends/git.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 45d336429d5..7e3ba4ac32c 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -55,9 +55,11 @@ def repo_exists(self): return code == 0 def are_submodules_available(self): - """Test whether git submodule checkout step should be performed + """ + Test whether git submodule checkout step should be performed. .. note:: + Temporarily, we support skipping these steps as submodule step can fail if using private submodules. This will eventually be configureable with our YAML config. @@ -85,9 +87,11 @@ def checkout_revision(self, revision=None): return [code, out, err] def clone(self): - """Clone the repository + """ + Clone the repository. .. note:: + Temporarily, we support skipping submodule recursive clone via a feature flag. This will eventually be configureable with our YAML config.