Re: [PATCH 3/4] Makefile: use the sha1collisiondetection submodule by default

[Jeff King <peff@peff.net>]

On Tue, Nov 28, 2017 at 09:32:13PM +0000, Ævar Arnfjörð Bjarmason wrote:

> Change the build process so that instead of needing to supply
> DC_SHA1_SUBMODULE=YesPlease to use the sha1collisiondetection
> submodule instead of the copy of the same code shipped in the sha1dc
> directory, it uses the submodule by default unless
> NO_DC_SHA1_SUBMODULE=UnfortunatelyYes is supplied.
> 
> This reverses the logic added by me in 86cfd61e6b ("sha1dc: optionally
> use sha1collisiondetection as a submodule", 2017-07-01). Git has now
> shipped with the submodule in git.git for two major releases, if we're
> ever going to migrate to fully using it instead of perpetually
> maintaining both sha1collisiondetection and the sha1dc directory this
> is a logical first step.
> 
> This change removes the "auto" logic Junio added in
> cac87dc01d ("sha1collisiondetection: automatically enable when
> submodule is populated", 2017-07-01), I feel that automatically
> falling back to using sha1dc would defeat the point, which is to smoke
> out any remaining users of git.git who have issues cloning the
> submodule for whatever reason.

I'm not sure how I feel about this. I see your point that there's no
real value in maintaining two systems indefinitely.  At the same time, I
wonder how much value the submodule strategy is actually bringing us.

IOW, are we agreed that the path forward is to get everybody using the
submodule?

It seems like it's going to cause some minor pain for CI and packaging
systems that now need to care about submodules (so at least flipping the
switch, but maybe also dealing with having a network dependency for the
build that was not already there).

I'll admit I'm more sensitive to this than most people, since I happen
to maintain a fork of Git that I run through an internal CI system. And
that CI otherwise depends only on the locally-held fork, not any
external resources. But I'm probably in a fairly unique situation there.

-Peff