Re: [PATCH] Shift object enumeration out of upload-pack

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 10, 2009, 07:20 UTC
Message-ID: <7vr5xssggc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <c77435a80906091650j7709d006j64749c9692c25b29@mail.gmail.com>

Nick Edelen <sirnot@gmail.com> writes:

Show 11 quoted lines

> Offload object enumeration in upload-pack to pack-objects, but fall
> back on internal revision walker for shallow interaction.   Aside from
> architecturally making more sense, this also leaves the door open for
> pack-objects to employ a revision cache mechanism.  Test t5530 updated
> in order to explicitly check both enumeration methods.
>
> Signed-off-by: Nick Edelen <sirnot@gmail.com>
> Acked-by: Nicolas Pitre <nico@cam.org>
>
> ---
>  err, I guess you wanted me to resubmit this?

Strictly speaking, _I_ don't, but _you_ might ;-)

I doubt that the proposed commit log message justifies the claim "architecturally making more sense" with concrete enough discussion, but I'll let it pass.

Your log message got much better this time, by hinting that this is a preparatory step to introduce the rev-cache mechanism. It is good way to defend a patch saying that it is not just a useless code churn, but it is a necessary step to get us closer to a more useful goal.

Show 17 quoted lines

> diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh
> index f5102b9..22eec24 100755
> --- a/t/t5530-upload-pack-error.sh
> +++ b/t/t5530-upload-pack-error.sh
> ...
> @@ -51,11 +52,20 @@ test_expect_success 'fsck fails' '
>  test_expect_success 'upload-pack fails due to error in rev-list' '
>
>  	! echo "0032want $(git rev-parse HEAD)
> -00000009done
> +0034shallow $(git rev-parse HEAD^)00000009done
>  0000" | git upload-pack . > /dev/null 2> output.err &&
>  	grep "waitpid (async) failed" output.err
>  '
>
> +test_expect_success 'upload-pack fails due to error in pack-objects
> enumeration' '

You have a wrapped line here. Please check the setting of your MUA, especially if you are planning to send more patches to the list in the future.

No need to resend; I fixed this up when applying.

Thanks.