Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork
On Thu, Jul 28, 2011 at 7:02 PM, Jeff King <peff@peff.net> wrote:
Show 10 quoted lines
> On Thu, Jul 28, 2011 at 10:47:58AM -0600, Jeff King wrote:
>
>> Ah, nevermind. I see now; the command-line option interface to "git
>> archive" is not as rich as what you can pass via write_archive.
>>
>> I think you can get around it by adding an option to "git archive" to
>> indicate that we are filling a remote request, which is the only
>> extra information that my series adds.
>
> And that patch would look something like the one below.
This is the most straight forward way of doing this, thanks. I'll post a new version with this squashed in soon.
What's the proper way of attributing you for the work?
> You can also now
> drop the "remote" parameter to write_archive entirely, but I didn't do
> so here.
I'm not entirely sure how you propose we do this... do you mean by hoisting the relevant logic from write_archive up to cmd_archive or something?
> Applied on top of the merge of your series into next, this
> passes t5000.
Unfortunately, it doesn't pass on Windows, but this doesn't seem to be directly related to the remote-stuff: Here's the tests that fail:
$ make t5000-tar-tree *** t5000-tar-tree.sh *** ok 1 - populate workdir <snip> ok 53 - infer tgz from .tar.gz filename not ok - 54 extract tgz file # # $GUNZIP -c <j.tgz >j.tar && # test_cmp b.tar j.tar # not ok - 55 remote tar.gz is allowed by default # # git archive --remote=. --format=tar.gz HEAD >remote.tar.gz && # test_cmp j.tgz remote.tar.gz # ok 56 - remote tar.gz can be disabled # failed 2 among 56 test(s) 1..56 make: *** [t5000-tar-tree.sh] Error 1
It seems "git archive --format=tgz HEAD" is broken on Windows: $ git archive --format=tgz HEAD > j.tgz $ gunzip -c j.tgz > j.tar && echo ok
gzip: j.tgz: invalid compressed data--format violated
But if I perform the compression manually, the archive is fine: $ git archive --format=tar HEAD | gzip -cn > j.tgz $ gunzip -c j.tgz > j.tar && echo ok ok
So, the big question is, are all tar-filters broken on Windows? It seems not; the tests for the foo-filter works fine... any clues? Could it somehow be newline-related, perhaps?
Show 49 quoted lines
>
> -Peff
>
> ---
> diff --git a/builtin/archive.c b/builtin/archive.c
> index 883c009..6668340 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -85,6 +85,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
> const char *exec = "git-upload-archive";
> const char *output = NULL;
> const char *remote = NULL;
> + int is_remote = 0;
> struct option local_opts[] = {
> OPT_STRING('o', "output", &output, "file",
> "write the archive to this file"),
> @@ -92,6 +93,9 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
> "retrieve the archive from remote repository <repo>"),
> OPT_STRING(0, "exec", &exec, "cmd",
> "path to the remote git-upload-archive command"),
> + { OPTION_BOOLEAN, 0, "remote-request", &is_remote, NULL,
> + "indicate we are serving a remote request",
> + PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
> OPT_END()
> };
>
> @@ -106,5 +110,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
>
> setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
>
> - return write_archive(argc, argv, prefix, 1, output, 0);
> + return write_archive(argc, argv, prefix, 1, output, is_remote);
> }
> diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
> index 0c192b5..c57e8bd 100644
> --- a/builtin/upload-archive.c
> +++ b/builtin/upload-archive.c
> @@ -27,8 +27,9 @@ static void prepare_argv(const char **sent_argv, const char **argv)
> int len;
>
> /* put received options in sent_argv[] */
> - sent_argc = 1;
> + sent_argc = 2;
> sent_argv[0] = "archive";
> + sent_argv[1] = "--remote-request";
> for (p = buf;;) {
> /* This will die if not enough free space in buf */
> len = packet_read_line(0, p, (buf + sizeof buf) - p);
>
Erik Faye-Lund
