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

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 07.08.2011 22:02, schrieb Johannes Sixt:
> Am 06.08.2011 11:40, schrieb René Scharfe:
>> 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.
> 
> *If* we are working around the CRLF conversion issue in git itself,
> wouldn't it be much simpler to just:
> 
> diff --git a/archive-tar.c b/archive-tar.c
> index 20af005..da3d98a 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -364,9 +364,9 @@ void init_tar_archiver(void)
>  	int i;
>  	register_archiver(&tar_archiver);
> 
> -	tar_filter_config("tar.tgz.command", "gzip -cn", NULL);
> +	tar_filter_config("tar.tgz.command", "gzip -cn | cat", NULL);
>  	tar_filter_config("tar.tgz.remote", "true", NULL);
> -	tar_filter_config("tar.tar.gz.command", "gzip -cn", NULL);
> +	tar_filter_config("tar.tar.gz.command", "gzip -cn | cat", NULL);
>  	tar_filter_config("tar.tar.gz.remote", "true", NULL);
>  	git_config(git_tar_config, NULL);
>  	for (i = 0; i < nr_tar_filters; i++) {
> 
> (provided that 'cat' magically does not suffer from the same problem,
> and I do think that it does not.)

The external cat can indeed be used.  We'd need to do that for user
supplied commands as well, though, like this (ugh):

diff --git a/archive-tar.c b/archive-tar.c
index 20af005..eaa9a1c 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -326,6 +326,9 @@ static int write_tar_filter_archive(const struct archiver *ar,
 		die("BUG: tar-filter archiver called with no filter defined");
 
 	strbuf_addstr(&cmd, ar->data);
+#ifdef WIN32
+	strbuf_addstr(&cmd, " | cat");
+#endif
 	if (args->compression_level >= 0)
 		strbuf_addf(&cmd, " -%d", args->compression_level);
 

> Anyway, I think it would be better to address the problem on the msysgit
> side. Perhaps by providing a gzip of a different vintage (e.g. a
> self-compiled one) that does not suffer from the CRLF conversion issue.

Users would probably also need sanitized versions of bzip2 and xz, and
perhaps more.

If MSYS could be asked to refrain from switching file descriptors to text
mode, e.g. by setting an environment variable, we could solve the issue
in a generic way instead.

René