From b7ab9d3d4ecd8ba8be6b31dac00c3e9abdc78bbb Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 26 Jul 2018 12:56:19 -0300 Subject: [PATCH 1/5] Support git unicode branches Use `gitpython` to get repository branches instead of our custom/hacky csv parser. With this change, we loose the output from `git branch -r` command shown to the user under the build output. --- readthedocs/rtd_tests/tests/test_backend.py | 52 ++++++++++---------- readthedocs/vcs_support/backends/git.py | 53 ++++----------------- 2 files changed, 36 insertions(+), 69 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 239c2dc8f57..0e9bdffeb16 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -49,30 +49,34 @@ def setUp(self): self.dummy_conf.submodules.include = ALL self.dummy_conf.submodules.exclude = [] - def test_parse_branches(self): - data = """ - develop - master - release/2.0.0 - origin/2.0.X - origin/HEAD -> origin/master - origin/master - origin/release/2.0.0 - origin/release/foo/bar - """ - - expected_ids = [ - ('develop', 'develop'), - ('master', 'master'), - ('release/2.0.0', 'release/2.0.0'), - ('origin/2.0.X', '2.0.X'), - ('origin/master', 'master'), - ('origin/release/2.0.0', 'release/2.0.0'), - ('origin/release/foo/bar', 'release/foo/bar'), + def test_git_branches(self): + repo_path = self.project.repo + default_branches = [ + # comes from ``make_test_git`` function + 'submodule', + 'relativesubmodule', + 'invalidsubmodule', ] - given_ids = [(x.identifier, x.verbose_name) for x in - self.project.vcs_repo().parse_branches(data)] - self.assertEqual(expected_ids, given_ids) + branches = [ + 'develop', + 'master', + '2.0.X', + 'release/2.0.0', + 'release/foo/bar', + 'release-ünîø∂é', + ] + for branch in branches: + create_git_branch(repo_path, branch) + + repo = self.project.vcs_repo() + # We aren't cloning the repo, + # so we need to hack the repo path + repo.working_dir = repo_path + + self.assertEqual( + set(branches + default_branches), + {branch.verbose_name for branch in repo.branches}, + ) def test_git_checkout(self): repo = self.project.vcs_repo() @@ -90,7 +94,7 @@ def test_git_tags(self): repo.working_dir = repo_path self.assertEqual( set(['v01', 'v02', 'release-ünîø∂é']), - set(vcs.verbose_name for vcs in repo.tags) + {vcs.verbose_name for vcs in repo.tags}, ) def test_check_for_submodules(self): diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 2959add5493..07584a310f6 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -174,51 +174,14 @@ def tags(self): @property def branches(self): - # Only show remote branches - retcode, stdout, _ = self.run( - 'git', - 'branch', - '-r', - record_as_success=True, - ) - # error (or no branches found) - if retcode != 0: - return [] - return self.parse_branches(stdout) - - def parse_branches(self, data): - """ - Parse output of git branch -r. - - e.g.: - - origin/2.0.X - origin/HEAD -> origin/master - origin/develop - origin/master - origin/release/2.0.0 - origin/release/2.1.0 - """ - clean_branches = [] - # StringIO below is expecting Unicode data, so ensure that it gets it. - if not isinstance(data, str): - data = str(data) - delimiter = str(' ').encode('utf-8') if PY2 else str(' ') - raw_branches = csv.reader(StringIO(data), delimiter=delimiter) - for branch in raw_branches: - branch = [f for f in branch if f not in ('', '*')] - # Handle empty branches - if branch: - branch = branch[0] - if branch.startswith('origin/'): - verbose_name = branch.replace('origin/', '') - if verbose_name in ['HEAD']: - continue - clean_branches.append( - VCSVersion(self, branch, verbose_name)) - else: - clean_branches.append(VCSVersion(self, branch, branch)) - return clean_branches + repo = git.Repo(self.working_dir) + versions = [] + for branch in repo.branches: + verbose_name = branch.name + if verbose_name.startswith('origin/'): + verbose_name = verbose_name.replace('origin/', '') + versions.append(VCSVersion(self, str(branch), verbose_name)) + return versions @property def commit(self): From c271cc7441698002a0fce0acdfe6ea01a658c7c0 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 20 Nov 2018 12:44:24 +0100 Subject: [PATCH 2/5] Adapt tests --- readthedocs/rtd_tests/tests/test_celery.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index 4e5ed8856e6..83a5fdbe815 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -124,7 +124,9 @@ def test_sync_repository(self): self.assertTrue(result.successful()) @patch('readthedocs.projects.tasks.api_v2') - def test_check_duplicate_reserved_version_latest(self, api_v2): + @patch('readthedocs.projects.models.Project.checkout_path') + def test_check_duplicate_reserved_version_latest(self, checkout_path, api_v2): + checkout_path.return_value = self.project.repo create_git_branch(self.repo, 'latest') create_git_tag(self.repo, 'latest') @@ -144,7 +146,9 @@ def test_check_duplicate_reserved_version_latest(self, api_v2): api_v2.project().sync_versions.post.assert_called() @patch('readthedocs.projects.tasks.api_v2') - def test_check_duplicate_reserved_version_stable(self, api_v2): + @patch('readthedocs.projects.models.Project.checkout_path') + def test_check_duplicate_reserved_version_stable(self, checkout_path, api_v2): + checkout_path.return_value = self.project.repo create_git_branch(self.repo, 'stable') create_git_tag(self.repo, 'stable') From 3e2de0ea6711e2b1fe8bc992182ece7335ae3fa4 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 20 Nov 2018 12:44:34 +0100 Subject: [PATCH 3/5] Cleanup --- readthedocs/vcs_support/backends/git.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 07584a310f6..1bdaf0d2a07 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -4,7 +4,6 @@ from __future__ import ( absolute_import, division, print_function, unicode_literals) -import csv import logging import os import re @@ -13,7 +12,6 @@ from builtins import str from django.core.exceptions import ValidationError from git.exc import BadName -from six import PY2, StringIO from readthedocs.config import ALL from readthedocs.projects.exceptions import RepositoryError From 59209aed7f288dde2de2dfcfbd6890447b5f49a3 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 21 Nov 2018 11:57:43 +0100 Subject: [PATCH 4/5] Consider local and remote branches on gitpython --- readthedocs/vcs_support/backends/git.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 1bdaf0d2a07..e8400df662e 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -174,10 +174,14 @@ def tags(self): def branches(self): repo = git.Repo(self.working_dir) versions = [] - for branch in repo.branches: + # ``repo.branches`` returns local branches and + # ``repo.remotes.origin.refs`` returns remote branches + for branch in repo.branches + repo.remotes.origin.refs: verbose_name = branch.name if verbose_name.startswith('origin/'): verbose_name = verbose_name.replace('origin/', '') + if verbose_name == 'HEAD': + continue versions.append(VCSVersion(self, str(branch), verbose_name)) return versions From 999a39b78208fe9df389799295ee26d21fa34819 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 21 Nov 2018 12:32:50 +0100 Subject: [PATCH 5/5] Include remote branches only if there is remotes --- readthedocs/vcs_support/backends/git.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index e8400df662e..69e0adae338 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -174,9 +174,14 @@ def tags(self): def branches(self): repo = git.Repo(self.working_dir) versions = [] + # ``repo.branches`` returns local branches and + branches = repo.branches # ``repo.remotes.origin.refs`` returns remote branches - for branch in repo.branches + repo.remotes.origin.refs: + 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/', '')