[PATCH] - Added recurse command to git submodule

[PATCH] - Added recurse command to git submodule

[imyousuf]

From: Imran M Yousuf <imyousuf@smartitengineering.com>


 - The purpose of the recurse command in the git submodule is to recurse
 a command in its submodule at depths specified. For example if one wants
 to do a diff on its project with submodules at once, one can simply do
 git-submodule recurse diff HEAD and would see the diff for all the modules
 it contains. If during recursion it is found that a module has not been
 initialized or updated than the command also initializes and updates them.
 It is to be noted that argument specification to the command intended (in
 above example diff) to recurse will be same as its original command (i.e.
 git diff). If the original command spec changes it will have no effect on
 the recurse command. Depth of recursion can be specified simply by
 mentioning the -d <recursion depth> argument to recurse command. If not
 specified or specified to 0, it will be comprehended to recurse at all
 depth; if a positive number is specified than maximum recursion depth will
 never cross that depth. In order to see some information -v option may be
 used

Signed-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>
---
 git-submodule.sh |  112 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 111 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 8a29382..5cb931e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,7 +4,7 @@
 #
 # Copyright (c) 2007 Lars Hjemli
 
-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'
+USAGE='[[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]|[recurse [-v] [-d <recursion depth>] <command> <arguments> ...]]'
 OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
@@ -17,6 +17,9 @@ status=
 quiet=
 cached=
 command=
+recurse_verbose=0
+recursion_depth=0
+current_recursion_depth=0
 
 #
 # print stuff on stdout unless -q was specified
@@ -294,6 +297,82 @@ modules_list()
 	done
 }
 
+# Initializes the submodule if already not initialized
+initialize_sub_module() {
+	if [ ! -d "$1"/.git ]; then
+		if [ $recurse_verbose -eq 1 ]; then
+			echo Initializing and updating "$1"
+		fi
+		git-submodule init "$1"; git-submodule update "$1"
+	fi
+}
+
+# This actually traverses the module; checks
+# whether the module is initialized or not.
+# if not initialized, then done so and then the
+# intended command is evaluated. Then it
+# recursively goes into it modules.
+traverse_module() {
+	if [ $recurse_verbose -eq 1 ]; then
+		echo Current recursion depth: "$current_recursion_depth"
+	fi
+	initialize_sub_module "$1"
+	(
+		submod_path="$1"
+		shift
+		cd "$submod_path"
+		if [ $recurse_verbose -eq 1 ]; then
+			echo Working in mod "$submod_path" @ `pwd` with "$@" \("$#"\)
+		fi
+	        git "$@"
+		# Check whether submodules exists only if recursion to that depth
+		# was requested by user
+		if test "$recursion_depth" -eq 0 || test "$current_recursion_depth" -lt "$recursion_depth"
+		then
+			if [ -f .gitmodules ]; then
+		                for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
+					export current_recursion_depth=$(echo "$current_recursion_depth+1" | bc)
+		                        traverse_module "$mod_path" "$@"
+					export current_recursion_depth=$(echo "$current_recursion_depth-1" | bc)
+		                done
+			fi
+	        fi
+	)
+}
+
+# Propagates or recurses over all the submodules at any
+# depth with any git command, e.g. git-clone, git-status,
+# git-commit etc., with the arguments supplied exactly as
+# it would have been supplied to the command otherwise.
+# This actually starts the recursive propagation
+modules_recurse() {
+	project_home=`pwd`
+	echo Project Home: "$project_home"
+	if [ $recurse_verbose -eq 1 ]; then
+		if [ $recursion_depth = 0 ]; then
+			echo Recursion will go to all depths
+		else
+			echo Recursion depth specified to "$recursion_depth"
+		fi
+	fi
+	if [ -d "$project_home"/.git/ ]; then
+		if [ $recurse_verbose -eq 1 ]; then
+			echo Command to recurse: git "$@"
+		fi
+		git "$@"
+		if [ -f .gitmodules ]; then
+			for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
+				export current_recursion_depth=1
+				traverse_module $mod_path "$@"
+			done
+		fi
+	else
+		echo "$project_home" not a git repo thus exiting
+		exit
+	fi
+	unset current_recursion_depth
+}
+
 # command specifies the whole function name since 
 # one of theirs prefix is module not modules
 while test $# != 0
@@ -326,6 +405,37 @@ do
 	--cached)
 		command="modules_list"
 		;;
+	recurse)
+		command="modules_$1"
+		repeat=1
+		while test $repeat = 1
+		do
+			case "$2" in
+				-v)
+					recurse_verbose=1
+					shift
+					;;
+				-d)
+					if [ -z $3 ]; then
+						echo No \<recursion depth\> specified
+						usage
+					elif [ `expr "$3" : '[1-9][0-9]*.*'` -eq `expr "$3" : '.*'` ]; then
+						recursion_depth="$3"
+						shift
+						shift
+					else
+						echo \<recursion depth\> not an integer
+						usage
+					fi
+					;;
+				*)
+					repeat=0
+					;;
+			esac
+		done
+		shift
+		break
+		;;
 	--)
 		break
 		;;
-- 
1.5.3.7

Re: [PATCH] - Added recurse command to git submodule

[Junio C Hamano]

imyousuf@gmail.com writes:

> From: Imran M Yousuf <imyousuf@smartitengineering.com>
>
>
>  - The purpose of the recurse command in the git submodule is to recurse
>  a command in its submodule at depths specified. For example if one wants
>  to do a diff on its project with submodules at once, one can simply do
>  git-submodule recurse diff HEAD and would see the diff for all the modules
>  it contains. If during recursion it is found that a module has not been
>  initialized or updated than the command also initializes and updates them.
>  It is to be noted that argument specification to the command intended (in
>  above example diff) to recurse will be same as its original command (i.e.
>  git diff). If the original command spec changes it will have no effect on
>  the recurse command. Depth of recursion can be specified simply by
>  mentioning the -d <recursion depth> argument to recurse command. If not
>  specified or specified to 0, it will be comprehended to recurse at all
>  depth; if a positive number is specified than maximum recursion depth will
>  never cross that depth. In order to see some information -v option may be
>  used

The indentation style seems, eh, unique.  An overlong single
paragraph with solid run of sentences is somewhat hard to read.

I am not still convinced that a subcommand other than init,
which is started recursively, should initialize and update
submodules that are uninitialized.  Currently there is no way,
other than not having an initialized submodule repository, to
mark that the user is _not_ interested in a particular
submodule, and you will lose that if you make recursing into
uninitialized ones too easy and aggressive.

I suspect that it might be a saner approach to:

 - allow "git submodule recurse init [-d depth]" (although I am
   not sure if limit by depth is so useful in practice -- only
   experience will tell us) to auto-initialize the uninitialized
   submodules;

 - allow any other "git submodule recurse $cmd" to run
   recursively to _any_ depth, but never auto-initialize the
   uninitialized submodules.

In other words, I think it is a very bad idea to rob the
existing mechanism from the user to mark uninteresting
submodules by not initializing them.  An alternative would be to
give them other means to mark "I am not interested in these
submodules", if you insist on this auto-initialization, but I do
not offhand think of a mechanism that is easier to use than what
we already have (i.e. not checking them out).

> @@ -17,6 +17,9 @@ status=
>  quiet=
>  cached=
>  command=
> +recurse_verbose=0
> +recursion_depth=0
> +current_recursion_depth=0

I wonder if we want this "verbose" option to be specific to the
recurse subcommand, or perhaps other subcommands may want to
support more verbose output mode, even if they currently don't.
Perhaps the variable should be just called $verbose?

> +# Initializes the submodule if already not initialized
> +initialize_sub_module() {
> +	if [ ! -d "$1"/.git ]; then
> +		if [ $recurse_verbose -eq 1 ]; then
> +			echo Initializing and updating "$1"

That's a sensible message for the $verbose mode.

> +		fi
> +		git-submodule init "$1"; git-submodule update "$1"

Can init fail for repository "$1"?  Do you want to proceed
updating it if it does?

> +# This actually traverses the module; checks
> +# whether the module is initialized or not.
> +# if not initialized, then done so and then the
> +# intended command is evaluated. Then it
> +# recursively goes into it modules.
> +traverse_module() {
> +	if [ $recurse_verbose -eq 1 ]; then
> +		echo Current recursion depth: "$current_recursion_depth"
> +	fi
> +	initialize_sub_module "$1"
> +	(
> +		submod_path="$1"
> +		shift
> +		cd "$submod_path"
> +		if [ $recurse_verbose -eq 1 ]; then
> +			echo Working in mod "$submod_path" @ `pwd` with "$@" \("$#"\)

Is this a sensible message, I have to wonder...  Saying `pwd`
after already saying "$submod_path" feels more like a debugging
message than just being $verbose.

> +		fi
> +	        git "$@"

Is the user interested in exit status from this command?  Does
the user want to continue on to other submodules if this one
fails?

> +		# Check whether submodules exists only if recursion to that depth
> +		# was requested by user
> +		if test "$recursion_depth" -eq 0 || test "$current_recursion_depth" -lt "$recursion_depth"

Overly long line.  Perhaps...

	if test "$depth" -eq 0 ||
           test "$current_depth" -lt "$depth"
	then
		...

> +		then
> +			if [ -f .gitmodules ]; then
> +		                for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> +					export current_recursion_depth=$(echo "$current_recursion_depth+1" | bc)

 (1) I do not think you need to export this variable;

 (2) It was reported some shells that we intend to support
     mishandle "export VAR=VAL" construct and we tend to write
     this "VAR=VAL" followed by "export VAR" as two separate
     commands on two separate lines;

 (3) We do not use "bc" (and traditionally, shell scripts
     outside git don't, either) in scripts.

So, I think:

    current_recursion_depth=$(( $current_recursion_depth + 1 ))

is enough, although the variable name feels overly long.

Probably I would even do:

	if test "$depth -ne 0 && test "$current_depth" -ge "$depth"
	then
        	exit 0
	fi
	if test -f .gitmodules
        then
        	current_recursion_depth=$(( $current_recursion_depth + 1 ))
		for p in $(sed -n -e 's/path = ....)
                do
                	traverse_module "$p" "$@"
		done
	fi

which would avoid one level of nesting (and indentation), and
removes unnecessary increment and decrement inside the loop.
You are in your own subprocess, so your increment will not
affect the process that called you, and after leaving the loop
the only thing you do is just to exit the subprocess.

> +# Propagates or recurses over all the submodules at any
> +# depth with any git command, e.g. git-clone, git-status,
> +# git-commit etc., with the arguments supplied exactly as
> +# it would have been supplied to the command otherwise.
> +# This actually starts the recursive propagation
> +modules_recurse() {
> +	project_home=`pwd`
> +	echo Project Home: "$project_home"

Doesn't this message belong to $verbose mode?

> +	if [ $recurse_verbose -eq 1 ]; then
> +		if [ $recursion_depth = 0 ]; then
> +			echo Recursion will go to all depths
> +		else
> +			echo Recursion depth specified to "$recursion_depth"
> +		fi

These sounds more like debugging message than $verbose.

> +	fi
> +	if [ -d "$project_home"/.git/ ]; then
> +		if [ $recurse_verbose -eq 1 ]; then
> +			echo Command to recurse: git "$@"

Likewise -- you will repeat that inside traverse_module anyway.

> +		fi
> +		git "$@"
> +		if [ -f .gitmodules ]; then
> +			for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> +				export current_recursion_depth=1
> +				traverse_module $mod_path "$@"
> +			done
> +		fi

Do I see a code duplication here?  Why isn't this done as the
first level recursion inside traverse_module?  _Even_ _if_ you
insist auto-initializing submodules, by moving the
initialize_sub_module call in traverse_module a bit down
(namely, before it recursively invoke traverse_module itself and
make it auto initialize $submod_path, not "$1"), I think you can
remove this duplication.

> +	else
> +		echo "$project_home" not a git repo thus exiting
> +		exit

And its exit code is 0 which tells the caller that there is no
error?

> +	fi
> +	unset current_recursion_depth

The reason for this unset is...?

> @@ -326,6 +405,37 @@ do
>  	--cached)
>  		command="modules_list"
>  		;;
> +	recurse)
> +		command="modules_$1"
> +		repeat=1
> +		while test $repeat = 1
> +		do

You do not need that $repeat thing.  Just use "break", like this.

	while :
        do
        	case ... in
                ...
                *)
                	break ;;
		esac
	done

> +				-d)
> +					if [ -z $3 ]; then

(minor style)

	if test -z "$3"
        then
        	...

> +						echo No \<recursion depth\> specified
> +						usage
> +					elif [ `expr "$3" : '[1-9][0-9]*.*'` -eq `expr "$3" : '.*'` ]; then
> +						recursion_depth="$3"
> +						shift
> +						shift
> +					else
> +						echo \<recursion depth\> not an integer
> +						usage
> +					fi
> +					;;

Instead of checking integerness by hand, it would probably be
much simpler if you did something like this:

	depth="$3"
        depth=$(( $depth + 0 ))
        if test "$depth" != "$3"
        then
        	die "what's that '$3' for?"
	else
        	: happy
	fi

For a rough guideline of shell constructs we use (and do not
use), please see Documentation/CodingGuidelines.

Re: [PATCH] - Added recurse command to git submodule

[Imran M Yousuf]

Hi Junio,

Thanks for the feedback and specially thank you for the coding
standard documentation, I was looking for it. I will make fixes to
both the patches and email them again. Will send this patch again,
probably tomorrow.

Thank you,

Imran

On Jan 9, 2008 2:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> imyousuf@gmail.com writes:
>
> > From: Imran M Yousuf <imyousuf@smartitengineering.com>
> >
> >
> >  - The purpose of the recurse command in the git submodule is to recurse
> >  a command in its submodule at depths specified. For example if one wants
> >  to do a diff on its project with submodules at once, one can simply do
> >  git-submodule recurse diff HEAD and would see the diff for all the modules
> >  it contains. If during recursion it is found that a module has not been
> >  initialized or updated than the command also initializes and updates them.
> >  It is to be noted that argument specification to the command intended (in
> >  above example diff) to recurse will be same as its original command (i.e.
> >  git diff). If the original command spec changes it will have no effect on
> >  the recurse command. Depth of recursion can be specified simply by
> >  mentioning the -d <recursion depth> argument to recurse command. If not
> >  specified or specified to 0, it will be comprehended to recurse at all
> >  depth; if a positive number is specified than maximum recursion depth will
> >  never cross that depth. In order to see some information -v option may be
> >  used
>
> The indentation style seems, eh, unique.  An overlong single
> paragraph with solid run of sentences is somewhat hard to read.
>
> I am not still convinced that a subcommand other than init,
> which is started recursively, should initialize and update
> submodules that are uninitialized.  Currently there is no way,
> other than not having an initialized submodule repository, to
> mark that the user is _not_ interested in a particular
> submodule, and you will lose that if you make recursing into
> uninitialized ones too easy and aggressive.
>
> I suspect that it might be a saner approach to:
>
>  - allow "git submodule recurse init [-d depth]" (although I am
>    not sure if limit by depth is so useful in practice -- only
>    experience will tell us) to auto-initialize the uninitialized
>    submodules;
>
>  - allow any other "git submodule recurse $cmd" to run
>    recursively to _any_ depth, but never auto-initialize the
>    uninitialized submodules.
>
> In other words, I think it is a very bad idea to rob the
> existing mechanism from the user to mark uninteresting
> submodules by not initializing them.  An alternative would be to
> give them other means to mark "I am not interested in these
> submodules", if you insist on this auto-initialization, but I do
> not offhand think of a mechanism that is easier to use than what
> we already have (i.e. not checking them out).
>
> > @@ -17,6 +17,9 @@ status=
> >  quiet=
> >  cached=
> >  command=
> > +recurse_verbose=0
> > +recursion_depth=0
> > +current_recursion_depth=0
>
> I wonder if we want this "verbose" option to be specific to the
> recurse subcommand, or perhaps other subcommands may want to
> support more verbose output mode, even if they currently don't.
> Perhaps the variable should be just called $verbose?
>
> > +# Initializes the submodule if already not initialized
> > +initialize_sub_module() {
> > +     if [ ! -d "$1"/.git ]; then
> > +             if [ $recurse_verbose -eq 1 ]; then
> > +                     echo Initializing and updating "$1"
>
> That's a sensible message for the $verbose mode.
>
> > +             fi
> > +             git-submodule init "$1"; git-submodule update "$1"
>
> Can init fail for repository "$1"?  Do you want to proceed
> updating it if it does?
>
> > +# This actually traverses the module; checks
> > +# whether the module is initialized or not.
> > +# if not initialized, then done so and then the
> > +# intended command is evaluated. Then it
> > +# recursively goes into it modules.
> > +traverse_module() {
> > +     if [ $recurse_verbose -eq 1 ]; then
> > +             echo Current recursion depth: "$current_recursion_depth"
> > +     fi
> > +     initialize_sub_module "$1"
> > +     (
> > +             submod_path="$1"
> > +             shift
> > +             cd "$submod_path"
> > +             if [ $recurse_verbose -eq 1 ]; then
> > +                     echo Working in mod "$submod_path" @ `pwd` with "$@" \("$#"\)
>
> Is this a sensible message, I have to wonder...  Saying `pwd`
> after already saying "$submod_path" feels more like a debugging
> message than just being $verbose.
>
> > +             fi
> > +             git "$@"
>
> Is the user interested in exit status from this command?  Does
> the user want to continue on to other submodules if this one
> fails?
>
> > +             # Check whether submodules exists only if recursion to that depth
> > +             # was requested by user
> > +             if test "$recursion_depth" -eq 0 || test "$current_recursion_depth" -lt "$recursion_depth"
>
> Overly long line.  Perhaps...
>
>         if test "$depth" -eq 0 ||
>            test "$current_depth" -lt "$depth"
>         then
>                 ...
>
> > +             then
> > +                     if [ -f .gitmodules ]; then
> > +                             for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> > +                                     export current_recursion_depth=$(echo "$current_recursion_depth+1" | bc)
>
>  (1) I do not think you need to export this variable;
>
>  (2) It was reported some shells that we intend to support
>      mishandle "export VAR=VAL" construct and we tend to write
>      this "VAR=VAL" followed by "export VAR" as two separate
>      commands on two separate lines;
>
>  (3) We do not use "bc" (and traditionally, shell scripts
>      outside git don't, either) in scripts.
>
> So, I think:
>
>     current_recursion_depth=$(( $current_recursion_depth + 1 ))
>
> is enough, although the variable name feels overly long.
>
> Probably I would even do:
>
>         if test "$depth -ne 0 && test "$current_depth" -ge "$depth"
>         then
>                 exit 0
>         fi
>         if test -f .gitmodules
>         then
>                 current_recursion_depth=$(( $current_recursion_depth + 1 ))
>                 for p in $(sed -n -e 's/path = ....)
>                 do
>                         traverse_module "$p" "$@"
>                 done
>         fi
>
> which would avoid one level of nesting (and indentation), and
> removes unnecessary increment and decrement inside the loop.
> You are in your own subprocess, so your increment will not
> affect the process that called you, and after leaving the loop
> the only thing you do is just to exit the subprocess.
>
> > +# Propagates or recurses over all the submodules at any
> > +# depth with any git command, e.g. git-clone, git-status,
> > +# git-commit etc., with the arguments supplied exactly as
> > +# it would have been supplied to the command otherwise.
> > +# This actually starts the recursive propagation
> > +modules_recurse() {
> > +     project_home=`pwd`
> > +     echo Project Home: "$project_home"
>
> Doesn't this message belong to $verbose mode?
>
> > +     if [ $recurse_verbose -eq 1 ]; then
> > +             if [ $recursion_depth = 0 ]; then
> > +                     echo Recursion will go to all depths
> > +             else
> > +                     echo Recursion depth specified to "$recursion_depth"
> > +             fi
>
> These sounds more like debugging message than $verbose.
>
> > +     fi
> > +     if [ -d "$project_home"/.git/ ]; then
> > +             if [ $recurse_verbose -eq 1 ]; then
> > +                     echo Command to recurse: git "$@"
>
> Likewise -- you will repeat that inside traverse_module anyway.
>
> > +             fi
> > +             git "$@"
> > +             if [ -f .gitmodules ]; then
> > +                     for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> > +                             export current_recursion_depth=1
> > +                             traverse_module $mod_path "$@"
> > +                     done
> > +             fi
>
> Do I see a code duplication here?  Why isn't this done as the
> first level recursion inside traverse_module?  _Even_ _if_ you
> insist auto-initializing submodules, by moving the
> initialize_sub_module call in traverse_module a bit down
> (namely, before it recursively invoke traverse_module itself and
> make it auto initialize $submod_path, not "$1"), I think you can
> remove this duplication.
>
> > +     else
> > +             echo "$project_home" not a git repo thus exiting
> > +             exit
>
> And its exit code is 0 which tells the caller that there is no
> error?
>
> > +     fi
> > +     unset current_recursion_depth
>
> The reason for this unset is...?
>
> > @@ -326,6 +405,37 @@ do
> >       --cached)
> >               command="modules_list"
> >               ;;
> > +     recurse)
> > +             command="modules_$1"
> > +             repeat=1
> > +             while test $repeat = 1
> > +             do
>
> You do not need that $repeat thing.  Just use "break", like this.
>
>         while :
>         do
>                 case ... in
>                 ...
>                 *)
>                         break ;;
>                 esac
>         done
>
> > +                             -d)
> > +                                     if [ -z $3 ]; then
>
> (minor style)
>
>         if test -z "$3"
>         then
>                 ...
>
> > +                                             echo No \<recursion depth\> specified
> > +                                             usage
> > +                                     elif [ `expr "$3" : '[1-9][0-9]*.*'` -eq `expr "$3" : '.*'` ]; then
> > +                                             recursion_depth="$3"
> > +                                             shift
> > +                                             shift
> > +                                     else
> > +                                             echo \<recursion depth\> not an integer
> > +                                             usage
> > +                                     fi
> > +                                     ;;
>
> Instead of checking integerness by hand, it would probably be
> much simpler if you did something like this:
>
>         depth="$3"
>         depth=$(( $depth + 0 ))
>         if test "$depth" != "$3"
>         then
>                 die "what's that '$3' for?"
>         else
>                 : happy
>         fi
>
> For a rough guideline of shell constructs we use (and do not
> use), please see Documentation/CodingGuidelines.
>



-- 
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Mobile: +880-1711402557

Re: [PATCH] - Added recurse command to git submodule

[Lars Hjemli]

On Jan 9, 2008 9:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I am not still convinced that a subcommand other than init,
> which is started recursively, should initialize and update
> submodules that are uninitialized.

I very much agree; this behaviour would break the current 'usage model'.

> I suspect that it might be a saner approach to:
>
>  - allow "git submodule recurse init [-d depth]" (although I am
>    not sure if limit by depth is so useful in practice -- only
>    experience will tell us) to auto-initialize the uninitialized
>    submodules;

A possible extension is to specifiy "inter-submodule" paths to the
init subcommand, i.e. for a possible KDE layout:
  git submodule -r init kdelibs kdelibs/admin

This should then recursively initialize the kdelibs submodule and the
admin-submodule (in the kdelibs submodule).

Btw: from my reading of the code, the git-command specified for
'recurse' will be done top-to-bottom: I guess that's what you want for
something like 'git submodule recurse diff', but not for something
like 'git submodule recurse commit' (but IMHO the latter one should
never be executed ;-)

--
larsh

Re: [PATCH] - Added recurse command to git submodule

[Junio C Hamano]

"Lars Hjemli" <hjemli@gmail.com> writes:

> A possible extension is to specifiy "inter-submodule" paths to the
> init subcommand, i.e. for a possible KDE layout:
>   git submodule -r init kdelibs kdelibs/admin
>
> This should then recursively initialize the kdelibs submodule and the
> admin-submodule (in the kdelibs submodule).

Beautiful.

> Btw: from my reading of the code, the git-command specified for
> 'recurse' will be done top-to-bottom: I guess that's what you want for
> something like 'git submodule recurse diff', but not for something
> like 'git submodule recurse commit' (but IMHO the latter one should
> never be executed ;-)

Thanks for raising a very good point.  Yes, some commands
inherently wants depth first.

While I agree that making a recursive is a grave usage error
(submodule commit and toplevel commit are logically different
events and even their commit log message should be different, as
they talk about changes in logically different levels) from
project management point of view, I do not think it is something
a tool has to explicitly forbid the users to do.  I view it as a
kind of a long rope, a misuse the users can be allowed to
inflict on themselves if they really wanted to.

Also, some commands cannot be made recursive by driving them
from a higher level recursive wrapper.  "git submodule recursive
log" would not make much sense, not only because the order of
the log entries are output from different invocations would not
be useful, but because the revision range specifier would need
to be different in different submodules (e.g. library submodules
and application submodule will not share version name namespace,
i.e. "log v1.0..v2.0" is undefined, and worse yet, running "log
v1.0:path/to/sub..v2.0:path/to/sub" in a submodule when running
"log v1.0..v2.0" in the toplevel is not a correct solution
either in general).  

Re: [PATCH] - Added recurse command to git submodule

[Imran M Yousuf]

On Jan 10, 2008 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Lars Hjemli" <hjemli@gmail.com> writes:
>
> > A possible extension is to specifiy "inter-submodule" paths to the
> > init subcommand, i.e. for a possible KDE layout:
> >   git submodule -r init kdelibs kdelibs/admin
> >
> > This should then recursively initialize the kdelibs submodule and the
> > admin-submodule (in the kdelibs submodule).
>
> Beautiful.

Firstly, thank you for the feedback Junio and Lars. Secondly, I was
not planning to add recurse to the init/update command, but your
(Lars) suggestion seems really handy; I will implement it in my patch.
About auto-initialization, I, as writing, am implementing it to be
optional and an extra -i has to be specified if the user wants to do
it.

>
> > Btw: from my reading of the code, the git-command specified for
> > 'recurse' will be done top-to-bottom: I guess that's what you want for
> > something like 'git submodule recurse diff', but not for something
> > like 'git submodule recurse commit' (but IMHO the latter one should
> > never be executed ;-)
>
> Thanks for raising a very good point.  Yes, some commands
> inherently wants depth first.
>

I could not agree more and in fact, I wanted to leave to the user how
they use the recurse command. I basically will be using for status and
diff primarily; and may be for creating and checking out branches; but
as I said it mostly depends on how the user wants to use it.

> While I agree that making a recursive is a grave usage error
> (submodule commit and toplevel commit are logically different
> events and even their commit log message should be different, as
> they talk about changes in logically different levels) from
> project management point of view, I do not think it is something
> a tool has to explicitly forbid the users to do.  I view it as a
> kind of a long rope, a misuse the users can be allowed to
> inflict on themselves if they really wanted to.
>

In fact if I am also thinking whether to add intelligence in such
scenarios. What do you think if we choose DF of BF based on the
command and options?

> Also, some commands cannot be made recursive by driving them
> from a higher level recursive wrapper.  "git submodule recursive
> log" would not make much sense, not only because the order of
> the log entries are output from different invocations would not
> be useful, but because the revision range specifier would need
> to be different in different submodules (e.g. library submodules
> and application submodule will not share version name namespace,
> i.e. "log v1.0..v2.0" is undefined, and worse yet, running "log
> v1.0:path/to/sub..v2.0:path/to/sub" in a submodule when running
> "log v1.0..v2.0" in the toplevel is not a correct solution
> either in general).
>

What is you suggestion in such cases Junio?

>

-- 
Imran M Yousuf

Re: [PATCH] - Added recurse command to git submodule

[Junio C Hamano]

"Imran M Yousuf" <imyousuf@gmail.com> writes:

>> Also, some commands cannot be made recursive by driving them
>> from a higher level recursive wrapper.  "git submodule recursive
>> log" would not make much sense, not only because the order of
>> the log entries are output from different invocations would not
>> be useful, but because the revision range specifier would need
>> to be different in different submodules (e.g. library submodules
>> and application submodule will not share version name namespace,
>> i.e. "log v1.0..v2.0" is undefined, and worse yet, running "log
>> v1.0:path/to/sub..v2.0:path/to/sub" in a submodule when running
>> "log v1.0..v2.0" in the toplevel is not a correct solution
>> either in general).
>
> What is you suggestion in such cases Junio?

Not doing it using the wrapper approach, but actually have the
underlying command be aware of the recursiveness.

Let's take a small example of library project contained within
an application project as a submodule (think of ffmpeg in
mplayer or something like that).

Library project has this history:

             3---A
            /
    ---1---2---4---B

while the application project has this history:

    ---5---X---6---Y

and at time X (and before that point), it binds commit A at a
directory "lib/" as a submodule.  The commit 6 (between X and Y)
changes it to bind commit B there instead.  You have both
toplevel and submodule checked out.  The HEAD in the application
project is at Y while the HEAD in the library project is at B.
Your work tree may or may not be clean.

How would a recursive "git diff" between two versions should
behave, with various arguments?

	$ git diff X Y

Currently this will say something like:

	--- a/lib
        +++ b/lib
	@@ -1 +1 @@
	-Subproject commit A
        +Subproject commit B

You can make it recurse naturally by recursing into lib/
subproject instead (at least conceptually this is a simple
change but it may not be so straightforward, implementation
wise).

How would you handle this, then?

	$ git diff X Y -- Documentation/

A wrapper approach that blindly descends into lib/ and runs "git
diff X Y -- Documentation/" there is wrong at two levels.
Commits X and Y do not even exist there, and Documentation/
pathspec is wrong.  The documentation may be called docs/ in the
library project, or it may not even exist, and that is not what
the user asked to see anyway.  If the user were interested in
the documentation of the library, the pathspec would have said
lib/Documentaiton/ (or lib/docs/).

For "git diff", the right solution happens to be invoking the
command recursively without any pathspec.  The higher level
chose to recurse into the directory already because it saw
changes --- by definition everything underneath is interesting.

	Side note.  If we support asking for lib/docs/ from the
	toplevel, the recursive one would use docs/ as its
	pathspec in this case.  

The point is that pathspec needs to be munged from the higher
level iteration, and more importantly that is pretty much
specific to "git diff".  "git diff" itself needs to have the
knowledge of what to do when working recursively --- wrapper
approach would not work well.

How would a recursive version of "git log" work, then?

	$ git log X..Y

Again, a naive wrapper approach of descending into lib/ and
running "git log X..Y" recursively would not give us anything
useful.

But if "git log" itself knew recursive behaviour, it should be
able to do much better.  It can show Y and 6, and at that point
it could notice that between 6 and its parent X the commit bound
at lib/ as submodule has changed from A to B, so it could insert
the log from submodule there.  If we were running with
--left-right, we might see something like this:

	>Y
	>6
            >B
            >4
            >2
            <A
            <3
	    -2
	-X

If the toplevel "git log" was invoked with a pathspec, again, it
needs to be adjusted to submodule.

I think a wrapper approach could be adequate for simple things
like "checking out the whole tree including all of its
submodules".  But even that has to be done carefully.

What should this command (recursive version) do?

	$ git checkout X

The toplevel detaches head at commit X, and notices that it
contains a submodule at lib/ whose HEAD now needs to point at
A.  The recursive command should go there, and say

	$ git checkout A

What should it do when this checkout cannot be made because your
work tree is not clean?  Ideally, it should abort and roll-back
the checkout of commit X at the toplevel (otherwise you will end
up in a mixed state).

There are more interesting issues when you bring up a situation
where X and Y binds that library project at different place
(i.e. submodule was moved inside the toplevel), which I avoided
to talk about here to keep this message short.

Re: [PATCH] - Added recurse command to git submodule

[Imran M Yousuf]

On Jan 10, 2008 10:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Imran M Yousuf" <imyousuf@gmail.com> writes:
>
> >> Also, some commands cannot be made recursive by driving them
> >> from a higher level recursive wrapper.  "git submodule recursive
> >> log" would not make much sense, not only because the order of
> >> the log entries are output from different invocations would not
> >> be useful, but because the revision range specifier would need
> >> to be different in different submodules (e.g. library submodules
> >> and application submodule will not share version name namespace,
> >> i.e. "log v1.0..v2.0" is undefined, and worse yet, running "log
> >> v1.0:path/to/sub..v2.0:path/to/sub" in a submodule when running
> >> "log v1.0..v2.0" in the toplevel is not a correct solution
> >> either in general).
> >
> > What is you suggestion in such cases Junio?
>
> Not doing it using the wrapper approach, but actually have the
> underlying command be aware of the recursiveness.

Yes, I do agree fully that the wrapper approach has problem in
recursing; as I mentioned earlier it is the user who has to be aware
of the command he wants to execute recursively; as all currently
available work can still be performed even if the wrapper recurse is
added. But at least having recurse would allow users to execute
certain simple commands from the top level which otherwise would have
required to be done from each sub-module.

>
> Let's take a small example of library project contained within
> an application project as a submodule (think of ffmpeg in
> mplayer or something like that).
>
> Library project has this history:
>
>              3---A
>             /
>     ---1---2---4---B
>
> while the application project has this history:
>
>     ---5---X---6---Y
>
> and at time X (and before that point), it binds commit A at a
> directory "lib/" as a submodule.  The commit 6 (between X and Y)
> changes it to bind commit B there instead.  You have both
> toplevel and submodule checked out.  The HEAD in the application
> project is at Y while the HEAD in the library project is at B.
> Your work tree may or may not be clean.
>
> How would a recursive "git diff" between two versions should
> behave, with various arguments?
>
>         $ git diff X Y
>
> Currently this will say something like:
>
>         --- a/lib
>         +++ b/lib
>         @@ -1 +1 @@
>         -Subproject commit A
>         +Subproject commit B
>
> You can make it recurse naturally by recursing into lib/
> subproject instead (at least conceptually this is a simple
> change but it may not be so straightforward, implementation
> wise).
>
> How would you handle this, then?
>
>         $ git diff X Y -- Documentation/
>
> A wrapper approach that blindly descends into lib/ and runs "git
> diff X Y -- Documentation/" there is wrong at two levels.
> Commits X and Y do not even exist there, and Documentation/
> pathspec is wrong.  The documentation may be called docs/ in the
> library project, or it may not even exist, and that is not what
> the user asked to see anyway.  If the user were interested in
> the documentation of the library, the pathspec would have said
> lib/Documentaiton/ (or lib/docs/).
>
> For "git diff", the right solution happens to be invoking the
> command recursively without any pathspec.  The higher level
> chose to recurse into the directory already because it saw
> changes --- by definition everything underneath is interesting.
>
>         Side note.  If we support asking for lib/docs/ from the
>         toplevel, the recursive one would use docs/ as its
>         pathspec in this case.
>
> The point is that pathspec needs to be munged from the higher
> level iteration, and more importantly that is pretty much
> specific to "git diff".  "git diff" itself needs to have the
> knowledge of what to do when working recursively --- wrapper
> approach would not work well.

Yes, it is absolutely right that if the command was aware of the
recursion it could necessary measures to ensure that it can fallback
to default execution; e.g. remove the path spec from git-diff.
Alternatively, the recurse module could actually check whether if
simply passed the command would generate error or not; if yes then
display a message mentioning it and fallback to simplest form of the
command. The real disadvantage is that, firstly in this approach the
command gets executed twice in average case and secondly, the module
will have to know the fallback version of the command.

>
> How would a recursive version of "git log" work, then?
>
>         $ git log X..Y
>
> Again, a naive wrapper approach of descending into lib/ and
> running "git log X..Y" recursively would not give us anything
> useful.
>
> But if "git log" itself knew recursive behaviour, it should be
> able to do much better.  It can show Y and 6, and at that point
> it could notice that between 6 and its parent X the commit bound
> at lib/ as submodule has changed from A to B, so it could insert
> the log from submodule there.  If we were running with
> --left-right, we might see something like this:
>
>         >Y
>         >6
>             >B
>             >4
>             >2
>             <A
>             <3
>             -2
>         -X
>
> If the toplevel "git log" was invoked with a pathspec, again, it
> needs to be adjusted to submodule.
>
> I think a wrapper approach could be adequate for simple things
> like "checking out the whole tree including all of its
> submodules".  But even that has to be done carefully.
>
> What should this command (recursive version) do?
>
>         $ git checkout X
>
> The toplevel detaches head at commit X, and notices that it
> contains a submodule at lib/ whose HEAD now needs to point at
> A.  The recursive command should go there, and say
>
>         $ git checkout A
>
> What should it do when this checkout cannot be made because your
> work tree is not clean?  Ideally, it should abort and roll-back
> the checkout of commit X at the toplevel (otherwise you will end
> up in a mixed state).
>
> There are more interesting issues when you bring up a situation
> where X and Y binds that library project at different place
> (i.e. submodule was moved inside the toplevel), which I avoided
> to talk about here to keep this message short.
>

Thank you for the detailed explanation and I can visualize more
scenarios which you did not mention. From this I get the following
ideas for the recurse command -

1. Instead of supporting all he git commands support a subset with
limited feature

2. Support full range of git commands, as is submitted in this patch,
but if error occurs in execution fall back to the default version of
the command. This is probably not a efficient version; but would be
beneficial as other commands will not be effected.

3. Support full range of git commands, as is submitted in this patch,
leave it to the user on how he wants to use it.

4. Scrap the recurse idea (I actually do not prefer it :)).

Let me know which one you prefer (please not 4 :)).

>
>

Best regards,

-- 
Imran M Yousuf