Subject: Re: [WIP] Shift rev-list enumeration from upload-pack to pack-objects

From: Johannes Sixt <[email-masked]>
Date: Fri, 05 Jun 2009 10:10:23 +0200

[email-masked] schrieb:
> instead of using the internal revision walker and piping object refs
> to pack-objects this patch passes only the revs to pack-objects, which
> in turn handles both enumeration and packing.

I appreciate this move. We have one test failing in MinGW git (t5530.6)
because of the rev-list that is run using start_async(). Even though this
patch doesn't change that (the test case still uses the start_async()
path), it is one step closer to the solution.

> +	/* sending rev params to pack-objects directly is great, but unfortunately pack-objects 
> +	 * has no way of turning off thin pack generation.  this would be a relatively simple 
> +	 * addition, but as we also have to deal with shallow grafts and all it's simplest to 
> +	 * just resort to piping object refs.
> +	 */

You certainly will reformat comments like this to shorter lines, proper
capitalization, without trailing spaces, and adjust the style (initial /*
is on its own line)?

> @@ -181,6 +195,28 @@ static void create_pack_file(void)
>  	if (start_command(&pack_objects))
>  		die("git upload-pack: unable to fork git-pack-objects");
>  
> +	/* pass on revisions we (don't) want 
> +	 * (do we need to check the validity of pack_objects.in?)

No, you don't need to check. It's valid, or you would have died above.

FYI, with this patch MinGW git hangs in t5530.8; the test-case exercises
the new code path.

-- Hannes