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

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Jun 5, 2009, 08:10 UTC
Message-ID: <4A28D2EF.8040704@viscovery.net>
In-Reply-To: <20090605054500.06A9D21C3F4@mail.utsl.gen.nz>

sam@vilain.net 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.

Show 5 quoted lines

> +	/* 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)?

Show 6 quoted lines

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