Thread (15 messages) flat view 15 messages, 5 authors, 2016-06-15

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

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:56:26

Possibly related (same subject, not in this thread)

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.
quoted hunk ↗ jump to hunk
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)?
quoted hunk ↗ jump to hunk
    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?
quoted hunk ↗ jump to hunk
     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?
quoted hunk ↗ jump to hunk
     # 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
+                    sm_eval "$post_order"
                 fi
             ) <&3 3<&- ||
             die "$(eval_gettext "Stopping at '\$sm_path'; script returned non-zero status.")"
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help