Re: [PATCH 4/5] Makefile: add USE_SHA1DC knob

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 17, 2017, 05:24 UTC
Message-ID: <xmqqh92sa31r.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170317001115.xau6nzp6i7ilkqxv@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 14 quoted lines

> On Thu, Mar 16, 2017 at 03:43:13PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > +ifdef USE_SHA1DC
>> > +	LIB_OBJS += sha1dc/sha1.o
>> > +	LIB_OBJS += sha1dc/ubc_check.o
>> > +	BASIC_CFLAGS += -DSHA1_SHA1DC
>> 
>> The name of this CPP symbol is one difference between this and
>> Linus's version.  Wouldn't "-DSHA1_DC" make more sense?
>
> I'm fine with either. Somehow SHA1_DC felt too short, but it doesn't
> really matter in practice.

I'm fine with either, too. It was just double SHA1 felt a bit strange, when the naming convention was SHA1_ followed by the characteristic attribute of the implementation (e.g. came from Mozilla, etc.) and I thought "Detecting Collision" was the notable characteristic of this one.

Show 7 quoted lines

>> Another difference is that your version adds USE_SHA1DC to
>> GIT-BUILD-OPTIONS in patch 5/5; I thought GIT-CFLAGS forces
>> rebuilding and that was sufficient, but GIT-BUILD-OPTIONS is
>> available to tests for introspection, so adding it is needed
>> for that reason.
>
> Yep, exactly.

Thanks.