[PATCH/RFC] remote-helpers: set up a pipe to read fast-import response

[Jonathan Nieder <jrnieder@gmail.com>]

From: Dmitry Ivankov <divanorama@gmail.com>
Date: Tue, 21 Jun 2011 20:52:34 +0600

Remote helpers may want to issue "ls" or "cat-blob" requests to
fast-import.  For example, the svn remote helper from the
svn-dump-fast-import project uses that capability.

The response should be fed back to remote helper; let it go to the
remote helper's standard input (command stream).

After listing the branches to import:

	'import' SP 'refs/heads/foo' LF
	'import' SP 'refs/heads/bar' LF
	'import' SP 'refs/heads/baz' LF
	LF

the transport-helper does not send any commands to the remote helper
until the remote helper sends the "done" command to fast-import, so
this does not result in interleaved output from the transport-helper
and fast-import.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,

Something like this patch was sitting in my tree gathering moss but I
quite like it so I thought I should send it out for feedback.

I can't seem to find any copy of this patch in the list archives.
Maybe I received it by private email.  Dmitry, do you remember better?

Can also be found at

  git://repo.or.cz/git/jrn.git topics/di/remote-helper-blob-access

and will be part of the svn-fe-pu branch there in the next push.

It is possible (likely, even) that this doesn't work on Windows since
file descriptors are not inherited by default, but it won't cause harm
until someone tries to use "ls" or "cat-blob" from a remote helper.
If someone wants to write tests for it, that would be very useful.
Otherwise, we can deal with it once the remote helper hits the tree.

Changes since v2:

 - new description
 - documented in remote-helpers manpage
 - don't waste so much space for fastimport->argv.  (I restrained
   myself from using malloc in place of calloc since that change
   probably belongs in a separate patch.)

Thoughts?  Bugs?  Improvements?

Thanks,
Jonathan

 Documentation/git-remote-helpers.txt |    5 ++++-
 transport-helper.c                   |    6 +++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 3a23477c..0cae14c7 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -133,7 +133,10 @@ Supported if the helper has the "push" capability.
 
 'import' <name>::
 	Produces a fast-import stream which imports the current value
-	of the named ref. It may additionally import other refs as
+	of the named ref. Standard input for the helper is connected
+	to the fast-import backend's cat-blob stream so the helper
+	can read responses from "ls" and "cat-blob" requests.
+	The helper may additionally import other refs as
 	needed to construct the history efficiently. The script writes
 	to a helper-specific private namespace. The value of the named
 	ref should be written to a location in this namespace derived
diff --git a/transport-helper.c b/transport-helper.c
index acfc88e3..4c547b9a 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -349,12 +349,16 @@ static int fetch_with_fetch(struct transport *transport,
 
 static int get_importer(struct transport *transport, struct child_process *fastimport)
 {
+	char buf[32];
+	struct helper_data *data = transport->data;
 	struct child_process *helper = get_helper(transport);
 	memset(fastimport, 0, sizeof(*fastimport));
 	fastimport->in = helper->out;
-	fastimport->argv = xcalloc(5, sizeof(*fastimport->argv));
+	fastimport->argv = xcalloc(4, sizeof(*fastimport->argv));
 	fastimport->argv[0] = "fast-import";
 	fastimport->argv[1] = "--quiet";
+	snprintf(buf, 32, "--cat-blob-fd=%d", data->helper->in);
+	fastimport->argv[2] = buf;
 
 	fastimport->git_cmd = 1;
 	return start_command(fastimport);
-- 
1.7.10