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

[Jeff King <peff@peff.net>]

On Mon, Aug 01, 2011 at 04:45:03PM +0200, Erik Faye-Lund wrote:

> > 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?

If you're squashing, just make a note in the text, or add a Helped-by
header at the end.

> > 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?

Sorry, I wrote that comment, then totally changed how my patch was
implemented and failed to update the comment. :)

The arguments to git-archive actually get parsed in two places:

  1. cmd_archive passes them to decide on meta stuff like remote vs
     local

  2. write_archive has a parse_archive_args function to actually parse
     the arguments that both upload-archive and archive share

My patch puts the new argument in (1). We could put it in (2) and do
away with passing the flag all the way down the call stack. But it seems
conceptually cleaner to me to have it in (1) (and the diff is much
shorter, too).

> $ 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

Weird. What does j.tgz end up looking like? Is it empty, or does it have
bogus data in it? Does gzip actually get invoked at all? Try running
with GIT_TRACE=1. I don't suppose you guys have something like strace,
which might be helpful.

> 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?

I'm not sure that newlines would make a difference. We use straight
write() everywhere in the archive code, which should be binary safe.

-Peff