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

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

On Fri, Dec 08 2017, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Instead the Makefile will emit an error if the contents of the
>> submodule aren't checked out (line-wrapped. GNU make emits this all on
>> one line):
>>
>>     Makefile:1031: *** The sha1collisiondetection submodule is not
>>     checked out. Please make it available, either by cloning with
>>     --recurse-submodules, or by running "git submodule update
>>     --init". If you can't use it for whatever reason you can define
>>     NO_DC_SHA1_SUBMODULE=NoThanks.  Stop.
>
> Sounds OK.
>
> But I actually do not mind to (and may even prefer to) have an
> endgame opposite of what this series tries to lead us to.  We've
> tried to have this as submodule, we've seen that the arrangement
> works, and now we declare victory and get rid of the submodule.

I don't think we can say we tried without having this 4/5 (5/5 not
needed) in a couple of releases. Without this series we always smoothly
fall back to the non-submodule, and only use it if you opt in.

So all we've really tested so far is:

 * CI systems that consume git.git and provide --recurse-submodules to
   git-clone by default.

 * People on this list that have gone out of their way to test by
   manually toggling the the flag.

> That endgame allows us not force people to grab an essential part of
> the codebase as an external dependency from another place, which
> feels quite bad, especially when their primary interest is not in
> dogfooding submodule but in building a working version of Git.

As noted previously my two motivations are:

 1) That we decide what we want to do with this, ultimately I don't
    really mind which way we go.

 2) That if we go with the submodule by default, it should be understood
    that one of the main benefits is us *actually* dogfooding it and
    subsequently improving it for all git users.

> Removing one and keeping the other between the two will make the
> distribution more streamlined by removing the build-time knob we
> need to tweak, but the one that gets removed does not have to be the
> in-tree one that allows people to "git clone" from just one place.

What you're describing here is a great example of #2, and also a way of
making my point above that we really haven't tried submodules in git.git
yet, since you're just worrying about this issue now that using it would
the default.

This is a UX issue with submodules that I agree sucks and there's no
reason for why it couldn't be solved. E.g. one solution is that
submodules could have something like:

    [submodule "sha1collisiondetection"]
            path = sha1collisiondetection
            url = https://github.com/cr-marcstevens/sha1collisiondetection.git
            branch = master
            localbranch = sha1collisiondetection/master

Where the localbranch would be git.git's own copy in a branch of the the
sha1collisiondetection/ commit. Then when you update the ref
sha1collisiondetection/ points to it would also update the
sha1collisiondetection/master branch (and warn/die when you push git.git
master but not that branch).

This would solve offer a solution to this submodule UX issue, but more
importantly I think the likelyhood of such a patch (and others) actually
being written is going to go up *significantly* if the git project
itself is dogfooding the feature, with exhibit A being that you're
already annoyed by it :)