Jens Lehmann [off-list ref] writes:
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:
quoted
In this patch, foreach --recursive acts depth-first, much like the default
behavior described in the patch by Imram Yousuf in this
post <http://marc.info/?l=git&m=121066084508631&w=2>.
Changes were made so that the submodule "Entering ..." message was right
next to the output generated by the command too.
It also adds the --parent option for executing the command in the
supermodule as well.
From reading the linked pages I assume a valid use case you have is:
git submodule foreach --recursive 'git add -A && git commit ...'
This will currently not work because the depth first algorithm of foreach
will execute the command /before/ recursing deeper. You'd need it to
execute the command /after/ returning from the deeper level (which is what
your patch seems to be about).
...
What we currently get from your example is:
Entering 'a'
Entering 'a/b'
Entering 'a/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 '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).
Are you comparing pre-order vs post-order traversal?
Both can be useful depending on what you are trying to achieve. You
need a pre-order traversal (i.e. you "visit" and perform some action
on the node and then descend into its children) if you need to do
some preparation before you visit deeper levels; you need a
post-order traversal (i.e. you "visit" and perform some action on
the node after you have done all its children) if you know you will
be readly only after you are done with all your children.
You can throw in in-order traversal to the mix (i.e. you "visit" and
perform some action on the node after visiting some but not all of
your children and then continue visiting the remainder of your
children), but I do not know what practical value you would get out
of it.
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.
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 <redacted>
---
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 3<&0
+
+ # 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 "$@" || die "$epitaph"
+ }
+
+ cd "$sm_path" &&
+ if test -z "$post_order"
+ then
+ sm_eval "$@"
+ fi &&
+ if test -n "$recursive"
+ then
+ cmd_foreach $recursive_flags "$@"
+ fi &&
+ if test -n "$post_order"
+ then
+ sm_eval "$@"
+ fi
+ # Since the (...) seems to limit exit's scope, make
sure to kill things here if something goes awry
+ # (the `|| exit 1` at the end)
+ ) <&3 3<&- || exit 1
+ fi
+ done
+ ) &&
+ if test -n "$include_super" -a -n "$post_order"
+ then
+ super_eval "$@"
+ fi
}
#
--
1.8.2.rc1.24.g06d67b8.dirty
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