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 4, 2013, 22:15 UTC
Message-ID: <51351CF5.7010308@web.de>
In-Reply-To: <CA+aSAWuoxZkSnRybhefnFr9ngs3tHmt6hAH4o0ebjYKvjJ-vpw@mail.gmail.com>

Please don't attach your patches, see Documentation/SubmittingPatches on how to post patches to this list.

Am 04.03.2013 09:41, schrieb Eric Cousineau:

Show 7 quoted lines

> In this patch, foreach --recursive acts depth-first, much like the default
> behavior described in the patch by Imram Yousuf in this
> post <http://marc.info/?l=git&m=121066084508631&w=2>.
> Changes were made so that the submodule "Entering ..." message was right
> next to the output generated by the command too.
> It also adds the --parent option for executing the command in the
> supermodule as well.

>From reading the linked pages I assume a valid use case you have is:

   git submodule foreach --recursive 'git add -A && git commit ...'

This will currently not work because the depth first algorithm of foreach will execute the command /before/ recursing deeper. You'd need it to execute the command /after/ returning from the deeper level (which is what your patch seems to be about).

Show 7 quoted lines

> I began by adding a --depth option, to preserve the original --recursive
> behavior, and the --parent option, and trying to get that to work. However,
> I pretty much confused myself for a while trying to straighten that out, so
> I just ended up modifying the --recursive behavior.
> If the --recursive behavior should be preserved, I could add the --depth
> option back and only have --parent affect non-recursive and --depth
> recursive behavior.

I would rather not change default behavior without having a *very* good reason to do so (and I'm not sure what you need the --depth option for).

What we currently get from your example is:
  Entering 'a'
  Entering 'a/b'
  Entering 'a/b/d'
  Entering 'a/c'
  Entering 'b'
  Entering 'b/d'
  Entering 'c'
  Entering 'd'
Me thinks this is what most users would expect of a recursion, enter each
level before descending into the next.

For your use case you'd need to have:
  Entering 'a/b/d'
  Entering 'a/b'
  Entering 'a/c'
  Entering 'a'
  Entering 'b/d'
  Entering 'b'
  Entering 'c'
  Entering 'd'
(Please note that this is still depth-first)

I won't object to adding an option to foreach that will execute the command after recursing (but I'm not convinced --parent is a very good name for that).

Show 9 quoted lines

> I had kind-of implemented this behavior with aliases / bash functions
> (posted to pastebin <http://pastebin.com/yLHe9XWy>
> , spurned by a
> question I asked in StackOverflow <http://stackoverflow.com/q/14846967/170413>),
> however I would always run into issues with escaping characters when
> passing from the bash functions to git aliases (i.e., putting "'ello" as an
> test commit message). I also tried out mb14's method from the StackOverflow
> post, but I ran into the same issues.
> Figured the best way to avoid that was to cut out the extra layers.

It seems to be really hard to do what you have in mind with shell commands or aliases, which is a good point for adding such an option to foreach. But I don't see a reason why we would want to change the current default, which is what your RFC proposes.