Re: [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack

From: Stefan Beller <sbeller@google.com>
Date: Jun 2, 2015, 23:08 UTC
Message-ID: <CAGZ79kbe+1okiUL5bGcfykyn7MmhALF+3ANY9k-ycadk0RNAuw@mail.gmail.com>
In-Reply-To: <xmqqfv6a2ayx.fsf@gitster.dls.corp.google.com>

On Tue, Jun 2, 2015 at 11:59 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> Subject: [RFCv2 04/16] upload-pack-2: Implement the version 2 of upload-pack
>
> Nit; s/I/i/, to match others in the series, I think.
>
>> In upload-pack-2 we send each capability in its own packet buffer.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>
>> Notes:
>>     Moved the capabilities into a struct containing all the capabilities,
>>     and then we selectively cancel out unwanted capabilities.
>
>> diff --git a/upload-pack-2.c b/upload-pack-2.c
>> new file mode 120000
>> index 0000000..e30a871
>> --- /dev/null
>> +++ b/upload-pack-2.c
>> @@ -0,0 +1 @@
>> +upload-pack.c
>> \ No newline at end of file
>
> Yuck.
>
> Can't we do an equivalent without this symbolic link, i.e. a new
> Makefile rule to compile upload-pack.c in two different ways to two
> different object files?

Ok I changed that and it works now (only one upload-pack.c file no upload-pack-2.c and no corresponding object either.)

However we don't want to have the version used in upload pack depending on the file name at run time, which is why I am reverting to this state and depending on the file name at compile time. Instead of a symlink we could use an option passed into the compiler as well, but I am not sure if that is as easy to add to the Makefile as this way.

Show 5 quoted lines

>
> The way this patch is organized makes it unclear which part is what
> was added for v2 and which part is shared with v1 (and changes can
> be possible breakage to the existing code), leading to a patch that
> is hard to review.

ok :(

Changed in a reroll.