launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-worker-no-flush into lp:launchpad.

Commit message:
Don't expect a flush-pkt in the response to turnip-set-symbolic-ref.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-worker-no-flush/+merge/310801

turnip doesn't send one, which I think is consistent with other aspects of the git protocol (compare e.g. errors from git-upload-pack/git-receive-pack).  This didn't cause incorrect or failed imports, merely a bit of odd output in the import log.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-worker-no-flush into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/tests/servers.py'
--- lib/lp/codehosting/codeimport/tests/servers.py	2016-11-05 11:11:00 +0000
+++ lib/lp/codehosting/codeimport/tests/servers.py	2016-11-14 18:49:57 +0000
@@ -260,18 +260,15 @@
         line = self.proto.read_pkt_line()
         if line is None:
             self.proto.write_pkt_line(b"ERR Invalid set-symbolic-ref-line\n")
-            self.proto.write_pkt_line(None)
             return
         name, target = line.split(b" ", 1)
         if name != b"HEAD":
             self.proto.write_pkt_line(
                 b'ERR Symbolic ref name must be "HEAD"\n')
-            self.proto.write_pkt_line(None)
             return
         if target.startswith(b"-"):
             self.proto.write_pkt_line(
                 b'ERR Symbolic ref target may not start with "-"\n')
-            self.proto.write_pkt_line(None)
             return
         try:
             self.repo.refs.set_symbolic_ref(name, target)
@@ -279,7 +276,6 @@
             self.proto.write_pkt_line(b'ERR %s\n' % e)
         else:
             self.proto.write_pkt_line(b'ACK %s\n' % name)
-        self.proto.write_pkt_line(None)
 
 
 class HTTPGitServerThread(threading.Thread):

=== modified file 'lib/lp/codehosting/codeimport/worker.py'
--- lib/lp/codehosting/codeimport/worker.py	2016-11-12 21:08:52 +0000
+++ lib/lp/codehosting/codeimport/worker.py	2016-11-14 18:49:57 +0000
@@ -1050,14 +1050,16 @@
                 "Invalid Content-Type from server: %s" % content_type)
         content = io.BytesIO(response.content)
         proto = Protocol(content.read, None)
-        pkts = list(proto.read_pkt_seq())
-        if len(pkts) == 1 and pkts[0].rstrip(b"\n") == b"ACK HEAD":
+        pkt = proto.read_pkt_line()
+        if pkt is None:
+            raise GitProtocolError("Unexpected flush-pkt from server")
+        elif pkt.rstrip(b"\n") == b"ACK HEAD":
             pass
-        elif pkts and pkts[0].startswith(b"ERR "):
+        elif pkt.startswith(b"ERR "):
             raise GitProtocolError(
-                pkts[0][len(b"ERR "):].rstrip(b"\n").decode("UTF-8"))
+                pkt[len(b"ERR "):].rstrip(b"\n").decode("UTF-8"))
         else:
-            raise GitProtocolError("Unexpected packets %r from server" % pkts)
+            raise GitProtocolError("Unexpected packet %r from server" % pkt)
 
     def _deleteRefs(self, repository, pattern):
         """Delete all refs in `repository` matching `pattern`."""