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

Junio C Hamano [off-list ref] 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 <redacted>
---
 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

-- 
2.13.0-440-g3ce6d2d5b8

On Mon, May 22, 2017 at 4:48 AM, Junio C Hamano [off-list ref] wrote:

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.