Merge pull request #4052 from stsewd/use-git-python-for-tags

Use gitpython for tags

readthedocs/rtd_tests/tests/test_backend.py

@@ -1,4 +1,6 @@
-from __future__ import absolute_import
+# -*- coding: utf-8 -*-
+
+from __future__ import absolute_import, unicode_literals
 from os.path import exists
 
 import pytest
@@ -9,7 +11,7 @@
 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
+from readthedocs.rtd_tests.utils import create_git_tag, make_test_git, make_test_hg
 
 
 class TestGitBackend(RTDTestCase):
@@ -57,29 +59,19 @@ def test_git_checkout(self):
         repo.checkout()
         self.assertTrue(exists(repo.working_dir))
 
-    def test_parse_git_tags(self):
-        data = """\
-            3b32886c8d3cb815df3793b3937b2e91d0fb00f1 refs/tags/2.0.0
-            bd533a768ff661991a689d3758fcfe72f455435d refs/tags/2.0.1
-            c0288a17899b2c6818f74e3a90b77e2a1779f96a refs/tags/2.0.2
-            a63a2de628a3ce89034b7d1a5ca5e8159534eef0 refs/tags/2.1.0.beta2
-            c7fc3d16ed9dc0b19f0d27583ca661a64562d21e refs/tags/2.1.0.rc1
-            edc0a2d02a0cc8eae8b67a3a275f65cd126c05b1 refs/tags/2.1.0.rc2
-            274a5a8c988a804e40da098f59ec6c8f0378fe34 refs/tags/release/foobar
-         """
-        expected_tags = [
-            ('3b32886c8d3cb815df3793b3937b2e91d0fb00f1', '2.0.0'),
-            ('bd533a768ff661991a689d3758fcfe72f455435d', '2.0.1'),
-            ('c0288a17899b2c6818f74e3a90b77e2a1779f96a', '2.0.2'),
-            ('a63a2de628a3ce89034b7d1a5ca5e8159534eef0', '2.1.0.beta2'),
-            ('c7fc3d16ed9dc0b19f0d27583ca661a64562d21e', '2.1.0.rc1'),
-            ('edc0a2d02a0cc8eae8b67a3a275f65cd126c05b1', '2.1.0.rc2'),
-            ('274a5a8c988a804e40da098f59ec6c8f0378fe34', 'release/foobar'),
-        ]
-
-        given_ids = [(x.identifier, x.verbose_name) for x in
-                     self.project.vcs_repo().parse_tags(data)]
-        self.assertEqual(expected_tags, given_ids)
+    def test_git_tags(self):
+        repo_path = self.project.repo
+        create_git_tag(repo_path, 'v01')
+        create_git_tag(repo_path, 'v02', annotated=True)
+        create_git_tag(repo_path, 'release-ünîø∂é')
+        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(['v01', 'v02', 'release-ünîø∂é']),
+            set(vcs.verbose_name for vcs in repo.tags)
+        )
 
     def test_check_for_submodules(self):
         repo = self.project.vcs_repo()

readthedocs/rtd_tests/utils.py

@@ -1,6 +1,6 @@
 """Utility functions for use in tests."""
 
-from __future__ import absolute_import
+from __future__ import absolute_import, unicode_literals
 
 import logging
 import subprocess
@@ -16,12 +16,12 @@
 log = logging.getLogger(__name__)
 
 
-def check_output(l, env=()):
-    if env == ():
-        output = subprocess.Popen(l, stdout=subprocess.PIPE).communicate()[0]
-    else:
-        output = subprocess.Popen(l, stdout=subprocess.PIPE,
-                                  env=env).communicate()[0]
+def check_output(command, env=None):
+    output = subprocess.Popen(
+        command, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+        env=env
+    ).communicate()[0]
+    log.info(output)
     return output
 
 
@@ -36,63 +36,76 @@ def make_test_git():
     chdir(directory)
 
     # Initialize and configure
-    # TODO: move the ``log.info`` call inside the ``check_output```
-    log.info(check_output(['git', 'init'] + [directory], env=env))
-    log.info(check_output(
+    check_output(['git', 'init'] + [directory], env=env)
+    check_output(
         ['git', 'config', 'user.email', '[email protected]'],
         env=env
-    ))
-    log.info(check_output(
+    )
+    check_output(
         ['git', 'config', 'user.name', 'Read the Docs'],
         env=env
-    ))
+    )
 
     # Set up the actual repository
-    log.info(check_output(['git', 'add', '.'], env=env))
-    log.info(check_output(['git', 'commit', '-m"init"'], env=env))
+    check_output(['git', 'add', '.'], env=env)
+    check_output(['git', 'commit', '-m"init"'], env=env)
 
     # Add fake repo as submodule. We need to fake this here because local path
     # URL are not allowed and using a real URL will require Internet to clone
     # the repo
-    log.info(check_output(['git', 'checkout', '-b', 'submodule', 'master'], env=env))
+    check_output(['git', 'checkout', '-b', 'submodule', 'master'], env=env)
     # https://stackoverflow.com/a/37378302/2187091
     mkdir(pjoin(directory, 'foobar'))
     gitmodules_path = pjoin(directory, '.gitmodules')
     with open(gitmodules_path, 'w') as fh:
         fh.write('''[submodule "foobar"]\n\tpath = foobar\n\turl = https://foobar.com/git\n''')
-    log.info(check_output(
+    check_output(
         [
             'git', 'update-index', '--add', '--cacheinfo', '160000',
             '233febf4846d7a0aeb95b6c28962e06e21d13688', 'foobar',
         ],
         env=env,
-    ))
-    log.info(check_output(['git', 'add', '.'], env=env))
-    log.info(check_output(['git', 'commit', '-m"Add submodule"'], env=env))
+    )
+    check_output(['git', 'add', '.'], env=env)
+    check_output(['git', 'commit', '-m"Add submodule"'], env=env)
 
     # Add a relative submodule URL in the relativesubmodule branch
-    log.info(check_output(['git', 'checkout', '-b', 'relativesubmodule', 'master'], env=env))
-    log.info(check_output(
+    check_output(['git', 'checkout', '-b', 'relativesubmodule', 'master'], env=env)
+    check_output(
         ['git', 'submodule', 'add', '-b', 'master', './', 'relativesubmodule'],
         env=env
-    ))
-    log.info(check_output(['git', 'add', '.'], env=env))
-    log.info(check_output(['git', 'commit', '-m"Add relative submodule"'], env=env))
+    )
+    check_output(['git', 'add', '.'], env=env)
+    check_output(['git', 'commit', '-m"Add relative submodule"'], env=env)
     # Add an invalid submodule URL in the invalidsubmodule branch
-    log.info(check_output(['git', 'checkout', '-b', 'invalidsubmodule', 'master'], env=env))
-    log.info(check_output(
+    check_output(['git', 'checkout', '-b', 'invalidsubmodule', 'master'], env=env)
+    check_output(
         ['git', 'submodule', 'add', '-b', 'master', './', 'invalidsubmodule'],
         env=env,
-    ))
-    log.info(check_output(['git', 'add', '.'], env=env))
-    log.info(check_output(['git', 'commit', '-m"Add invalid submodule"'], env=env))
+    )
+    check_output(['git', 'add', '.'], env=env)
+    check_output(['git', 'commit', '-m"Add invalid submodule"'], env=env)
 
     # Checkout to master branch again
-    log.info(check_output(['git', 'checkout', 'master'], env=env))
+    check_output(['git', 'checkout', 'master'], env=env)
     chdir(path)
     return directory
 
 
+def create_git_tag(directory, tag, annotated=False):
+    env = environ.copy()
+    env['GIT_DIR'] = pjoin(directory, '.git')
+    path = getcwd()
+    chdir(directory)
+
+    command = ['git', 'tag']
+    if annotated:
+        command.extend(['-a', '-m', 'Some tag'])
+    command.append(tag)
+    check_output(command, env=env)
+    chdir(path)
+
+
 def make_test_hg():
     directory = mkdtemp()
     path = getcwd()

readthedocs/vcs_support/backends/git.py

@@ -17,6 +17,7 @@
 from readthedocs.core.validators import validate_submodule_url
 from readthedocs.projects.exceptions import RepositoryError
 from readthedocs.vcs_support.base import BaseVCS, VCSVersion
+from builtins import str
 
 log = logging.getLogger(__name__)
 
@@ -122,46 +123,12 @@ def clone(self):
 
     @property
     def tags(self):
-        retcode, stdout, _ = self.run(
-            'git',
-            'show-ref',
-            '--tags',
-            record_as_success=True,
-        )
-        # error (or no tags found)
-        if retcode != 0:
-            return []
-        return self.parse_tags(stdout)
-
-    def parse_tags(self, data):
-        """
-        Parses output of show-ref --tags, eg:
-
-            3b32886c8d3cb815df3793b3937b2e91d0fb00f1 refs/tags/2.0.0
-            bd533a768ff661991a689d3758fcfe72f455435d refs/tags/2.0.1
-            c0288a17899b2c6818f74e3a90b77e2a1779f96a refs/tags/2.0.2
-            a63a2de628a3ce89034b7d1a5ca5e8159534eef0 refs/tags/2.1.0.beta2
-            c7fc3d16ed9dc0b19f0d27583ca661a64562d21e refs/tags/2.1.0.rc1
-            edc0a2d02a0cc8eae8b67a3a275f65cd126c05b1 refs/tags/2.1.0.rc2
-
-        Into VCSTag objects with the tag name as verbose_name and the commit
-        hash as identifier.
-        """
-        # parse the lines into a list of tuples (commit-hash, tag ref name)
-        # 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_tags = csv.reader(StringIO(data), delimiter=delimiter)
-        vcs_tags = []
-        for row in raw_tags:
-            row = [f for f in row if f != '']
-            if row == []:
-                continue
-            commit_hash, name = row
-            clean_name = name.replace('refs/tags/', '')
-            vcs_tags.append(VCSVersion(self, commit_hash, clean_name))
-        return vcs_tags
+        repo = git.Repo(self.working_dir)
+        versions = [
+            VCSVersion(self, str(tag.commit), str(tag))
+            for tag in repo.tags
+        ]
+        return versions
 
     @property
     def branches(self):

tox.ini

@@ -14,6 +14,7 @@ setenv =
     PYTHONPATH={toxinidir}/readthedocs:{toxinidir}
     DJANGO_SETTINGS_MODULE=readthedocs.settings.test
     LANG=C
+    LC_CTYPE=C.UTF-8
     DJANGO_SETTINGS_SKIP_LOCAL=True
 deps = -r{toxinidir}/requirements/testing.txt
 changedir = {toxinidir}/readthedocs