Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well

From: Jens Lehmann <jens.lehmann@web.de>
Date: Mar 5, 2013, 20:51 UTC
Message-ID: <51365AB6.2010602@web.de>
In-Reply-To: <7vboaxu23y.fsf@alter.siamese.dyndns.org>

Am 05.03.2013 19:34, schrieb Junio C Hamano:

Show 45 quoted lines

> Eric Cousineau <eacousineau@gmail.com> writes:
> 
>> Would these be the correct behaviors of Heiko's implementation?
> 
> I do not think Heiko already has an implementation, but let's try to
> see how each example makes sense.
> 
>> git submodule foreach # Empty command, pre-order
>> git submodule foreach --pre-order # Same behavior
>> git submodule foreach --post-order # Empty command, post-order
> 
> OK.  The last one shows "I am here" output differently from the
> other two, but otherwise they are all no-op.
> 
>> git submodule foreach 'frotz' # Do 'frotz' pre-order in each submodule
> 
> OK.  And it would be the same if you said either one of:
> 
> 	git submodule foreach --pre-order 'frotz'
> 	git submodule foreach --pre-order='frotz'
> 
>> git submodule foreach --post-order 'frotz' # Do 'frotz' post-order in
>> each submodule
> 
> OK.
> 
>> git submodule foreach --pre-order='frotz' --post-order='shimmy' # Do
>> 'frotz' pre-order and 'shimmy' post-order in each submodule
> 
> OK.
> 
>> git submodule foreach --post-order='shimmy' 'frotz' # Invalid usage of
>> the command
> 
> I would expect this to behave exactly the same as:
> 
> 	git submodule foreach \
>         	--post-order=shimmy \
>                 --pre-order=frotz
> 
>> git submodule foreach --post-order --pre-order #
> 
> I expect it to behave exactly the same as:
> 
> 	git submodule foreach --post-order=: --pre-order=:

I'd favor to just drop the --pre-order option and do this:

  foreach [--recursive] [--post-order <command>] [<command>]

Me thinks pre-order is a sane default and we shouldn't add an explicit option for that. And even with current Git you can simply give no command at all and it'll show you all the submodules it enters without doing anything in them, so we'd only need to add the --post-order handling anyway (and fix the synopsis by adding square brackets around the command while at it, as that is optional).

Show 16 quoted lines

>> It should not be too hard to have this functionality affect the
>> --include-super command as well.
> 
> I would assume that
> 
> 	git submodule foreach --pre-order=A --post-order=B --include-super
> 
> would be identical to running
> 
> 	A &&
>         git submodule foreach --pre-order=A --post-order=B &&
>         B
>
> I am not entirely convinced we would want --include-super in the
> first place, though.  It does not belong to "submodule foreach";
> it is doing something _outside_ the submoudules.

I totally agree with that. First, adding --include-super does not belong into the --post-order patch at all, as that is a different topic (even though it belongs to the same use case Eric has). Also the reason why we are thinking about adding the --post-order option IMO cuts the other way for --include-super: It is so easy to do that yourself I'm not convinced we should add an extra option to foreach for that, especially as it has nothing to do with submodules. So I think we should just drop --include-super.