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

2013-04-02 Thread Jens Lehmann 
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 eacousin...@gmail.com
 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 eacousin...@gmail.com
 ---
 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 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=

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.

  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
  }

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-25 Thread Eric Cousineau 
On 03/18/2013 04:10 PM, Jens Lehmann wrote:
 Am 12.03.2013 17:01, schrieb Phil Hord:
 On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 22:17, schrieb Phil Hord:
 On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@gmail.com writes:
 ...
 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}'
 uphomedel{30-ish}endbackspaceenter

 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
 uphomedel{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.
 
 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.

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

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

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

2013-03-25 Thread Eric Cousineau 

From 2c2923ada809d671828aa58dcda05a1b71222b70 Mon Sep 17 00:00:00 2001
From: Eric Cousineau eacousin...@gmail.com
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 eacousin...@gmail.com
---
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


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-25 Thread Eric Cousineau 

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

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

Am 12.03.2013 17:01, schrieb Phil Hord:

On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann jens.lehm...@web.de wrote:

Am 05.03.2013 22:17, schrieb Phil Hord:

...


I agree on the possible problems a configuration option introduces.


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.


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



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

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

2013-03-25 Thread Junio C Hamano 
Jens Lehmann jens.lehm...@web.de writes:

 Am 12.03.2013 17:01, schrieb Phil Hord:
 On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 22:17, schrieb Phil Hord:
 On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@gmail.com writes:
 ...
 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.

FWIW, after thinking about it a bit more and especially after
thinking about the nested submodule layout, I changed my mind.

The reasoning is very simple.  In short, your top-level may be
somebody else's submodule.

If you have a project A, that has a submodule B  C that in turn
have submodules D, E  F, G, like this:

A
   / \
  B   C
 / \ / \
D  E F  G

you may want your submodule foreach [--post-order] that is run at
the top-level to visit B D E C F G (or D E B F G C).  A is not a
submodule, and it may be rational to do without --also-toplevel
option from the point of view of yourself.

But if submodule foreach [--post-order] B run at the top-level
visits B D E (or D E B), wouldn't it be more natural if you had a
way to optionally make this

cd B  submodule foreach [--post-order]

visit the same modules in the same way?  The story is the same if
your top-level project A is bound at a path in somebody else's
project as a submodule.  His submodulle foreach will visit your
top-level A while visiting the hierarchy of your submodules (and
other submodules he has as your siblings).

I do not know if foreach should visit your top-level by default;
changing that may be too late and too disruptive.  But I think an
optional I want this traversal to also visit the top would not be
so _wrong_ even at the conceptual level.

Of course, it may make the implementation simpler, too ;-)  foreach
could just scan the immediate submodules, chdir into each of them
and then run the equivalent foreach with --also-toplevel option,
with the same --post-order (or --pre-order) option.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-25 Thread Junio C Hamano 
Junio C Hamano gits...@pobox.com writes:

 Jens Lehmann jens.lehm...@web.de writes:
 ...
 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).

I forgot to say that I haven't changed my mind about this part.  Do
we visit the top-level? is an orthogonal and different topic and is
better done in a patch separate from the one for --post-order.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-18 Thread Jens Lehmann 
Am 12.03.2013 17:01, schrieb Phil Hord:
 On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 22:17, schrieb Phil Hord:
 On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@gmail.com writes:
 ...
 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}'
uphomedel{30-ish}endbackspaceenter

 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
 uphomedel{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.

 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).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-18 Thread Jens Lehmann 
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 eacousin...@gmail.com
 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 eacousin...@gmail.com
 ---
 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.

 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

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

 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=

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?

  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)

Why do you have to add the local keyword here?

  # 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

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
 +

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

2013-03-14 Thread Eric Cousineau 

From 59fb432e17a1aae9de26bbaaca7f09cc7f03b471 Mon Sep 17 00:00:00 2001
From: Eric Cousineau eacousin...@gmail.com
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 eacousin...@gmail.com
---
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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-12 Thread Phil Hord 
On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 22:17, schrieb Phil Hord:
 On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@gmail.com writes:
 ...
 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}'
uphomedel{30-ish}endbackspaceenter

 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
uphomedel{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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-11 Thread Heiko Voigt 
On Sat, Mar 09, 2013 at 07:18:48PM +0100, Jens Lehmann wrote:
 Am 05.03.2013 22:17, schrieb Phil Hord:
  In a shell, it usually goes like this:
  
 git submodule foreach --recursive '${cmd}'
 uphomedel{30-ish}endbackspaceenter
  
  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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-09 Thread Jens Lehmann 
Am 05.03.2013 22:17, schrieb Phil Hord:
 On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@gmail.com writes:
 ...
 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}'
uphomedel{30-ish}endbackspaceenter
 
 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 $@\
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-05 Thread Junio C Hamano 
Heiko Voigt hvo...@hvoigt.net writes:

 On Mon, Mar 04, 2013 at 03:00:45PM -0800, Junio C Hamano wrote:
 So if you want a single boolean to toggle between the current
 behaviour and the other one, it would be --post-order.  But you may
 at least want to consider pros and cons of allowing users to give
 two separate commands, one for the pre-order visitation (which is
 the current command) and the other for the post-order
 visitation. Being able to run both might turn out to be useful.

 I second that. Having a --post-order=command/script switch will give
 us much more flexibility. For ease of use we could allow --post-order
 without command to switch the meaning of the main command.

 So a final solution would have these switches:

 git submodule foreach ... [--pre-order[=command]] 
 [--post-order[=command]] [command]

 If only --pre-order without argument is given the command will be
 executed pre-order. If only --post-order the command will be executed
 post-order. If both are given its an error and so on...

 There are some combinations we would need to catch as errors but this
 design should allow a step by step implementation:

   1. just the --post-order switch
   2. --post-order with argument switch
   3. --pre-order (including argument) for symmetry of usage

Yeah, I think I can agree with that direction, and Eric's patch
could be that first step of the three-step progression, without
painting us into a corner we cannot get out of when we want to
advance to 2 and 3 later.

I was more interested in the design aspect and I didn't look at the
actual patch text, though.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-05 Thread Eric Cousineau 
On Tue, Mar 5, 2013 at 10:09 AM, Junio C Hamano gits...@pobox.com wrote:
 Heiko Voigt hvo...@hvoigt.net writes:

 On Mon, Mar 04, 2013 at 03:00:45PM -0800, Junio C Hamano wrote:
 So if you want a single boolean to toggle between the current
 behaviour and the other one, it would be --post-order.  But you may
 at least want to consider pros and cons of allowing users to give
 two separate commands, one for the pre-order visitation (which is
 the current command) and the other for the post-order
 visitation. Being able to run both might turn out to be useful.

 I second that. Having a --post-order=command/script switch will give
 us much more flexibility. For ease of use we could allow --post-order
 without command to switch the meaning of the main command.

 So a final solution would have these switches:

 git submodule foreach ... [--pre-order[=command]] 
 [--post-order[=command]] [command]

 If only --pre-order without argument is given the command will be
 executed pre-order. If only --post-order the command will be executed
 post-order. If both are given its an error and so on...

 There are some combinations we would need to catch as errors but this
 design should allow a step by step implementation:

   1. just the --post-order switch
   2. --post-order with argument switch
   3. --pre-order (including argument) for symmetry of usage

 Yeah, I think I can agree with that direction, and Eric's patch
 could be that first step of the three-step progression, without
 painting us into a corner we cannot get out of when we want to
 advance to 2 and 3 later.

 I was more interested in the design aspect and I didn't look at the
 actual patch text, though.

Would these be the correct behaviors of Heiko's implementation?

git submodule foreach # Empty command, pre-order
git submodule foreach --pre-order # Same behavior
git submodule foreach --post-order # Empty command, post-order
git submodule foreach 'frotz' # Do 'frotz' pre-order in each submodule
git submodule foreach --post-order 'frotz' # Do 'frotz' post-order in
each submodule
git submodule foreach --pre-order='frotz' --post-order='shimmy' # Do
'frotz' pre-order and 'shimmy' post-order in each submodule
git submodule foreach --post-order='shimmy' 'frotz' # Invalid usage of
the command
git submodule foreach --post-order --pre-order #

It should not be too hard to have this functionality affect the
--include-super command as well.

And would it be worth it to abstract this traversal to expose it to
other commands, such as 'update', to consolidate the code some?
I think Imram was doing something like that in his post.

- Eric
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-05 Thread Junio C Hamano 
Eric Cousineau eacousin...@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=:

 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-05 Thread Jens Lehmann 
Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@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).

 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-05 Thread Phil Hord 
On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 05.03.2013 19:34, schrieb Junio C Hamano:
 Eric Cousineau eacousin...@gmail.com writes:
 ...
 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}'
   uphomedel{30-ish}endbackspaceenter

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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-04 Thread Eric Cousineau 
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=gitm=121066084508631w=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.

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

I've attached a test script to generate the tree that VonC suggested with
output showing the iteration.


0001-area-submodules.patch
Description: Binary data


test.sh
Description: Bourne shell script

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

2013-03-04 Thread Jens Lehmann 
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:
 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=gitm=121066084508631w=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).

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

 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-04 Thread Eric Cousineau 
git-submodule.sh: In foreach, make '-post-order' yield post-order 
traversal and
'--include-super' execute commands at the top-level supermodule, with 
both of these

options compatible with '--recursive'.

Signed-off-by: Eric Cousineau eacousin...@gmail.com
---
Sorry about missing the part about not included MIME attachments, hope 
this is in a better format now.
Jens, I changed the '--parent' option to '--include-super' which is 
hopefully less vague.
Junio, you made an excellent point about both being useful. In 
particular, I overlooked the case
for doing a submodule pull / update (if, for whatever reason, it is more 
convenient than a submodule
update, maybe for merging). In that case, you might want to initialize 
new submodules and ignore the

old ones, instead of wasting time on them with a post-order traversal pull.
I've implemented your suggestions to have a boolean '--post-order' 
option, and made the '--include-super'
option compatible with it. This way, the original behavior of 'foreach' 
is preserved.


I've updated the test and uploaded it to pastebin: 
http://pastebin.com/BgZNzFpi


 git-submodule.sh | 102 
+--

 1 file changed, 77 insertions(+), 25 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..652bea0 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] [--include-super] 
[--post-order] 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 include_super= recursive= post_order=
 while test $# -ne 0
 do
 case $1 in
@@ -443,6 +445,12 @@ cmd_foreach()
 --recursive)
 recursive=1
 ;;
+--post-order)
+post_order=1
+;;
+--include-super)
+include_super=1
+;;
 -*)
 usage
 ;;
@@ -453,35 +461,79 @@ cmd_foreach()
 shift
 done

-toplevel=$(pwd)
+if test -n $recursive
+then
+local recursive_flags=--recursive
+if test -n $post_order
+then
+recursive_flags=$recursive_flags --post-order
+fi
+fi
+
+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)
 exec 30
+
+# Use nested functions
+super_eval() {
+name=$(basename $toplevel)
+clear_local_git_env
+path=.
+say $(eval_gettext Entering '\$name') # Not sure of proper 
thing here
+eval $@ || die $(eval_gettext Stopping at supermodule; 
script returned non-zero status.)

+}

-module_list |
-while read mode sha1 stage sm_path
-do
-die_if_unmatched $mode
-if test -e $sm_path/.git
-then
-say $(eval_gettext Entering '\$prefix\$sm_path')
-name=$(module_name $sm_path)
-(
-prefix=$prefix$sm_path/
-clear_local_git_env
-# we make $path available to scripts ...
-path=$sm_path
-cd $sm_path 
-eval $@ 
-if test -n $recursive
-then
-cmd_foreach --recursive $@
-fi
-) 3 3- ||
-die $(eval_gettext Stopping at '\$sm_path'; script 
returned non-zero status.)

-fi
-done
+if test -n $include_super -a -z $post_order
+then
+super_eval $@
+fi 
+(
+module_list |
+while read mode sha1 stage sm_path
+do
+die_if_unmatched $mode
+if test -e $sm_path/.git
+then
+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 $@ ||

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

2013-03-04 Thread Heiko Voigt 
On Mon, Mar 04, 2013 at 03:00:45PM -0800, Junio C Hamano wrote:
 So if you want a single boolean to toggle between the current
 behaviour and the other one, it would be --post-order.  But you may
 at least want to consider pros and cons of allowing users to give
 two separate commands, one for the pre-order visitation (which is
 the current command) and the other for the post-order
 visitation. Being able to run both might turn out to be useful.

I second that. Having a --post-order=command/script switch will give
us much more flexibility. For ease of use we could allow --post-order
without command to switch the meaning of the main command.

So a final solution would have these switches:

git submodule foreach ... [--pre-order[=command]] [--post-order[=command]] 
[command]

If only --pre-order without argument is given the command will be
executed pre-order. If only --post-order the command will be executed
post-order. If both are given its an error and so on...

There are some combinations we would need to catch as errors but this
design should allow a step by step implementation:

1. just the --post-order switch
2. --post-order with argument switch
3. --pre-order (including argument) for symmetry of usage

Cheers Heiko
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html