Re: [PATCH 5/5] Teach checkout to recursively checkout submodules

From: Jens Lehmann <jens.lehmann@web.de>
Date: Dec 27, 2013, 14:14 UTC
Message-ID: <52BD8B42.2060507@web.de>
In-Reply-To: <20131226162238.GQ20443@google.com>

Am 26.12.2013 17:22, schrieb Jonathan Nieder:

Show 27 quoted lines

> From: Jens Lehmann <Jens.Lehmann@web.de>
> Date: Wed, 13 Jun 2012 18:50:10 +0200
> 
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> This is the patch that actually introduces the --recurse-submodules
> option, which makes the rest work.
> 
> The tests indicate some future directions for improving this, but
> it works reasonably well already.  I think I'd be most comfortable
> applying these if they were rearranged a little to the following
> order:
> 
>  1. First, introducing the --recurse-submodules option, perhaps
>     with no actual effect, with tests that it is parsed correctly,
>     the default works, etc.
> 
>  2. Then, adding the desired behaviors one by one with corresponding
>     tests (handling submodule modifications, removals, additions).
> 
>  3. Finally, any needed tweaks on top.
> 
> That way, it is easy to play around with early patches without
> worrying about the later ones at first, and the patches are easy
> to review to confirm that they at least don't break anything in
> the --no-recurse-submodules case.

Makes tons of sense.

The only feature I'm aware of that is currently missing is to read the path <-> name mappings from the correct .gitmodules blob, which is needed to populate submodules that appear.

> That said, Debian experimental has these applied as is already,
> and there haven't been any complaints yet.

Great!

I'm also using this code at $dayjob successfully for quite some time now. Additionally I also enable unconditional recursive checkout by putting a "return 1" in the first line of the submodule_needs_update() function (Which is a hack to emulate "autoupdate=true" while at the same time pretending to already having added "--recurse-submodules=config" to all call sites in git porcelain scripts that call plumbing themselves). Only in a few corner cases submodules aren't properly updated, but we'll weed those out while implementing the tests. And we need lots of those to cover all important combinations ...