launchpad-reviewers team mailing list archive

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

Thiago F. Pappacena has proposed merging ~pappacena/turnip:git-backend-stdin into turnip:master with ~pappacena/turnip:protocol-v2-parameters-parsing as a prerequisite.

Commit message:
Adding possibility to set env variable and send data directly to git subprocess' stdin

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Most of git protocol v2 communication with git subprocess happens by setting GIT_PROTOCOL env variable, and sending data from the wire protocol directly to `git upload-pack` stdin. This MP changes GitProcessProtocol to allow such operations when PackBackendProtocol uses it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:git-backend-stdin into turnip:master.

diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 763d5d6..6ae6dd5 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -239,15 +239,20 @@ class GitProcessProtocol(protocol.ProcessProtocol):
 
     _err_buffer = b''
 
-    def __init__(self, peer):
+    def __init__(self, peer, cmd_input=None):
         self.peer = peer
+        self.cmd_input = cmd_input
         self.out_started = False
 
     def connectionMade(self):
         self.peer.setPeer(self)
         self.peer.transport.registerProducer(self, True)
-        self.transport.registerProducer(
-            UnstoppableProducerWrapper(self.peer.transport), True)
+        if not self.cmd_input:
+            self.transport.registerProducer(
+                UnstoppableProducerWrapper(self.peer.transport), True)
+        else:
+            self.transport.write(self.cmd_input)
+            self.loseWriteConnection()
         self.peer.resumeProducing()
 
     def outReceived(self, data):
@@ -456,6 +461,9 @@ class PackBackendProtocol(PackServerProtocol):
             self.resumeProducing()
             return
 
+        send_path_as_option = False
+        cmd_input = None
+        cmd_env = {}
         write_operation = False
         if command == b'git-upload-pack':
             subcmd = b'upload-pack'
@@ -472,13 +480,16 @@ class PackBackendProtocol(PackServerProtocol):
         if params.pop(b'turnip-advertise-refs', None):
             args.append(b'--advertise-refs')
         args.append(self.path)
-        self.spawnGit(subcmd,
-                      args,
-                      write_operation=write_operation,
-                      auth_params=auth_params)
+        self.spawnGit(
+            subcmd, args,
+            write_operation=write_operation,
+            auth_params=auth_params,
+            send_path_as_option=send_path_as_option,
+            cmd_env=cmd_env, cmd_input=cmd_input)
 
     def spawnGit(self, subcmd, extra_args, write_operation=False,
-                 send_path_as_option=False, auth_params=None):
+                 send_path_as_option=False, auth_params=None,
+                 cmd_env=None, cmd_input=None):
         cmd = b'git'
         args = [b'git']
         if send_path_as_option:
@@ -487,6 +498,7 @@ class PackBackendProtocol(PackServerProtocol):
         args.extend(extra_args)
 
         env = {}
+        env.update((cmd_env or {}))
         if write_operation and self.factory.hookrpc_handler:
             # This is a write operation, so prepare config, hooks, the hook
             # RPC server, and the environment variables that link them up.
@@ -499,7 +511,7 @@ class PackBackendProtocol(PackServerProtocol):
             env[b'TURNIP_HOOK_RPC_KEY'] = self.hookrpc_key
 
         self.log.info('Spawning {args}', args=args)
-        self.peer = GitProcessProtocol(self)
+        self.peer = GitProcessProtocol(self, cmd_input)
         self.spawnProcess(cmd, args, env=env)
 
     def spawnProcess(self, cmd, args, env=None):
diff --git a/turnip/pack/tests/test_git.py b/turnip/pack/tests/test_git.py
index 8d4ed32..5adffb1 100644
--- a/turnip/pack/tests/test_git.py
+++ b/turnip/pack/tests/test_git.py
@@ -34,6 +34,7 @@ from turnip.pack import (
     git,
     helpers,
     )
+from turnip.pack.git import GitProcessProtocol
 from turnip.pack.tests.fake_servers import FakeVirtInfoService
 from turnip.pack.tests.test_hooks import MockHookRPCHandler
 from turnip.tests.compat import mock
@@ -49,6 +50,18 @@ class DummyPackServerProtocol(git.PackServerProtocol):
         self.test_request = (command, pathname, host)
 
 
+class TestGitProcessProtocol(TestCase):
+    def test_can_write_to_stdin_directly(self):
+        peer = mock.Mock()
+        transport = mock.Mock()
+        protocol = GitProcessProtocol(peer, b"this is the stdin")
+        protocol.transport = transport
+        protocol.connectionMade()
+        self.assertEqual(
+            [mock.call(b'this is the stdin', )],
+            transport.write.call_args_list)
+
+
 class TestPackServerProtocol(TestCase):
     """Test the base implementation of the git pack network protocol."""