Re: [PATCH v2 2/2] sha1dc: optionally use sha1collisiondetection as a submodule

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, May 22, 2017 at 4:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> But when somebody (like me?) feels strongly enough, such a change
>> can always come on top of this patch, so let's have this
>> manual-configuration-only version as our first step.
>
> Just so that I have something I can come back to, here it is with a
> log message.
>
> -- >8 --
> Subject: [PATCH] sha1collisiondetection: automatically enable when submodule is populated
>
> If a user wants to experiment with the version of collision
> detecting sha1 from the submodule, the user needed to not just
> populate the submodule but also needed to turn the knob.
>
> A Makefile trick is easy enough to do so, so let's do this.  When
> somebody with a copy of the submodule populated wants not to use it,
> that can be done by overriding it in config.mak or from the command
> line, e.g. "make DC_SHA1_SUBMODULE= all".
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 6baad1669e..8d33936a12 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -989,6 +989,10 @@ EXTLIBS =
>
>  GIT_USER_AGENT = git/$(GIT_VERSION)
>
> +ifeq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
> +DC_SHA1_SUBMODULE = auto
> +endif
> +
>  include config.mak.uname
>  -include config.mak.autogen
>  -include config.mak


I thought this should have something like this:

diff --git a/Makefile b/Makefile
index 8d33936a12..bbcac648a4 100644
--- a/Makefile
+++ b/Makefile
@@ -989,9 +989,11 @@ EXTLIBS =

 GIT_USER_AGENT = git/$(GIT_VERSION)

+ifndef DC_SHA1_SUBMODULE
 ifeq ($(wildcard
sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)
 DC_SHA1_SUBMODULE = auto
 endif
+endif

 include config.mak.uname
 -include config.mak.autogen

I.e. so using sha1dc via DC_SHA1_SUBMODULE= even with the submodule
checked-out would work, but experimenting with it now that seems to do
the right thing, even though this is set unconditionally. There must
be some Makefile magic I'm missing here.

But in any case it would make sense to squash something like this into
your patch:

diff --git a/Makefile b/Makefile
index 8d33936a12..4d9c49ae17 100644
--- a/Makefile
+++ b/Makefile
@@ -148,7 +148,8 @@ all::
 # sha1collisiondetection shipped as a submodule instead of the
 # non-submodule copy in sha1dc/. This is an experimental option used
 # by the git project to migrate to using sha1collisiondetection as a
-# submodule.
+# submodule. Supplied implicitly if the sha1collisiondetection/
+# directory is found to be populated.
 #
 # Define OPENSSL_SHA1 environment variable when running make to link
 # with the SHA1 routine from openssl library.