Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Aug 6, 2011, 09:40 UTC
Message-ID: <4E3D0C1D.9000807@lsrfire.ath.cx>
In-Reply-To: <4E388A55.6080606@kdbg.org>

Am 03.08.2011 01:37, schrieb Johannes Sixt:

Show 12 quoted lines

> Am 02.08.2011 20:13, schrieb Jeff King:
>> Hmm. So it's not _just_ the pipe vs file thing. What's different about
>> calling it from the shell, versus the way we call it from git-archive?
> 
> When the parent process of an MSYS process is itself an MSYS process,
> such as bash, then the child does not do its own
> binary-mode-vs.-text-mode detection, but just uses whatever it is told
> by the parent. This is achieved by MSYS's fork emulation.
> 
> But if the parent is a regular Windows program, such as git(-archive),
> then the autodection happens and file descriptors pointing to files are
> put into text mode.

So here's an ugly patch to implement an internal passthrough filter to avoid newline conversions. It makes the tar filter command (gzip etc.) write to a pipe instead of directly to a file.

The patch does this unconditionally, which is a waste on all unaffected systems, of course. We could #ifdef it out, but is there perhaps a nice way to integrate this functionality into run_command/finish_command?

René

---
 archive-tar.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 20af005..22d52c5 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -314,6 +314,25 @@ static int write_tar_archive(const struct archiver *ar,
 	return err;
 }
 
+static int cat_fn(int in, int out, void *data)
+{
+	for (;;) {
+		char buf[16 * 1024];
+		ssize_t got = xread(in, buf, sizeof(buf));
+		if (got == 0)
+			break;
+		if (got < 0) {
+			if (errno == EPIPE)
+				break;
+			die_errno("read errror");
+		}
+		write_or_die(out, buf, got);
+	}
+	close(in);
+	close(out);
+	return 0;
+}
+
 static int write_tar_filter_archive(const struct archiver *ar,
 				    struct archiver_args *args)
 {
@@ -321,6 +340,14 @@ static int write_tar_filter_archive(const struct archiver *ar,
 	struct child_process filter;
 	const char *argv[2];
 	int r;
+	struct async cat;
+
+	memset(&cat, 0, sizeof(cat));
+	cat.proc = cat_fn;
+	cat.in = -1;
+	cat.out = dup(1);
+	if (start_async(&cat))
+		die("unable to start passthrough filter");
 
 	if (!ar->data)
 		die("BUG: tar-filter archiver called with no filter defined");
@@ -335,6 +362,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
 	filter.argv = argv;
 	filter.use_shell = 1;
 	filter.in = -1;
+	filter.out = cat.in;
 
 	if (start_command(&filter) < 0)
 		die_errno("unable to start '%s' filter", argv[0]);
@@ -349,6 +377,9 @@ static int write_tar_filter_archive(const struct archiver *ar,
 	if (finish_command(&filter) != 0)
 		die("'%s' filter reported error", argv[0]);
 
+	if (finish_async(&cat))
+		die("passthrough filter reported error");
+
 	strbuf_release(&cmd);
 	return r;
 }