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

Eric Cousineau [off-list ref] 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=:

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.

Am 05.03.2013 19:34, schrieb Junio C Hamano:

Eric Cousineau [off-list ref] writes:

quoted

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.

quoted

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.

quoted

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'

quoted

git submodule foreach --post-order 'frotz' # Do 'frotz' post-order in
each submodule

OK.

quoted

git submodule foreach --pre-order='frotz' --post-order='shimmy' # Do
'frotz' pre-order and 'shimmy' post-order in each submodule

OK.

quoted

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

quoted

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).

quoted

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.

On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann [off-list ref] wrote:

Am 05.03.2013 19:34, schrieb Junio C Hamano:

quoted

Eric Cousineau [off-list ref] writes:

quoted

...

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.

I agree it should not be part of this commit, but I've often found
myself in need of an --include-super switch.   To me,
git-submodule-foreach means "visit all my .git repos in this project
and execute $cmd".  It's a pity that the super-project is considered a
second-class citizen in this regard.

I have to do this sometimes:

   ${cmd} && git submodule foreach --recursive '${cmd}'

I often forget the first part in scripts, though, and I've seen others
do it too.  I usually create a function for it in git-heavy scripts.

In a shell, it usually goes like this:

   git submodule foreach --recursive '${cmd}'
   <up><home><del>{30-ish}<end><backspace><enter>

It'd be easier if I could just include a switch for this, and maybe
even create an alias for it.  But maybe this is different command
altogether.

Am 05.03.2013 22:17, schrieb Phil Hord:

On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 19:34, schrieb Junio C Hamano:

quoted

Eric Cousineau [off-list ref] writes:

quoted

...

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.

I agree it should not be part of this commit, but I've often found
myself in need of an --include-super switch.   To me,
git-submodule-foreach means "visit all my .git repos in this project
and execute $cmd".  It's a pity that the super-project is considered a
second-class citizen in this regard.

Hmm, for me the super-project is a very natural second-class citizen
to "git *submodule* foreach". But also I understand that sometimes the
user wants to apply a command to superproject and submodules alike (I
just recently did exactly that with "git gc" on our build server).

I have to do this sometimes:

   ${cmd} && git submodule foreach --recursive '${cmd}'

I often forget the first part in scripts, though, and I've seen others
do it too.  I usually create a function for it in git-heavy scripts.

In a shell, it usually goes like this:

   git submodule foreach --recursive '${cmd}'
   <up><home><del>{30-ish}<end><backspace><enter>

It'd be easier if I could just include a switch for this, and maybe
even create an alias for it.  But maybe this is different command
altogether.

Are you sure you wouldn't forget to provide such a switch too? ;-)

I'm still not convinced we should add a new switch, as it can easily
be achieved by adding "${cmd} &&" to your scripts. And on the command
line you could use an alias like this one to achieve that:

[alias]
	recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"

On Sat, Mar 09, 2013 at 07:18:48PM +0100, Jens Lehmann wrote:

Am 05.03.2013 22:17, schrieb Phil Hord:

quoted

In a shell, it usually goes like this:

   git submodule foreach --recursive '${cmd}'
   <up><home><del>{30-ish}<end><backspace><enter>

It'd be easier if I could just include a switch for this, and maybe
even create an alias for it.  But maybe this is different command
altogether.

Are you sure you wouldn't forget to provide such a switch too? ;-)

I'm still not convinced we should add a new switch, as it can easily
be achieved by adding "${cmd} &&" to your scripts. And on the command
line you could use an alias like this one to achieve that:

[alias]
	recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"

I also think it would be useful to have a switch (or even configuration)
to include the superproject.

The following (quite typical) use cases come to my mind:

# Assuming some not yet existing configuration values
git config submodule.recursive true
git config submodule.includeSuper true

# commit your work over the whole tree into one branch
git submodule foreach git checkout -b hv/my-super-cool-feature
git submodule foreach --post-order git commit -a -m "DRAFT: finished work for today"
git submodule foreach git push hvoigt hv/my-super-cool-feature

# cleanup
git submodule foreach git clean -xfd

# reset
git submodule foreach git reset --hard

...

Assuming you have a submodule heavy project and you work on multiple
submodules including the superproject. These are quite typical commands
you would use during development of your feature I imagine. Once you are
finished you need to get your feature upstream by the individual
submodule rules.

On a feature branch during development there is nothing wrong in simply
doing full cross-submodule project commits.

At some point we will probably extend the above commands with a
--recurse-submodules switch but until then this is a good substitute so
why not have a --include-super maybe even as a configuration option ?

Cheers Heiko

On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann [off-list ref] wrote:

Am 05.03.2013 22:17, schrieb Phil Hord:

quoted

On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 19:34, schrieb Junio C Hamano:

quoted

Eric Cousineau [off-list ref] writes:

quoted

...

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.

I agree it should not be part of this commit, but I've often found
myself in need of an --include-super switch.   To me,
git-submodule-foreach means "visit all my .git repos in this project
and execute $cmd".  It's a pity that the super-project is considered a
second-class citizen in this regard.

Hmm, for me the super-project is a very natural second-class citizen
to "git *submodule* foreach". But also I understand that sometimes the
user wants to apply a command to superproject and submodules alike (I
just recently did exactly that with "git gc" on our build server).

quoted

I have to do this sometimes:

   ${cmd} && git submodule foreach --recursive '${cmd}'

I often forget the first part in scripts, though, and I've seen others
do it too.  I usually create a function for it in git-heavy scripts.

In a shell, it usually goes like this:

   git submodule foreach --recursive '${cmd}'
   <up><home><del>{30-ish}<end><backspace><enter>

It'd be easier if I could just include a switch for this, and maybe
even create an alias for it.  But maybe this is different command
altogether.

Are you sure you wouldn't forget to provide such a switch too? ;-)

No.  However, when I remember to add the switch, my shell history will
remember it for me.  This does not happen naturally for me in the
"<up><home><del>{30-ish}..." workflow.

I also hope this switch grows up into a configuration option someday.
Or maybe a completely different command, like I said before; because I
actually think it could be dangerous as a configuration option since
it would have drastic consequences for users executing scripts or
commands in other users' environments.

I'm still not convinced we should add a new switch, as it can easily
be achieved by adding "${cmd} &&" to your scripts. And on the command
line you could use an alias like this one to achieve that:

[alias]
        recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"

Yes, making the feature itself a 2nd-class citizen.  :-)

But this alias also denies me the benefit of the --post-order option.
For 'git recurse git push', for example, I wouldn't want the
superproject push to occur first; I would want it to occur last after
the submodules have been successfully pushed.

I agree this should go in some other commit, but I do not think it is
so trivial it should never be considered as a feature for git.  That's
all I'm trying to say.

Phil

 From 59fb432e17a1aae9de26bbaaca7f09cc7f03b471 Mon Sep 17 00:00:00 2001
From: Eric Cousineau <redacted>
Date: Thu, 14 Mar 2013 01:19:53 -0500
Subject: [PATCH] submodule-foreach: Added in --post-order=<command> per Jens
  Lehmann's suggestion

Signed-off-by: Eric Cousineau <redacted>
---
Made the scope of the patch only relate to --post-order.
Would we want to rename this to just --post=<command> ?

Anywho, here it is running in a test setup, where the structure is:
a
- b
- - d
- c

$ git submodule foreach --recursive --post-order 'echo Post $name' 'echo 
Pre $path'
Entering 'b'
Pre b
Entering 'b/d'
Pre d
Entering 'b/d'
Post d
Entering 'b'
Post b
Entering 'c'
Pre c
Entering 'c'
Post c

An interesting note is that it fails with 'git submodule foreach 
--post-order', but not 'git submodule foreach --post-order=', since it 
simply interprets that as an empty command.
If that is important, I could add in a check for $# when parsing the 
argument for --post-order=*.

  git-submodule.sh | 39 ++++++++++++++++++++++++++++++++++-----
  1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..9b70bc2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -10,7 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] 

[--name <name>] [--reference <re
     or: $dashless [--quiet] init [--] [<path>...]
     or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
[-f|--force] [--rebase] [--reference <repository>] [--merge] 
[--recursive] [--] [<path>...]
     or: $dashless [--quiet] summary [--cached|--files] [--summary-limit 
<n>] [commit] [--] [<path>...]
-   or: $dashless [--quiet] foreach [--recursive] <command>
+   or: $dashless [--quiet] foreach [--recursive] 
[--post-order=<command>] <command>
     or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
  OPTIONS_SPEC=
  . git-sh-setup

@@ -434,6 +434,8 @@ Use -f if you really want to add it." >&2
  cmd_foreach()
  {
      # parse $args after "submodule ... foreach".
+    # Gratuitous (empty) local's to prevent recursive bleeding
+    local recursive= post_order=
      while test $# -ne 0
      do
          case "$1" in

@@ -443,6 +445,15 @@ cmd_foreach()
          --recursive)
              recursive=1
              ;;
+        --post-order)
+            test "$#" = "1" && usage
+            post_order="$2"
+            shift
+            ;;
+        --post-order=*)
+            # Will skip empty commands
+            post_order=${1#*=}
+            ;;
          -*)
              usage
              ;;

@@ -453,7 +464,7 @@ cmd_foreach()
          shift
      done

-    toplevel=$(pwd)
+    local toplevel=$(pwd)

      # dup stdin so that it can be restored when running the external
      # command in the subshell (and a recursive call to this function)

@@ -465,18 +476,36 @@ cmd_foreach()
          die_if_unmatched "$mode"
          if test -e "$sm_path"/.git
          then
-            say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
+            local name prefix path message epitaph
+            message="$(eval_gettext "Entering '\$prefix\$sm_path'")"
+            epitaph="$(eval_gettext "Stopping at '\$sm_path'; script 

returned non-zero status.")"
              name=$(module_name "$sm_path")
              (
                  prefix="$prefix$sm_path/"
                  clear_local_git_env
                  # we make $path available to scripts ...
                  path=$sm_path
+
+                sm_eval() {
+                    say "$message"
+                    eval "$@" || die "$epitaph"
+                }
+
                  cd "$sm_path" &&
-                eval "$@" &&
+                sm_eval "$@" &&
                  if test -n "$recursive"
                  then
-                    cmd_foreach "--recursive" "$@"
+                    if test -n "$post_order"
+                    then
+                        # Tried keeping flags as a variable, but was 
having difficulty
+                        cmd_foreach --recursive --post-order 
"$post_order" "$@"
+                    else
+                        cmd_foreach --recursive "$@"
+                    fi
+                fi &&
+                if test -n "$post_order"
+                then
+                    sm_eval "$post_order"
                  fi
              ) <&3 3<&- ||
              die "$(eval_gettext "Stopping at '\$sm_path'; script 
returned non-zero status.")"
-- 
1.8.2.rc1.24.g06d67b8.dirty

Am 12.03.2013 17:01, schrieb Phil Hord:

On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 22:17, schrieb Phil Hord:

quoted

On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 19:34, schrieb Junio C Hamano:

quoted

Eric Cousineau [off-list ref] writes:

quoted

...

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.

I agree it should not be part of this commit, but I've often found
myself in need of an --include-super switch.   To me,
git-submodule-foreach means "visit all my .git repos in this project
and execute $cmd".  It's a pity that the super-project is considered a
second-class citizen in this regard.

Hmm, for me the super-project is a very natural second-class citizen
to "git *submodule* foreach". But also I understand that sometimes the
user wants to apply a command to superproject and submodules alike (I
just recently did exactly that with "git gc" on our build server).

quoted

I have to do this sometimes:

   ${cmd} && git submodule foreach --recursive '${cmd}'

I often forget the first part in scripts, though, and I've seen others
do it too.  I usually create a function for it in git-heavy scripts.

In a shell, it usually goes like this:

   git submodule foreach --recursive '${cmd}'
   <up><home><del>{30-ish}<end><backspace><enter>

It'd be easier if I could just include a switch for this, and maybe
even create an alias for it.  But maybe this is different command
altogether.

Are you sure you wouldn't forget to provide such a switch too? ;-)

No.  However, when I remember to add the switch, my shell history will
remember it for me.  This does not happen naturally for me in the
"<up><home><del>{30-ish}..." workflow.

I started to use '&&' in my daily shell work for exactly that reason:
that the bash history remembers groups of two or more commands for me.

I also hope this switch grows up into a configuration option someday.
Or maybe a completely different command, like I said before; because I
actually think it could be dangerous as a configuration option since
it would have drastic consequences for users executing scripts or
commands in other users' environments.

I agree on the possible problems a configuration option introduces.

quoted

I'm still not convinced we should add a new switch, as it can easily
be achieved by adding "${cmd} &&" to your scripts. And on the command
line you could use an alias like this one to achieve that:

[alias]
        recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"

Yes, making the feature itself a 2nd-class citizen.  :-)

But this alias also denies me the benefit of the --post-order option.
For 'git recurse git push', for example, I wouldn't want the
superproject push to occur first; I would want it to occur last after
the submodules have been successfully pushed.

[alias]
         recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
;-)

I agree this should go in some other commit, but I do not think it is
so trivial it should never be considered as a feature for git.  That's
all I'm trying to say.

I am not against adding such a functionality to Git, I'm just not
convinced "git submodule foreach" is the right command for that. I
suspect the "git for-each-repo" Lars proposed earlier this year might
be a better choice, as that could also recurse into other repos which
aren't registered as submodules. And a "for-each-repo" to me looks
like a command which could include the superproject too (at least when
told to do so with an option).

Thanks, just a quick review before I find some time do take a
deeper look.

Am 14.03.2013 07:30, schrieb Eric Cousineau:

From 59fb432e17a1aae9de26bbaaca7f09cc7f03b471 Mon Sep 17 00:00:00 2001
From: Eric Cousineau <redacted>
Date: Thu, 14 Mar 2013 01:19:53 -0500
Subject: [PATCH] submodule-foreach: Added in --post-order=<command> per Jens
 Lehmann's suggestion

Signed-off-by: Eric Cousineau <redacted>
---
Made the scope of the patch only relate to --post-order.
Would we want to rename this to just --post=<command> ?

Hmm, while having no strong preference on that, "post order"
looks more like the correct term describing what we do here.

Anywho, here it is running in a test setup, where the structure is:
a
- b
- - d
- c

$ git submodule foreach --recursive --post-order 'echo Post $name' 'echo Pre $path'
Entering 'b'
Pre b
Entering 'b/d'
Pre d
Entering 'b/d'
Post d
Entering 'b'
Post b
Entering 'c'
Pre c
Entering 'c'
Post c

Looking good.

Maybe drop the '=' from the description (see --reference for an example)?

Wouldn't it be sufficient to add "post_order=" to the top of the
file where "recursive" is already initialized? Or am I missing
something here?

Why do you have to add the "local" keyword here?

Same here?

+            message="$(eval_gettext "Entering '\$prefix\$sm_path'")"
+            epitaph="$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
             name=$(module_name "$sm_path")
             (
                 prefix="$prefix$sm_path/"
                 clear_local_git_env
                 # we make $path available to scripts ...
                 path=$sm_path
+
+                sm_eval() {
+                    say "$message"
+                    eval "$@" || die "$epitaph"
+                }
+
                 cd "$sm_path" &&
-                eval "$@" &&
+                sm_eval "$@" &&
                 if test -n "$recursive"
                 then
-                    cmd_foreach "--recursive" "$@"
+                    if test -n "$post_order"
+                    then
+                        # Tried keeping flags as a variable, but was having difficulty

Maybe because you set the "post_order" variable to empty at the
beginning of this function? If I read that right moving that
initialization to the top of the file could get rid of the if
here?

+                        cmd_foreach --recursive --post-order "$post_order" "$@"
+                    else
+                        cmd_foreach --recursive "$@"
+                    fi
+                fi &&
+                if test -n "$post_order"
+                then
+                    sm_eval "$post_order"
                 fi
             ) <&3 3<&- ||
             die "$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"

On 03/18/2013 04:10 PM, Jens Lehmann wrote:

Am 12.03.2013 17:01, schrieb Phil Hord:

quoted

On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 22:17, schrieb Phil Hord:

quoted

On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 19:34, schrieb Junio C Hamano:

quoted

Eric Cousineau [off-list ref] writes:

quoted

...

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.

I agree it should not be part of this commit, but I've often found
myself in need of an --include-super switch.   To me,
git-submodule-foreach means "visit all my .git repos in this project
and execute $cmd".  It's a pity that the super-project is considered a
second-class citizen in this regard.

Hmm, for me the super-project is a very natural second-class citizen
to "git *submodule* foreach". But also I understand that sometimes the
user wants to apply a command to superproject and submodules alike (I
just recently did exactly that with "git gc" on our build server).

quoted

I have to do this sometimes:

    ${cmd} && git submodule foreach --recursive '${cmd}'

I often forget the first part in scripts, though, and I've seen others
do it too.  I usually create a function for it in git-heavy scripts.

In a shell, it usually goes like this:

    git submodule foreach --recursive '${cmd}'
    <up><home><del>{30-ish}<end><backspace><enter>

It'd be easier if I could just include a switch for this, and maybe
even create an alias for it.  But maybe this is different command
altogether.

Are you sure you wouldn't forget to provide such a switch too? ;-)

No.  However, when I remember to add the switch, my shell history will
remember it for me.  This does not happen naturally for me in the
"<up><home><del>{30-ish}..." workflow.

I started to use '&&' in my daily shell work for exactly that reason:
that the bash history remembers groups of two or more commands for me.

quoted

I also hope this switch grows up into a configuration option someday.
Or maybe a completely different command, like I said before; because I
actually think it could be dangerous as a configuration option since
it would have drastic consequences for users executing scripts or
commands in other users' environments.

I agree on the possible problems a configuration option introduces.

quoted

quoted

I'm still not convinced we should add a new switch, as it can easily
be achieved by adding "${cmd} &&" to your scripts. And on the command
line you could use an alias like this one to achieve that:

[alias]
         recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"

I tried this and the 'recurse-post' alias, but could not get it to function as
it does inside of 'git submodule foreach'. I also tried out some different escaping
methods, but nothing seemed to work. I've added the examples below.

quoted

Yes, making the feature itself a 2nd-class citizen.  :-)

But this alias also denies me the benefit of the --post-order option.
For 'git recurse git push', for example, I wouldn't want the
superproject push to occur first; I would want it to occur last after
the submodules have been successfully pushed.

[alias]
          recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
;-)

quoted

I agree this should go in some other commit, but I do not think it is
so trivial it should never be considered as a feature for git.  That's
all I'm trying to say.

I am not against adding such a functionality to Git, I'm just not
convinced "git submodule foreach" is the right command for that. I
suspect the "git for-each-repo" Lars proposed earlier this year might
be a better choice, as that could also recurse into other repos which
aren't registered as submodules. And a "for-each-repo" to me looks
like a command which could include the superproject too (at least when
told to do so with an option).

Here are the aliases I am using:

[alias]
	recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"
	recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
	fer = !sh -c \"eval \\\"$@\\\" && git submodule foreach --recursive \\\"$@\\\"\"
	ferpo = !sh -c \"git submodule foreach --recursive --post-order \\\"$@\\\" && eval \\\"$@\\\"\"
	fers = !sh -c \"eval '$@' && git submodule foreach --recursive '$@'\"
	ferpos = !sh -c \"git submodule foreach --recursive --post-order '$@' && eval '$@'\"

And these are the results I get with the following example:

$ cmd="echo \"'ello world: \$PWD\""
$ eval "$cmd"
'ello world: /tmp/a
$ git submodule foreach --recursive "$cmd"
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Entering 'c'
'ello world: /tmp/a/c
$ git submodule foreach --recursive --post-order "$cmd" "$cmd"
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b'
'ello world: /tmp/a/b
Entering 'c'
'ello world: /tmp/a/c
Exiting 'c'
'ello world: /tmp/a/c
$ git recurse "$cmd"
'ello world: /tmp/a
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.
$ git recurse-post "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.
$ git fer "$cmd"
ello world: /tmp/a
Entering 'b'
ello world: /tmp/a
Entering 'b/d'
ello world: /tmp/a
Entering 'c'
ello world: /tmp/a
$ git ferpo "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: world:: not found
Stopping at 'b'; script returned non-zero status.
Stopping at 'b'; script returned non-zero status.
$ git fers "$cmd"
ello world: /tmp/a' && git submodule foreach --recursive 'echo ello world: /tmp/a
$ git ferpos "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.

The problem is trying to escape with double-quotes, where the single-quotes are evaluated
as a shell token thing and not as a string argument, versus single-quotes, where you cannot (easily) escape single
quotes inside of it (though please correct me if I'm wrong!).
It seems the best solution would be to have it as a script to allow recursion to occur in the scope of one script,
like submodule foreach.

I understand now why it does not fit in the scope of 'git submodule', though, so I could implement it as a *very*
lightweight stand-in for Lars's "git for-each-repo" via some copy-and-paste :P

- Eric

 From 2c2923ada809d671828aa58dcda05a1b71222b70 Mon Sep 17 00:00:00 2001
From: Eric Cousineau <redacted>
Date: Mon, 25 Mar 2013 22:27:06 -0500
Subject: [PATCH] submodule-foreach: Added in --post-order=<command> and
  adjusted code per Jens Lehmann's suggestion

Signed-off-by: Eric Cousineau <redacted>
---
Updated the usage line.
I had put the locals in there before because I think I was having 
trouble with resolving some
of the variables in nested submodules, but now that I've taken them out 
they seem to work fine.
I also changed the message for the post-order to say "Exiting".

I did not have a chance to look into why I couldn't group the 
--post-order stuff into a string
when passing it on to submodule. I can look at it later on though.

Now the output is as follows:

$ git submodule foreach --recursive --post-order 'echo Post $name' 'echo 
Pre $path'
Entering 'b'
Pre b
Entering 'b/d'
Pre d
Exiting 'b/d'
Post d
Exiting 'b'
Post b
Entering 'c'
Pre c
Exiting 'c'
Post c

  git-submodule.sh |   35 ++++++++++++++++++++++++++++++-----
  1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..4c9923a 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -10,7 +10,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] 

[--name <name>] [--reference <re
     or: $dashless [--quiet] init [--] [<path>...]
     or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
[-f|--force] [--rebase] [--reference <repository>] [--merge] 
[--recursive] [--] [<path>...]
     or: $dashless [--quiet] summary [--cached|--files] [--summary-limit 
<n>] [commit] [--] [<path>...]
-   or: $dashless [--quiet] foreach [--recursive] <command>
+   or: $dashless [--quiet] foreach [--recursive] [--post-order 
<command>] <command>
     or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
  OPTIONS_SPEC=
  . git-sh-setup

@@ -434,6 +434,8 @@ Use -f if you really want to add it." >&2
  cmd_foreach()
  {
  	# parse $args after "submodule ... foreach".
+	recursive=
+	post_order=
  	while test $# -ne 0
  	do
  		case "$1" in

@@ -443,6 +445,15 @@ cmd_foreach()
  		--recursive)
  			recursive=1
  			;;
+		--post-order)
+			test "$#" = "1" && usage
+			post_order="$2"
+			shift
+			;;
+		--post-order=*)
+			# Will skip empty commands
+			post_order=${1#*=}
+			;;
  		-*)
  			usage
  			;;

@@ -465,7 +476,9 @@ cmd_foreach()
  		die_if_unmatched "$mode"
  		if test -e "$sm_path"/.git
  		then
-			say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
+			enter_msg="$(eval_gettext "Entering '\$prefix\$sm_path'")"
+			exit_msg="$(eval_gettext "Exiting '\$prefix\$sm_path'")"
+			die_msg="$(eval_gettext "Stopping at '\$sm_path'; script returned 

non-zero status.")"
  			name=$(module_name "$sm_path")
  			(
  				prefix="$prefix$sm_path/"

@@ -473,13 +486,25 @@ cmd_foreach()
  				# we make $path available to scripts ...
  				path=$sm_path
  				cd "$sm_path" &&
-				eval "$@" &&
+				say "$enter_msg" &&
+				eval "$@" || die "$die_msg" &&
  				if test -n "$recursive"
  				then
-					cmd_foreach "--recursive" "$@"
+					if test -n "$post_order"
+					then
+						# tried keeping flags as a variable, but was having difficulty
+						cmd_foreach --recursive --post-order "$post_order" "$@"
+					else
+						cmd_foreach --recursive "$@"
+					fi
+				fi &&
+				if test -n "$post_order"
+				then
+					say "$exit_msg" &&
+					eval "$post_order" || die "$die_msg"
  				fi
  			) <&3 3<&- ||
-			die "$(eval_gettext "Stopping at '\$sm_path'; script returned 

non-zero status.")"
+			die "$die_msg"
  		fi
  	done
  }
-- 
1.7.9.5

On 03/25/2013 10:56 PM, Eric Cousineau wrote:

On 03/18/2013 04:10 PM, Jens Lehmann wrote:

quoted

Am 12.03.2013 17:01, schrieb Phil Hord:

quoted

On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann [off-list ref] wrote:

quoted

Am 05.03.2013 22:17, schrieb Phil Hord:

...

quoted

I agree on the possible problems a configuration option introduces.

quoted

quoted

I'm still not convinced we should add a new switch, as it can easily
be achieved by adding "${cmd} &&" to your scripts. And on the command
line you could use an alias like this one to achieve that:

[alias]
          recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"

I tried this and the 'recurse-post' alias, but could not get it to function as
it does inside of 'git submodule foreach'. I also tried out some different escaping
methods, but nothing seemed to work. I've added the examples below.

quoted

quoted

Yes, making the feature itself a 2nd-class citizen.  :-)

But this alias also denies me the benefit of the --post-order option.
For 'git recurse git push', for example, I wouldn't want the
superproject push to occur first; I would want it to occur last after
the submodules have been successfully pushed.

[alias]
           recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
;-)

quoted

I agree this should go in some other commit, but I do not think it is
so trivial it should never be considered as a feature for git.  That's
all I'm trying to say.

I am not against adding such a functionality to Git, I'm just not
convinced "git submodule foreach" is the right command for that. I
suspect the "git for-each-repo" Lars proposed earlier this year might
be a better choice, as that could also recurse into other repos which
aren't registered as submodules. And a "for-each-repo" to me looks
like a command which could include the superproject too (at least when
told to do so with an option).

Here are the aliases I am using:

[alias]
	recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"
	recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
	fer = !sh -c \"eval \\\"$@\\\" && git submodule foreach --recursive \\\"$@\\\"\"
	ferpo = !sh -c \"git submodule foreach --recursive --post-order \\\"$@\\\" && eval \\\"$@\\\"\"
	fers = !sh -c \"eval '$@' && git submodule foreach --recursive '$@'\"
	ferpos = !sh -c \"git submodule foreach --recursive --post-order '$@' && eval '$@'\"

And these are the results I get with the following example:

$ cmd="echo \"'ello world: \$PWD\""
$ eval "$cmd"
'ello world: /tmp/a
$ git submodule foreach --recursive "$cmd"
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Entering 'c'
'ello world: /tmp/a/c
$ git submodule foreach --recursive --post-order "$cmd" "$cmd"
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b'
'ello world: /tmp/a/b
Entering 'c'
'ello world: /tmp/a/c
Exiting 'c'
'ello world: /tmp/a/c
$ git recurse "$cmd"
'ello world: /tmp/a
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.
$ git recurse-post "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.
$ git fer "$cmd"
ello world: /tmp/a
Entering 'b'
ello world: /tmp/a
Entering 'b/d'
ello world: /tmp/a
Entering 'c'
ello world: /tmp/a
$ git ferpo "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: world:: not found
Stopping at 'b'; script returned non-zero status.
Stopping at 'b'; script returned non-zero status.
$ git fers "$cmd"
ello world: /tmp/a' && git submodule foreach --recursive 'echo ello world: /tmp/a
$ git ferpos "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.

The problem is trying to escape with double-quotes, where the single-quotes are evaluated
as a shell token thing and not as a string argument, versus single-quotes, where you cannot (easily) escape single
quotes inside of it (though please correct me if I'm wrong!).
It seems the best solution would be to have it as a script to allow recursion to occur in the scope of one script,
like submodule foreach.

I understand now why it does not fit in the scope of 'git submodule', though, so I could implement it as a *very*
lightweight stand-in for Lars's "git for-each-repo" via some copy-and-paste :P

- Eric

Put together a script with the --include-super functionality, named it 
'git-fer.sh' to start.
Posted as a Gist: https://gist.github.com/eacousineau/5243161

That test case:

$ git-fer --include-super --recursive --post-order "$cmd" "$cmd"
Entering supermodule 'a'
'ello world: /tmp/a
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b'
'ello world: /tmp/a/b
Entering 'c'
'ello world: /tmp/a/c
Exiting 'c'
'ello world: /tmp/a/c
Exiting supermodule 'a'
'ello world: /tmp/a

Seems were getting closer, some comments from a quick read of your
patch below.

Am 26.03.2013 05:03, schrieb Eric Cousineau:

From 2c2923ada809d671828aa58dcda05a1b71222b70 Mon Sep 17 00:00:00 2001
From: Eric Cousineau <redacted>
Date: Mon, 25 Mar 2013 22:27:06 -0500
Subject: [PATCH] submodule-foreach: Added in --post-order=<command> and
 adjusted code per Jens Lehmann's suggestion

Signed-off-by: Eric Cousineau <redacted>
---
Updated the usage line.
I had put the locals in there before because I think I was having trouble with resolving some
of the variables in nested submodules, but now that I've taken them out they seem to work fine.
I also changed the message for the post-order to say "Exiting".

That's better than "Stopping", but while I'm not a native speaker
I'd propose to use "Leaving" as the opposite of "Entering".

I'm still not sure we need that here, in fact the problem you have
with the cmd_foreach invocation below might just be because you
reset these variables here instead of once at the top of this file.

Signed-off-by: eacousineau <redacted>
---
I see what you meant by the extra variables, so I've fixed that so the
original flags aren't needed with recursion. Also updated it to not
print the entering command if there is only a post-order command.

Examples:

$ git submodule foreach --recursive --post-order 'echo Goodbye' "echo \"'ello\""
Entering 'b'
'ello
Entering 'b/d'
'ello
Leaving 'b/d'
Goodbye
Leaving 'b'
Goodbye
Entering 'c'
'ello
Leaving 'c'
Goodbye

$ git submodule foreach --recursive --post-order :
Leaving 'b/d'
Leaving 'b'
Leaving 'c'

 git-submodule.sh | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..e08a724 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -11,7 +11,7 @@ USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <re
    or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
-   or: $dashless [--quiet] foreach [--recursive] <command>
+   or: $dashless [--quiet] foreach [--recursive] [--post-order <command>] <command>
    or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
 OPTIONS_SPEC=
 . git-sh-setup

@@ -449,6 +449,15 @@ cmd_foreach()
 		--recursive)
 			recursive=1
 			;;
+		--post-order)
+			test "$#" = "1" && usage
+			post_order="$2"
+			shift
+			;;
+		--post-order=*)
+			# Will skip empty commands
+			post_order=${1#*=}
+			;;
 		-*)
 			usage
 			;;

@@ -471,7 +480,9 @@ cmd_foreach()
 		die_if_unmatched "$mode"
 		if test -e "$sm_path"/.git
 		then
-			say "$(eval_gettext "Entering '\$prefix\$sm_path'")"
+			enter_msg="$(eval_gettext "Entering '\$prefix\$sm_path'")"
+			leave_msg="$(eval_gettext "Leaving '\$prefix\$sm_path'")"
+			die_msg="$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
 			name=$(module_name "$sm_path")
 			(
 				prefix="$prefix$sm_path/"

@@ -479,13 +490,23 @@ cmd_foreach()
 				# we make $path available to scripts ...
 				path=$sm_path
 				cd "$sm_path" &&
-				eval "$@" &&
+				if test $# -gt 0 -o -z "$post_order"
+				then
+					say "$enter_msg" &&
+					eval "$@" || die "$die_msg"
+				fi &&
 				if test -n "$recursive"
 				then
-					cmd_foreach "--recursive" "$@"
+					# subshell will use parent-scoped values
+					cmd_foreach "$@"
+				fi &&
+				if test -n "$post_order"
+				then
+					say "$leave_msg" &&
+					eval "$post_order" || die "$die_msg"
 				fi
 			) <&3 3<&- ||
-			die "$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
+			die "$die_msg"
 		fi
 	done
 }

-- 
1.8.2.1.390.gd4ee029

Am 13.04.2013 06:04, schrieb eacousineau:

Signed-off-by: eacousineau <redacted>
---
I see what you meant by the extra variables, so I've fixed that so the
original flags aren't needed with recursion.

Thanks, the code is looking much better now and you nicely
described the changes you made since the last version. A few
comments though:

I think the subject line should read:

   [PATCH v2] submodule foreach: Add --post-order option

We use the imperative form, also the adjustments are a normal part
of the review process and don't need to be mentioned explicitly in
the title, just show the version of your iteration by adding "v2"
after the word "PATCH" (and of course the next iteration will be
"v3" ;-).

The commit message is not explaining what you did and why you did
it, please see the "Describe your changes well." section in
Documentation/SubmittingPatches on how to do that.

And you'll also want to add the new option to the man page in
Documentation/git-submodule.txt.

Also updated it to not
print the entering command if there is only a post-order command.

Good idea.

Examples:

$ git submodule foreach --recursive --post-order 'echo Goodbye' "echo \"'ello\""
Entering 'b'
'ello
Entering 'b/d'
'ello
Leaving 'b/d'
Goodbye
Leaving 'b'
Goodbye
Entering 'c'
'ello
Leaving 'c'
Goodbye

$ git submodule foreach --recursive --post-order :
Leaving 'b/d'
Leaving 'b'
Leaving 'c'

Makes sense to me. These two examples should be getting a test
case each in t/t7407-submodule-foreach.sh.

I don't think we gain much by putting enter_msg and leave_msg into
their own variables as they are only used once, I'd prefer to see
these messages inlined.

+			die_msg="$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"

I think there is a \$prefix missing in front of the \$sm_path here
(see enter_msg and leave_msg). As you only copied that message you
can simply say in the commit message "While at it also fix a missing
prefix in the die message" at the end of the last paragraph.

You should at least state that you dropped the --recursive here
on purpose, just add that to the "While at it ..." sentence. And I
suspect the comment above is more a reminder for yourself, we
could drop that too.

+				fi &&
+				if test -n "$post_order"
+				then
+					say "$leave_msg" &&
+					eval "$post_order" || die "$die_msg"
 				fi
 			) <&3 3<&- ||
-			die "$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
+			die "$die_msg"
 		fi
 	done
 }