launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:set-symbolic-ref-subprocess-error into turnip:master.

Commit message:
Fix crash if "git symbolic-ref" fails

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

`message` is str, but `die` expects bytes.  Noticed in passing while reading production logs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:set-symbolic-ref-subprocess-error into turnip:master.

diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index a74f1e0..46ffe0c 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -694,7 +694,7 @@ class PackBackendProtocol(PackServerProtocol):
         if message is None:
             self.transport.loseConnection()
         else:
-            self.die(message)
+            self.die(message.encode('UTF-8'))
 
     def readConnectionLost(self):
         # Forward the closed stdin down the stack.
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index be972b2..9c51a86 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -874,6 +874,26 @@ class TestSmartHTTPFrontendFunctional(FrontendFunctionalTestMixin, TestCase):
         head_target = yield self.get_symbolic_ref(repo, b'HEAD')
         self.assertEqual(b'refs/heads/master', head_target)
 
+    @defer.inlineCallbacks
+    def test_turnip_set_symbolic_ref_subprocess_error(self):
+        repo = os.path.join(self.root, self.internal_name)
+        head_target = yield self.get_symbolic_ref(repo, b'HEAD')
+        self.assertEqual(b'refs/heads/master', head_target)
+        response = yield self.make_set_symbolic_ref_request(
+            b'HEAD outside-refs')
+        # This is a little weird since an error occurred, but it's
+        # consistent with other HTTP pack protocol responses.
+        self.assertEqual(200, response.code)
+        body = yield client.readBody(response)
+        payload, body = helpers.decode_packet(body)
+        self.assertEqual(
+            b'ERR fatal: Refusing to point HEAD outside of refs/\n\n', payload)
+        self.assertEqual(
+            (b'ERR git symbolic-ref exited with status 128\n', b''),
+            helpers.decode_packet(body))
+        head_target = yield self.get_symbolic_ref(repo, b'HEAD')
+        self.assertEqual(b'refs/heads/master', head_target)
+
 
 class TestSmartHTTPFrontendWithAuthFunctional(TestSmartHTTPFrontendFunctional):