From 538e291f9e8575974e36d0f8983f122bc2f6e689 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 10 Jul 2019 14:39:49 -0500 Subject: [PATCH 1/4] Better msg when gitpython fails Currently we show a generic message. Related: https://github.com/readthedocs/readthedocs.org/issues/4371 This can be removed when https://github.com/gitpython-developers/GitPython/pull/818/ gets merged. --- readthedocs/projects/exceptions.py | 6 +++++- readthedocs/rtd_tests/tests/test_backend.py | 15 +++++++++++++++ readthedocs/vcs_support/backends/git.py | 7 ++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/exceptions.py b/readthedocs/projects/exceptions.py index 85b439400df..eebc24dee38 100644 --- a/readthedocs/projects/exceptions.py +++ b/readthedocs/projects/exceptions.py @@ -38,7 +38,11 @@ class RepositoryError(BuildEnvironmentError): 'Private repositories are not supported.', ) - INVALID_SUBMODULES = _('One or more submodule URLs are not valid: {}.',) + INVALID_SUBMODULES = _('One or more submodule URLs are not valid: {}.') + INVALID_SUBMODULES_PATH = _( + 'One or more submodule paths are not valid. ' + 'Check that all your submodules in .gitmodules are used.' + ) DUPLICATED_RESERVED_VERSIONS = _( 'You can not have two versions with the name latest or stable.', diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 5618dd92151..063bab43d9c 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -189,6 +189,21 @@ def test_check_invalid_submodule_urls(self): RepositoryError.INVALID_SUBMODULES.format(['invalid']), ) + def test_invalid_submodule_path(self): + repo_path = self.project.repo + gitmodules_path = os.path.join(repo_path, '.gitmodules') + + with open(gitmodules_path, 'w+') as f: + content = textwrap.dedent(""" + [submodule "not-valid-path"] + path = not-valid-path + url = https://github.com/readthedocs/readthedocs.org + """) + f.write(content) + + with self.assertRaises(RepositoryError, msg=RepositoryError.INVALID_SUBMODULES_PATH): + repo.update_submodules(self.dummy_conf) + @patch('readthedocs.projects.models.Project.checkout_path') def test_fetch_clean_tags_and_branches(self, checkout_path): upstream_repo = self.project.repo diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index aa3adc6c2bc..b4c2b759219 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -105,7 +105,12 @@ def validate_submodules(self, config): Returns the list of invalid submodules. """ repo = git.Repo(self.working_dir) - submodules = {sub.path: sub for sub in repo.submodules} + try: + submodules = {sub.path: sub for sub in repo.submodules} + except InvalidGitRepositoryError: + raise RepositoryError( + RepositoryError.INVALID_SUBMODULES_PATH, + ) for sub_path in config.submodules.exclude: path = sub_path.rstrip('/') From 6c38dd798908e59f0895c951bc1f49a49d68c4c2 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 10 Jul 2019 14:44:22 -0500 Subject: [PATCH 2/4] Missing import --- readthedocs/rtd_tests/tests/test_backend.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 063bab43d9c..5dc4d3e6a6b 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -3,6 +3,7 @@ import os from os.path import exists from tempfile import mkdtemp +import textwrap import django_dynamic_fixture as fixture from django.contrib.auth.models import User From 12497b2cca0707ea011f566ac22d2386d6bcafd2 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 10 Jul 2019 14:55:58 -0500 Subject: [PATCH 3/4] Better approach --- readthedocs/rtd_tests/tests/test_backend.py | 2 ++ readthedocs/vcs_support/backends/git.py | 22 ++++++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 5dc4d3e6a6b..6d37873d586 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -202,6 +202,8 @@ def test_invalid_submodule_path(self): """) f.write(content) + repo = self.project.vcs_repo() + repo.working_dir = repo_path with self.assertRaises(RepositoryError, msg=RepositoryError.INVALID_SUBMODULES_PATH): repo.update_submodules(self.dummy_conf) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index b4c2b759219..89f2e9efee0 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -80,8 +80,7 @@ def are_submodules_available(self, config): return False # Keep compatibility with previous projects - repo = git.Repo(self.working_dir) - return bool(repo.submodules) + return bool(self.submodules) def validate_submodules(self, config): """ @@ -104,13 +103,7 @@ def validate_submodules(self, config): Returns `False` if at least one submodule is invalid. Returns the list of invalid submodules. """ - repo = git.Repo(self.working_dir) - try: - submodules = {sub.path: sub for sub in repo.submodules} - except InvalidGitRepositoryError: - raise RepositoryError( - RepositoryError.INVALID_SUBMODULES_PATH, - ) + submodules = {sub.path: sub for sub in self.submodules} for sub_path in config.submodules.exclude: path = sub_path.rstrip('/') @@ -227,6 +220,17 @@ def commit(self): return stdout.strip() return None + @property + def submodules(self): + try: + repo = git.Repo(self.working_dir) + return list(repo.submodules) + except InvalidGitRepositoryError: + raise RepositoryError( + RepositoryError.INVALID_SUBMODULES_PATH, + ) + + def checkout(self, identifier=None): """Checkout to identifier or latest.""" super().checkout() From e4333ec15f49798fea521865fbd364e754a2b8b8 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 10 Jul 2019 15:00:24 -0500 Subject: [PATCH 4/4] Spaaaaaaaaaace --- 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 89f2e9efee0..52bd210d7ee 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -230,7 +230,6 @@ def submodules(self): RepositoryError.INVALID_SUBMODULES_PATH, ) - def checkout(self, identifier=None): """Checkout to identifier or latest.""" super().checkout()