launchpad-reviewers team mailing list archive

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

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

Commit message:
Improving compatibility of hook.py with python3.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/383754
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:hook-py3-compat into turnip:master.

diff --git a/turnip/compat/__init__.py b/turnip/compat/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/turnip/compat/__init__.py
diff --git a/turnip/compat/files.py b/turnip/compat/files.py
new file mode 100644
index 0000000..3f8bb22
--- /dev/null
+++ b/turnip/compat/files.py
@@ -0,0 +1,21 @@
+import sys
+
+
+def fd_buffer(file_descriptor):
+    """Returns the raw bytes stream for the given file descriptor.
+
+    On Python2, it returns the file descriptor itself (since it reads raw
+    binary data by default). On Python3, it returns the
+    file_descriptor.buffer object (since py3, by default, opens files with
+    an encoding).
+
+    It's useful to read and write from sys.std{in,out,err} without reopening
+    those files, for example.
+
+    :param file_descriptor: The file descriptor to get raw buffer from.
+    :return: A BufferedReader or BufferedWriter object."""
+    PY3K = sys.version_info >= (3, 0)
+    if PY3K:
+        return file_descriptor.buffer
+    else:
+        return file_descriptor
diff --git a/turnip/pack/hooks/hook.py b/turnip/pack/hooks/hook.py
index e4b6886..aa6e4a4 100755
--- a/turnip/pack/hooks/hook.py
+++ b/turnip/pack/hooks/hook.py
@@ -12,10 +12,13 @@ from __future__ import (
 import base64
 import json
 import os
+import six
 import socket
 import subprocess
 import sys
 
+from turnip.compat.files import fd_buffer
+
 # XXX twom 2018-10-23 This should be a pygit2 import, but
 # that currently causes CFFI warnings to be returned to the client.
 GIT_OID_HEX_ZERO = '0'*40
@@ -44,7 +47,8 @@ def determine_permissions_outcome(old, ref, rule_lines):
         if 'create' in rule:
             return
         else:
-            return b'You do not have permission to create ' + ref + b'.'
+            return (b'You do not have permission to create %s.' %
+                    six.ensure_binary(ref, "UTF-8"))
     # We have force-push permission, implies push, therefore okay
     if 'force_push' in rule:
         return
@@ -53,7 +57,8 @@ def determine_permissions_outcome(old, ref, rule_lines):
     if 'push' in rule:
         return
     # If we're here, there are no matching rules
-    return b"You do not have permission to push to " + ref + b"."
+    return (b"You do not have permission to push to %s." %
+            six.ensure_binary(ref, "UTF-8"))
 
 
 def match_rules(rule_lines, ref_lines):
@@ -95,7 +100,8 @@ def match_update_rules(rule_lines, ref_line):
     rule = rule_lines.get(ref, [])
     if 'force_push' in rule:
         return []
-    return [b'You do not have permission to force-push to ' + ref + b'.']
+    return [b'You do not have permission to force-push to %s.' %
+            six.ensure_binary(ref, "UTF-8")]
 
 
 def netstring_send(sock, s):
@@ -104,7 +110,7 @@ def netstring_send(sock, s):
 
 def netstring_recv(sock):
     c = sock.recv(1)
-    lengthstr = ''
+    lengthstr = b''
     while c != b':':
         assert c.isdigit()
         lengthstr += c
@@ -121,7 +127,7 @@ def rpc_invoke(sock, method, args):
     msg = dict(args)
     assert 'op' not in msg
     msg['op'] = method
-    netstring_send(sock, json.dumps(msg))
+    netstring_send(sock, six.ensure_binary(json.dumps(msg), 'UTF-8'))
     res = json.loads(netstring_recv(sock))
     if 'error' in res:
         raise Exception(res)
@@ -131,7 +137,7 @@ def rpc_invoke(sock, method, args):
 def check_ref_permissions(sock, rpc_key, ref_paths):
     ref_paths = [base64.b64encode(path).decode('UTF-8') for path in ref_paths]
     rule_lines = rpc_invoke(
-        sock, b'check_ref_permissions',
+        sock, 'check_ref_permissions',
         {'key': rpc_key, 'paths': ref_paths})
     return {
         base64.b64decode(path.encode('UTF-8')): permissions
@@ -148,38 +154,42 @@ def send_mp_url(received_line):
         pushed_branch = ref[len(b'refs/heads/'):]
         if not is_default_branch(pushed_branch):
             mp_url = rpc_invoke(
-                sock, b'get_mp_url',
-                {'key': rpc_key, 'branch': pushed_branch})
+                sock, 'get_mp_url',
+                {'key': rpc_key,
+                 'branch': six.ensure_text(pushed_branch, "UTF-8")})
             if mp_url is not None:
-                sys.stdout.write(b'      \n')
-                sys.stdout.write(
+                stdout = fd_buffer(sys.stdout)
+                stdout.write(b'      \n')
+                stdout.write(
                     b"Create a merge proposal for '%s' on Launchpad by"
                     b" visiting:\n" % pushed_branch)
-                sys.stdout.write(b'      %s\n' % mp_url)
-                sys.stdout.write(b'      \n')
+                stdout.write(b'      %s\n' % six.ensure_binary(mp_url, "UTF8"))
+                stdout.write(b'      \n')
 
 if __name__ == '__main__':
     # Connect to the RPC server, authenticating using the random key
     # from the environment.
+    stdin = fd_buffer(sys.stdin)
+    stdout = fd_buffer(sys.stdout)
     rpc_key = os.environ['TURNIP_HOOK_RPC_KEY']
     sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
     sock.connect(os.environ['TURNIP_HOOK_RPC_SOCK'])
     hook = os.path.basename(sys.argv[0])
     if hook == 'pre-receive':
         # Verify the proposed changes against rules from the server.
-        raw_paths = sys.stdin.readlines()
+        raw_paths = stdin.readlines()
         ref_paths = [p.rstrip(b'\n').split(b' ', 2)[2] for p in raw_paths]
         rule_lines = check_ref_permissions(sock, rpc_key, ref_paths)
         errors = match_rules(rule_lines, raw_paths)
         for error in errors:
-            sys.stdout.write(error + b'\n')
+            stdout.write(error + b'\n')
         sys.exit(1 if errors else 0)
     elif hook == 'post-receive':
         # Notify the server about the push if there were any changes.
         # Details of the changes aren't currently included.
-        lines = sys.stdin.readlines()
+        lines = stdin.readlines()
         if lines:
-            rpc_invoke(sock, b'notify_push', {'key': rpc_key})
+            rpc_invoke(sock, 'notify_push', {'key': rpc_key})
         if len(lines) == 1:
             send_mp_url(lines[0])
         sys.exit(0)
@@ -188,7 +198,7 @@ if __name__ == '__main__':
         rule_lines = check_ref_permissions(sock, rpc_key, [ref])
         errors = match_update_rules(rule_lines, sys.argv[1:4])
         for error in errors:
-            sys.stdout.write(error + b'\n')
+            stdout.write(error + b'\n')
         sys.exit(1 if errors else 0)
     else:
         sys.stderr.write('Invalid hook name: %s' % hook)