launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/turnip:py3-strings-compat into turnip:master.

Commit message:
Making some strings compatible both with python2 and python3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/380451

This MP should only be merged once we have the new dependencies in place.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:py3-strings-compat into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 11b4756..2179f27 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -12,6 +12,7 @@ import re
 import shutil
 import subprocess
 import uuid
+from six import ensure_binary
 
 from pygit2 import (
     GitError,
@@ -333,14 +334,21 @@ def repack(repo_path, ignore_alternates=False, single=False,
 
 def get_refs(repo_store, repo_name, exclude_prefixes=None):
     """Return all refs for a git repository."""
+    if exclude_prefixes:
+        # Convert exclude_prefixes to bytes, since refs will be bytes too.
+        exclude_prefixes = [ensure_binary(i, 'utf-8')
+                            for i in exclude_prefixes]
     with open_repo(repo_store, repo_name) as repo:
         refs = {}
-        for ref in repo.listall_references():
-            git_object = repo.references[ref].peel()
+        for ref_obj in repo.listall_reference_objects():
             # Filter non-unicode refs, as refs are treated as unicode
             # given json is unable to represent arbitrary byte strings.
             try:
-                ref = ref.decode('utf-8')
+                if hasattr(ref_obj.name, 'decode'):  # py2
+                    ref = ref_obj.name.decode('utf-8')
+                else:  # py3
+                    ref = bytes(ref_obj.name, 'utf-8')
+                git_object = repo.references[ref].peel()
             except UnicodeDecodeError:
                 pass
             else:
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index 2ff34d0..495f28f 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -10,6 +10,7 @@ import os
 import subprocess
 from textwrap import dedent
 import unittest
+
 try:
     from urllib.parse import quote
 except ImportError:
@@ -217,7 +218,7 @@ class ApiTestCase(TestCase):
         """Ensure non-unicode refs are dropped from ref collection."""
         factory = RepoFactory(self.repo_store)
         commit_oid = factory.add_commit('foo', 'foobar.txt')
-        tag = '\xe9\xe9\xe9'  # latin-1
+        tag = b'\xe9\xe9\xe9'  # latin-1
         tag_message = 'tag message'
         factory.add_tag(tag, tag_message, commit_oid)
 
@@ -335,7 +336,7 @@ class ApiTestCase(TestCase):
     def test_repo_diff_non_unicode_commits(self):
         """Ensure non utf-8 chars are handled but stripped from diff."""
         factory = RepoFactory(self.repo_store)
-        message = 'not particularly sensible latin-1: \xe9\xe9\xe9.'
+        message = b'not particularly sensible latin-1: \xe9\xe9\xe9.'
         oid = factory.add_commit(message, 'foo.py')
         oid2 = factory.add_commit('a sensible commit message', 'foo.py', [oid])
 
@@ -406,7 +407,7 @@ class ApiTestCase(TestCase):
             self.repo_path, quote('{}^'.format(c2)), c2)
         resp = self.app.get(path)
         self.assertIn(
-            b'diff --git a/foo.txt b/bar.txt\n', resp.json_body['patch'])
+            'diff --git a/foo.txt b/bar.txt\n', resp.json_body['patch'])
 
     def test_repo_diff_rename_and_change_content_conflict(self):
         # Create repo1 with foo.txt.
@@ -576,7 +577,7 @@ class ApiTestCase(TestCase):
         resp = self.app.get('/repo/{}/compare-merge/{}:{}'.format(
             self.repo_path, c1, c2))
         self.assertIn(
-            b'diff --git a/foo.txt b/bar.txt\n', resp.json_body['patch'])
+            'diff --git a/foo.txt b/bar.txt\n', resp.json_body['patch'])
 
     def test_repo_get_commit(self):
         factory = RepoFactory(self.repo_store)
@@ -637,7 +638,7 @@ class ApiTestCase(TestCase):
         """Ensure signatures are correct."""
         factory = RepoFactory(self.repo_store)
         committer = factory.makeSignature(u'村上 春樹'.encode('utf-8'),
-                                          u'tsukuru@猫の町.co.jp'.encode('utf-8'),
+                                          u'tsukuru@猫の町.co.jp'.encode('utf8'),
                                           encoding='utf-8')
         author = factory.makeSignature(
             u'Владимир Владимирович Набоков'.encode('utf-8'),
@@ -671,7 +672,7 @@ class ApiTestCase(TestCase):
     def test_repo_get_non_unicode_log(self):
         """Ensure that non-unicode data is discarded."""
         factory = RepoFactory(self.repo_store)
-        message = '\xe9\xe9\xe9'  # latin-1
+        message = b'\xe9\xe9\xe9'  # latin-1
         oid = factory.add_commit(message, 'foo.py')
         resp = self.app.get('/repo/{}/log/{}'.format(self.repo_path, oid))
         self.assertEqual(message.decode('utf-8', 'replace'),
@@ -809,15 +810,15 @@ class ApiTestCase(TestCase):
         factory.add_commit('b\n', 'dir/file', parents=[c1])
         resp = self.app.get('/repo/{}/blob/dir/file'.format(self.repo_path))
         self.assertEqual(2, resp.json['size'])
-        self.assertEqual('b\n', base64.b64decode(resp.json['data']))
+        self.assertEqual(b'b\n', base64.b64decode(resp.json['data']))
         resp = self.app.get('/repo/{}/blob/dir/file?rev=master'.format(
             self.repo_path))
         self.assertEqual(2, resp.json['size'])
-        self.assertEqual('b\n', base64.b64decode(resp.json['data']))
+        self.assertEqual(b'b\n', base64.b64decode(resp.json['data']))
         resp = self.app.get('/repo/{}/blob/dir/file?rev={}'.format(
             self.repo_path, c1.hex))
         self.assertEqual(2, resp.json['size'])
-        self.assertEqual('a\n', base64.b64decode(resp.json['data']))
+        self.assertEqual(b'a\n', base64.b64decode(resp.json['data']))
 
     def test_repo_blob_missing_commit(self):
         """Trying to get a blob from a non-existent commit returns HTTP 404."""
@@ -861,7 +862,7 @@ class ApiTestCase(TestCase):
         resp = self.app.get('/repo/{}/blob/dir/file?rev=tag-name'.format(
             self.repo_path))
         self.assertEqual(2, resp.json['size'])
-        self.assertEqual('a\n', base64.b64decode(resp.json['data']))
+        self.assertEqual(b'a\n', base64.b64decode(resp.json['data']))
 
     def test_repo_blob_from_non_commit(self):
         """Trying to get a blob from a non-commit returns HTTP 404."""
diff --git a/turnip/api/tests/test_helpers.py b/turnip/api/tests/test_helpers.py
index dac6950..cc2e471 100644
--- a/turnip/api/tests/test_helpers.py
+++ b/turnip/api/tests/test_helpers.py
@@ -5,15 +5,16 @@ import contextlib
 import fnmatch
 import itertools
 import os
+import subprocess
 import uuid
 
+import six
 from six.moves import urllib
 
 from pygit2 import (
     clone_repository,
     init_repository,
     GIT_FILEMODE_BLOB,
-    GIT_OBJ_COMMIT,
     IndexEntry,
     Repository,
     Signature,
@@ -96,12 +97,16 @@ class RepoFactory(object):
 
     def add_tag(self, tag_name, tag_message, oid):
         """Create a tag from tag_name and oid."""
-        repo = self.repo
-        repo.create_tag(tag_name, oid, GIT_OBJ_COMMIT,
-                        self.committer, tag_message)
+        subprocess.check_call(
+            ['git', '-C', self.repo_path,
+             'tag', '-m', tag_message, tag_name, oid.hex])
 
     def makeSignature(self, name, email, encoding='utf-8'):
         """Return an author or committer signature."""
+        # xxx: pappacena 2020-03-09:
+        # email should always be str on python3, but pygit2
+        # doesn't enforce the same for name.
+        email = six.ensure_str(email)
         return Signature(name, email, encoding=encoding)
 
     def stage(self, path, content):