Re: [BUG?] gitlink without .gitmodules no longer fails recursive clone

[Jeff King <peff@peff.net>]

On Tue, Jun 06, 2017 at 02:39:14PM -0400, Jeff King wrote:

> > Just for some background on the new behavior and how this functionality
> > changed: My series changed how 'submodule init' behaved if you have
> > 'submodule.active' set.  Once set (like how clone --recurse does now)
> > when not provided any path to a submodule, a list of 'active' submodules
> > matching the 'submodule.active' pathspec will be initialized.  One of
> > the requirements to be 'active' is to have an entry in the .gitmodules
> > file so gitlinks without an entry in the .gitmodules file will simply be
> > ignored now.
> 
> OK, thanks for the explanation. I certainly agree that treating
> .gitmodules as the source of truth is consistent and easy to explain.
> I'll update my test to handle the new behavior (it's a regression test
> for how GitHub Pages handles some broken setups).

I've been chatting with Pages folks about the kinds of errors they see,
and I was surprised how easy it is to get into this situation.

In an ideal world the user do:

  git submodule add git://host/repo.git path

which adds the gitlink and the .gitmodules entry. But it doesn't seem
unreasonable for somebody unfamiliar with submodules to do:

  git clone git://host/repo.git path
  git add path

This does add the entry as a gitlink, but doesn't write any sort of
.gitmodules entry. With the old code, cloning the repository (either by
another user, or in our case during a Pages build), a recursive clone or
submodule init would complain loudly. But now it's just quietly ignored.
Which seems unfortunate.

I actually don't think the old behavior was that great. Somebody _else_
would see the error and was responsible for filtering it back to the
original author. But what we really want is to warn the user when
they're creating the broken situation.

Should "git add" check whether there's a matching .gitmodules entry for
the path and issue a warning otherwise?

Sort of orthogonal, but I kind of wonder if the recursive clone should
also issue a warning. Not for an inactive submodule (which obviously is
normal) but for one that is inactive because it does not even have a
.gitmodules entry. I suppose that _could_ be considered normal in a
post-v2.13 world (e.g., if the owner of the repo "deactivates" a module
by deleting its .gitmodules entry).  But since it's indistinguishable
from the broken case, perhaps that would be better done with a specific
"active = false" flag in .gitmodules.

-Peff