git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Added recurse command to git submodule
@ 2008-01-08  3:14 Imran M Yousuf
  2008-01-08  5:44 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Imran M Yousuf @ 2008-01-08  3:14 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 4114 bytes --]

Added a recurse command to git-submodule. Using this recurse command
any git command (for example, git-status, git-diff, git-checkout) can
be performed from the top level to all its submodules at any depth; if
the module has not been initialized and updated (that is git sumodule
init and git submodule update) the script will take care of that too.

I needed this feature especially for diff, status and pull; as I am
working on a multi module maven project with each module having
separate repository due to architecture and future extensibility.

Following is the diff with git-submdoule version 1.5.3.7. I also
attached the diff and the modified file in the attachment.

diff --git a/git-submodule b/git-submodule
index b91d626..e819152 100755
--- a/git-submodule
+++ b/git-submodule
@@ -1,10 +1,11 @@
 #!/bin/sh
 #
 # git-submodules.sh: add, init, update or list git submodules
+# or recurse any git command over the submodules recursively.
 #
 # 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] command
arguments ...]]'
 . git-sh-setup
 require_work_tree

@@ -251,6 +252,78 @@ modules_list()
 	done
 }

+# Simply checks whether the submodule is initialized
+# or not. If not initialized it does so.
+initializeSubModule() {
+	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.
+traverseModule() {
+	current_dir=`pwd`
+	dir_path="$current_dir:$dir_path"
+	initializeSubModule "$1"
+        cd "$1"
+	if [ $recurse_verbose -eq 1 ]; then
+		echo Working in mod $1 @ `pwd` with $2
+	fi
+        eval "$2"
+	if [ -f .gitmodules ]; then
+                for mod_path in `grep "path =" .gitmodules | awk
'{print $3}'`; do
+                        traverseModule "$mod_path" "$2"
+                done
+        fi
+	old_dir=$(echo $dir_path | cut -d':' -f1-1)
+	length_old_dir=`expr "$old_dir" : '.*'`
+	cd $old_dir
+	index=$(echo "$length_old_dir+2" | bc)
+	dir_path=`echo $dir_path $index | awk '{print substr($1, $2)}'`
+}
+
+# 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
+propagate() {
+	project_home=`pwd`
+	echo Project Home: $project_home
+	if [ -d $project_home/.git/ ]; then
+		git_command=$1
+		shift
+		command_arguments=""
+		for arg in "$@"; do
+			if [ `expr index "$arg" ' '` -gt 0 ]; then
+				arg="\"$arg\""
+			fi
+			command_arguments="$command_arguments $arg"
+		done
+		if [ $recurse_verbose -eq 1 ]; then
+			echo GIT Command git-$git_command with arguments\($#\) "$command_arguments"
+		fi
+		main_command="git-$git_command $command_arguments"
+		eval $main_command
+		if [ -f .gitmodules ]; then
+			for mod_path in `grep "path =" .gitmodules | awk '{print $3}'`; do
+				traverseModule $mod_path "$main_command"
+			done
+		fi
+	else
+		echo $project_home not a git repo thus exiting
+		exit
+	fi
+}
+
+recurse_verbose=0
 while test $# != 0
 do
 	case "$1" in
@@ -286,6 +359,17 @@ do
 	-*)
 		usage
 		;;
+	recurse)
+		recurse=1
+		case "$2" in
+			-v)
+				recurse_verbose=1
+				shift
+				;;
+		esac
+		shift
+		break
+		;;
 	*)
 		break
 		;;
@@ -303,17 +387,21 @@ case "$add,$branch" in
 	;;
 esac

-case "$add,$init,$update,$status,$cached" in
-1,,,,)
+
+case "$add,$init,$update,$recurse,$status,$cached" in
+1,,,,,)
 	module_add "$@"
 	;;
-,1,,,)
+,1,,,,)
 	modules_init "$@"
 	;;
-,,1,,)
+,,1,,,)
 	modules_update "$@"
 	;;
-,,,*,*)
+,,,1,,)
+	propagate "$@"
+	;;
+,,,,*,*)
 	modules_list "$@"
 	;;
 *)

-- 
Imran M Yousuf

[-- Attachment #2: git-submodule-diff-1.5.3.7.txt --]
[-- Type: text/plain, Size: 3425 bytes --]

diff --git a/git-submodule b/git-submodule
index b91d626..e819152 100755
--- a/git-submodule
+++ b/git-submodule
@@ -1,10 +1,11 @@
 #!/bin/sh
 #
 # git-submodules.sh: add, init, update or list git submodules
+# or recurse any git command over the submodules recursively.
 #
 # 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] command arguments ...]]'
 . git-sh-setup
 require_work_tree
 
@@ -251,6 +252,78 @@ modules_list()
 	done
 }
 
+# Simply checks whether the submodule is initialized
+# or not. If not initialized it does so.
+initializeSubModule() {
+	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.
+traverseModule() {
+	current_dir=`pwd`
+	dir_path="$current_dir:$dir_path"
+	initializeSubModule "$1"
+        cd "$1"
+	if [ $recurse_verbose -eq 1 ]; then
+		echo Working in mod $1 @ `pwd` with $2
+	fi
+        eval "$2"
+	if [ -f .gitmodules ]; then
+                for mod_path in `grep "path =" .gitmodules | awk '{print $3}'`; do
+                        traverseModule "$mod_path" "$2"
+                done
+        fi
+	old_dir=$(echo $dir_path | cut -d':' -f1-1)
+	length_old_dir=`expr "$old_dir" : '.*'`
+	cd $old_dir
+	index=$(echo "$length_old_dir+2" | bc)
+	dir_path=`echo $dir_path $index | awk '{print substr($1, $2)}'`
+}
+
+# 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
+propagate() {
+	project_home=`pwd`
+	echo Project Home: $project_home
+	if [ -d $project_home/.git/ ]; then
+		git_command=$1
+		shift
+		command_arguments=""
+		for arg in "$@"; do
+			if [ `expr index "$arg" ' '` -gt 0 ]; then
+				arg="\"$arg\""
+			fi 
+			command_arguments="$command_arguments $arg"
+		done
+		if [ $recurse_verbose -eq 1 ]; then
+			echo GIT Command git-$git_command with arguments\($#\) "$command_arguments"
+		fi
+		main_command="git-$git_command $command_arguments"
+		eval $main_command
+		if [ -f .gitmodules ]; then
+			for mod_path in `grep "path =" .gitmodules | awk '{print $3}'`; do
+				traverseModule $mod_path "$main_command"
+			done
+		fi
+	else
+		echo $project_home not a git repo thus exiting
+		exit
+	fi
+}
+
+recurse_verbose=0
 while test $# != 0
 do
 	case "$1" in
@@ -286,6 +359,17 @@ do
 	-*)
 		usage
 		;;
+	recurse)
+		recurse=1
+		case "$2" in
+			-v)
+				recurse_verbose=1
+				shift
+				;;
+		esac
+		shift
+		break
+		;;
 	*)
 		break
 		;;
@@ -303,17 +387,21 @@ case "$add,$branch" in
 	;;
 esac
 
-case "$add,$init,$update,$status,$cached" in
-1,,,,)
+
+case "$add,$init,$update,$recurse,$status,$cached" in
+1,,,,,)
 	module_add "$@"
 	;;
-,1,,,)
+,1,,,,)
 	modules_init "$@"
 	;;
-,,1,,)
+,,1,,,)
 	modules_update "$@"
 	;;
-,,,*,*)
+,,,1,,)
+	propagate "$@"
+	;;
+,,,,*,*)
 	modules_list "$@"
 	;;
 *)

[-- Attachment #3: git-submodule --]
[-- Type: application/octet-stream, Size: 8787 bytes --]

#!/bin/sh
#
# git-submodules.sh: add, init, update or list git submodules
# or recurse any git command over the submodules recursively.
#
# Copyright (c) 2007 Lars Hjemli

USAGE='[[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]|[recurse [-v] command arguments ...]]'
. git-sh-setup
require_work_tree

add=
branch=
init=
update=
status=
quiet=
cached=

#
# print stuff on stdout unless -q was specified
#
say()
{
	if test -z "$quiet"
	then
		echo "$@"
	fi
}

# NEEDSWORK: identical function exists in get_repo_base in clone.sh
get_repo_base() {
	(
		cd "`/bin/pwd`" &&
		cd "$1" || cd "$1.git" &&
		{
			cd .git
			pwd
		}
	) 2>/dev/null
}

#
# Map submodule path to submodule name
#
# $1 = path
#
module_name()
{
	# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
	re=$(printf '%s' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
	name=$( GIT_CONFIG=.gitmodules \
		git config --get-regexp '^submodule\..*\.path$' |
		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
       test -z "$name" &&
       die "No submodule mapping found in .gitmodules for path '$path'"
       echo "$name"
}

#
# Clone a submodule
#
# Prior to calling, modules_update checks that a possibly existing
# path is not a git repository.
# Likewise, module_add checks that path does not exist at all,
# since it is the location of a new submodule.
#
module_clone()
{
	path=$1
	url=$2

	# If there already is a directory at the submodule path,
	# expect it to be empty (since that is the default checkout
	# action) and try to remove it.
	# Note: if $path is a symlink to a directory the test will
	# succeed but the rmdir will fail. We might want to fix this.
	if test -d "$path"
	then
		rmdir "$path" 2>/dev/null ||
		die "Directory '$path' exist, but is neither empty nor a git repository"
	fi

	test -e "$path" &&
	die "A file already exist at path '$path'"

	git-clone -n "$url" "$path" ||
	die "Clone of '$url' into submodule path '$path' failed"
}

#
# Add a new submodule to the working tree, .gitmodules and the index
#
# $@ = repo [path]
#
# optional branch is stored in global branch variable
#
module_add()
{
	repo=$1
	path=$2

	if test -z "$repo"; then
		usage
	fi

	# Turn the source into an absolute path if
	# it is local
	if base=$(get_repo_base "$repo"); then
		repo="$base"
	fi

	# Guess path from repo if not specified or strip trailing slashes
	if test -z "$path"; then
		path=$(echo "$repo" | sed -e 's|/*$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
	else
		path=$(echo "$path" | sed -e 's|/*$||')
	fi

	test -e "$path" &&
	die "'$path' already exists"

	git ls-files --error-unmatch "$path" > /dev/null 2>&1 &&
	die "'$path' already exists in the index"

	module_clone "$path" "$repo" || exit
	(unset GIT_DIR && cd "$path" && git checkout -q ${branch:+-b "$branch" "origin/$branch"}) ||
	die "Unable to checkout submodule '$path'"
	git add "$path" ||
	die "Failed to add submodule '$path'"

	GIT_CONFIG=.gitmodules git config submodule."$path".path "$path" &&
	GIT_CONFIG=.gitmodules git config submodule."$path".url "$repo" &&
	git add .gitmodules ||
	die "Failed to register submodule '$path'"
}

#
# Register submodules in .git/config
#
# $@ = requested paths (default to all)
#
modules_init()
{
	git ls-files --stage -- "$@" | grep -e '^160000 ' |
	while read mode sha1 stage path
	do
		# Skip already registered paths
		name=$(module_name "$path") || exit
		url=$(git config submodule."$name".url)
		test -z "$url" || continue

		url=$(GIT_CONFIG=.gitmodules git config submodule."$name".url)
		test -z "$url" &&
		die "No url found for submodule path '$path' in .gitmodules"

		git config submodule."$name".url "$url" ||
		die "Failed to register url for submodule path '$path'"

		say "Submodule '$name' ($url) registered for path '$path'"
	done
}

#
# Update each submodule path to correct revision, using clone and checkout as needed
#
# $@ = requested paths (default to all)
#
modules_update()
{
	git ls-files --stage -- "$@" | grep -e '^160000 ' |
	while read mode sha1 stage path
	do
		name=$(module_name "$path") || exit
		url=$(git config submodule."$name".url)
		if test -z "$url"
		then
			# Only mention uninitialized submodules when its
			# path have been specified
			test "$#" != "0" &&
			say "Submodule path '$path' not initialized"
			continue
		fi

		if ! test -d "$path"/.git
		then
			module_clone "$path" "$url" || exit
			subsha1=
		else
			subsha1=$(unset GIT_DIR && cd "$path" &&
				git rev-parse --verify HEAD) ||
			die "Unable to find current revision in submodule path '$path'"
		fi

		if test "$subsha1" != "$sha1"
		then
			(unset GIT_DIR && cd "$path" && git-fetch &&
				git-checkout -q "$sha1") ||
			die "Unable to checkout '$sha1' in submodule path '$path'"

			say "Submodule path '$path': checked out '$sha1'"
		fi
	done
}

set_name_rev () {
	revname=$( (
		unset GIT_DIR &&
		cd "$1" && {
			git describe "$2" 2>/dev/null ||
			git describe --tags "$2" 2>/dev/null ||
			git describe --contains --tags "$2"
		}
	) )
	test -z "$revname" || revname=" ($revname)"
}

#
# List all submodules, prefixed with:
#  - submodule not initialized
#  + different revision checked out
#
# If --cached was specified the revision in the index will be printed
# instead of the currently checked out revision.
#
# $@ = requested paths (default to all)
#
modules_list()
{
	git ls-files --stage -- "$@" | grep -e '^160000 ' |
	while read mode sha1 stage path
	do
		name=$(module_name "$path") || exit
		url=$(git config submodule."$name".url)
		if test -z "url" || ! test -d "$path"/.git
		then
			say "-$sha1 $path"
			continue;
		fi
		set_name_rev "$path" "$sha1"
		if git diff-files --quiet -- "$path"
		then
			say " $sha1 $path$revname"
		else
			if test -z "$cached"
			then
				sha1=$(unset GIT_DIR && cd "$path" && git rev-parse --verify HEAD)
				set_name_rev "$path" "$sha1"
			fi
			say "+$sha1 $path$revname"
		fi
	done
}

# Simply checks whether the submodule is initialized
# or not. If not initialized it does so.
initializeSubModule() {
	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.
traverseModule() {
	current_dir=`pwd`
	dir_path="$current_dir:$dir_path"
	initializeSubModule "$1"
        cd "$1"
	if [ $recurse_verbose -eq 1 ]; then
		echo Working in mod $1 @ `pwd` with $2
	fi
        eval "$2"
	if [ -f .gitmodules ]; then
                for mod_path in `grep "path =" .gitmodules | awk '{print $3}'`; do
                        traverseModule "$mod_path" "$2"
                done
        fi
	old_dir=$(echo $dir_path | cut -d':' -f1-1)
	length_old_dir=`expr "$old_dir" : '.*'`
	cd $old_dir
	index=$(echo "$length_old_dir+2" | bc)
	dir_path=`echo $dir_path $index | awk '{print substr($1, $2)}'`
}

# 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
propagate() {
	project_home=`pwd`
	echo Project Home: $project_home
	if [ -d $project_home/.git/ ]; then
		git_command=$1
		shift
		command_arguments=""
		for arg in "$@"; do
			if [ `expr index "$arg" ' '` -gt 0 ]; then
				arg="\"$arg\""
			fi 
			command_arguments="$command_arguments $arg"
		done
		if [ $recurse_verbose -eq 1 ]; then
			echo GIT Command git-$git_command with arguments\($#\) "$command_arguments"
		fi
		main_command="git-$git_command $command_arguments"
		eval $main_command
		if [ -f .gitmodules ]; then
			for mod_path in `grep "path =" .gitmodules | awk '{print $3}'`; do
				traverseModule $mod_path "$main_command"
			done
		fi
	else
		echo $project_home not a git repo thus exiting
		exit
	fi
}

recurse_verbose=0
while test $# != 0
do
	case "$1" in
	add)
		add=1
		;;
	init)
		init=1
		;;
	update)
		update=1
		;;
	status)
		status=1
		;;
	-q|--quiet)
		quiet=1
		;;
	-b|--branch)
		case "$2" in
		'')
			usage
			;;
		esac
		branch="$2"; shift
		;;
	--cached)
		cached=1
		;;
	--)
		break
		;;
	-*)
		usage
		;;
	recurse)
		recurse=1
		case "$2" in
			-v)
				recurse_verbose=1
				shift
				;;
		esac
		shift
		break
		;;
	*)
		break
		;;
	esac
	shift
done

case "$add,$branch" in
1,*)
	;;
,)
	;;
,*)
	usage
	;;
esac


case "$add,$init,$update,$recurse,$status,$cached" in
1,,,,,)
	module_add "$@"
	;;
,1,,,,)
	modules_init "$@"
	;;
,,1,,,)
	modules_update "$@"
	;;
,,,1,,)
	propagate "$@"
	;;
,,,,*,*)
	modules_list "$@"
	;;
*)
	usage
	;;
esac

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Added recurse command to git submodule
  2008-01-08  3:14 [PATCH] Added recurse command to git submodule Imran M Yousuf
@ 2008-01-08  5:44 ` Junio C Hamano
  2008-01-08  6:57   ` Imran M Yousuf
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2008-01-08  5:44 UTC (permalink / raw)
  To: Imran M Yousuf; +Cc: git

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

> Following is the diff with git-submdoule version 1.5.3.7. I also
> attached the diff and the modified file in the attachment.

Please do not do this when the submission is for real (I think
you did not mean this for inclusion but for comment so it is Ok
this time around), as such a message cannot be processed with
our automated tools but need to be applied by hand.

Just send a non whitespace-corrupted, plain text patch.  A
corrupt text patch followed by an attachment is the worst of
both worlds.

> @@ -251,6 +252,78 @@ modules_list()
>  	done
>  }
>
> +# Simply checks whether the submodule is initialized
> +# or not. If not initialized it does so.
> +initializeSubModule() {
> +	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
> +}

The comment above sounds quite wrong, isn't it?  It is not just
"simply check" but actively makes sure it is initialized.

Do we use CamelCase in our shell script?

Making -r to always recurse _fully_ feels quite wrong.  It is
one thing to allow "git submodule init --recursive" to recurse
fully and initialize everything, but it is another thing to
force full instantiation of submodules that the user _chose_ not
to check out, when the user has checkout of some but not all
submodules and asks "git submodule update --recursive".

> +# 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.
> +traverseModule() {
> +	current_dir=`pwd`
> +	dir_path="$current_dir:$dir_path"
> +	initializeSubModule "$1"
> +        cd "$1"
> +	if [ $recurse_verbose -eq 1 ]; then
> +		echo Working in mod $1 @ `pwd` with $2
> +	fi

Always quote variable references unless you mean you want field
splitting.  Even when you know $1 and $2 do not have IFS
characters, to make it easier to readers.

> +        eval "$2"
> +	if [ -f .gitmodules ]; then
> +                for mod_path in `grep "path =" .gitmodules | awk
> '{print $3}'`; do
> +                        traverseModule "$mod_path" "$2"
> +                done
> +        fi
> +	old_dir=$(echo $dir_path | cut -d':' -f1-1)
> +	length_old_dir=`expr "$old_dir" : '.*'`
> +	cd $old_dir
> +	index=$(echo "$length_old_dir+2" | bc)
> +	dir_path=`echo $dir_path $index | awk '{print substr($1, $2)}'`

This dir_path separated with ":" and shuffling the $cwd with
chdir'ing around makes me say "Yuck".  Is it essential that
these operation in the submodule happen in the same process?
IOW, wouldn't it be enough to do something like:


	initialize_submodule "$1"
        (
        	cd "$1"
                if test -n "$recursive_verbose"
                then
                	echo "Working in module $1 with $2"
		fi
                eval "$2"
		if test -f .gitmodules
                then
                	for p in `sed -n -e 's/^path = //p' .gitmodules`
			do
                        	traverse_module "$p" "$2"
			done
		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
> +propagate() {
> +	project_home=`pwd`
> +	echo Project Home: $project_home
> +	if [ -d $project_home/.git/ ]; then
> +		git_command=$1
> +		shift
> +		command_arguments=""
> +		for arg in "$@"; do

That 'in "$@"' is distracting.

	for arg
        do
		...

is enough.

> +			if [ `expr index "$arg" ' '` -gt 0 ]; then

Do we ever use `expr index` anywhere else?  I thought we fixed
that non-portability.

> +				arg="\"$arg\""
> +			fi
> +			command_arguments="$command_arguments $arg"
> +		done
> +		if [ $recurse_verbose -eq 1 ]; then
> +			echo GIT Command git-$git_command with arguments\($#\) "$command_arguments"
> +		fi
> +		main_command="git-$git_command $command_arguments"
> +		eval $main_command

This feels like a sloppy quoting, although I haven't looked at
the code very deeply.  Does it work when $arg has funny letters
like backslash, double-quote and or backquote?

> @@ -286,6 +359,17 @@ do
>  	-*)
>  		usage
>  		;;
> +	recurse)
> +		recurse=1
> +		case "$2" in
> +			-v)
> +				recurse_verbose=1
> +				shift
> +				;;
> +		esac
> +		shift
> +		break
> +		;;

I was ahead of myself earlier but I think it makes more sense to
pretend (at the command line syntax level) that --recursive is
an option to other commands.

> @@ -303,17 +387,21 @@ case "$add,$branch" in
>  	;;
>  esac
>
> -case "$add,$init,$update,$status,$cached" in
> -1,,,,)
> +
> +case "$add,$init,$update,$recurse,$status,$cached" in
> +1,,,,,)
>  	module_add "$@"
>  	;;
> -,1,,,)
> +,1,,,,)
>  	modules_init "$@"
>  	;;
> -,,1,,)
> +,,1,,,)
>  	modules_update "$@"
>  	;;
> -,,,*,*)
> +,,,1,,)
> +	propagate "$@"
> +	;;
> +,,,,*,*)
>  	modules_list "$@"
>  	;;
>  *)

I've always hated this part of the script.  How about a bit of
clean-up patch first before adding $recurse or anything else, to
introduce command variable and do:

	case "$command" in
        add)
        	module_add "$@"
                ;;
	init)
        	module_init "$@"
                ;;
                ...
	*)
        	modules_list "$@"
                ;;
	esac

or even just a single:

	"module_$command" "$@"

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Added recurse command to git submodule
  2008-01-08  5:44 ` Junio C Hamano
@ 2008-01-08  6:57   ` Imran M Yousuf
  0 siblings, 0 replies; 3+ messages in thread
From: Imran M Yousuf @ 2008-01-08  6:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio,

Thanks for the comments.
FIrstly, yes I did not mean for inclusion right now, but rather for
comment. I am already working with the branched out versions of the
files.
Secondly, I should apologize for messing the patches, as I read the
SubmittingPatches actually after sending out the patches. From next
time, I will try to follow the document strictly.
Sorry for mixing camel case, being a Java developer I always tend to
use them, but will fix them right now :). Will be also changing the
comments. I will also change the for loop to as you suggested.
Thanks a lot for the chdir solution. I actually did not that it could
be done; that saves a lot of effort. Will surely change it
accordingly.
I was actually not planning to introduce git submodule init
--recursive or git submodule update --recursive; but I will also
introduce it. Just wanted to clarify that only git submodule init will
behave as it used to earlier and there is no recursion involved.

>
> > +                             arg="\"$arg\""
> > +                     fi
> > +                     command_arguments="$command_arguments $arg"
> > +             done
> > +             if [ $recurse_verbose -eq 1 ]; then
> > +                     echo GIT Command git-$git_command with arguments\($#\) "$command_arguments"
> > +             fi
> > +             main_command="git-$git_command $command_arguments"
> > +             eval $main_command
>
> This feels like a sloppy quoting, although I haven't looked at
> the code very deeply.  Does it work when $arg has funny letters
> like backslash, double-quote and or backquote?

it does work if $arg has double-quote (commit -m "commit message"), I
will also test with the other 2 characters you mentioned.

I just wanted to confirm that I will submit the patch for commands of
submodule by today. I do agree with having --recursive option with all
the commands, but, IMHO, as it is particularly useful for submodules
only it also makes sense if it is a separate command for submodule.

At first I will make the command change and send the patch today and
then make the other patches and send them over.

Thank you once again for the hints.

Imran

On Jan 8, 2008 11:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Imran M Yousuf" <imyousuf@gmail.com> writes:
>
> > Following is the diff with git-submdoule version 1.5.3.7. I also
> > attached the diff and the modified file in the attachment.
>
> Please do not do this when the submission is for real (I think
> you did not mean this for inclusion but for comment so it is Ok
> this time around), as such a message cannot be processed with
> our automated tools but need to be applied by hand.
>
> Just send a non whitespace-corrupted, plain text patch.  A
> corrupt text patch followed by an attachment is the worst of
> both worlds.
>
> > @@ -251,6 +252,78 @@ modules_list()
> >       done
> >  }
> >
> > +# Simply checks whether the submodule is initialized
> > +# or not. If not initialized it does so.
> > +initializeSubModule() {
> > +     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
> > +}
>
> The comment above sounds quite wrong, isn't it?  It is not just
> "simply check" but actively makes sure it is initialized.
>
> Do we use CamelCase in our shell script?
>
> Making -r to always recurse _fully_ feels quite wrong.  It is
> one thing to allow "git submodule init --recursive" to recurse
> fully and initialize everything, but it is another thing to
> force full instantiation of submodules that the user _chose_ not
> to check out, when the user has checkout of some but not all
> submodules and asks "git submodule update --recursive".
>
> > +# 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.
> > +traverseModule() {
> > +     current_dir=`pwd`
> > +     dir_path="$current_dir:$dir_path"
> > +     initializeSubModule "$1"
> > +        cd "$1"
> > +     if [ $recurse_verbose -eq 1 ]; then
> > +             echo Working in mod $1 @ `pwd` with $2
> > +     fi
>
> Always quote variable references unless you mean you want field
> splitting.  Even when you know $1 and $2 do not have IFS
> characters, to make it easier to readers.
>
> > +        eval "$2"
> > +     if [ -f .gitmodules ]; then
> > +                for mod_path in `grep "path =" .gitmodules | awk
> > '{print $3}'`; do
> > +                        traverseModule "$mod_path" "$2"
> > +                done
> > +        fi
> > +     old_dir=$(echo $dir_path | cut -d':' -f1-1)
> > +     length_old_dir=`expr "$old_dir" : '.*'`
> > +     cd $old_dir
> > +     index=$(echo "$length_old_dir+2" | bc)
> > +     dir_path=`echo $dir_path $index | awk '{print substr($1, $2)}'`
>
> This dir_path separated with ":" and shuffling the $cwd with
> chdir'ing around makes me say "Yuck".  Is it essential that
> these operation in the submodule happen in the same process?
> IOW, wouldn't it be enough to do something like:
>
>
>         initialize_submodule "$1"
>         (
>                 cd "$1"
>                 if test -n "$recursive_verbose"
>                 then
>                         echo "Working in module $1 with $2"
>                 fi
>                 eval "$2"
>                 if test -f .gitmodules
>                 then
>                         for p in `sed -n -e 's/^path = //p' .gitmodules`
>                         do
>                                 traverse_module "$p" "$2"
>                         done
>                 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
> > +propagate() {
> > +     project_home=`pwd`
> > +     echo Project Home: $project_home
> > +     if [ -d $project_home/.git/ ]; then
> > +             git_command=$1
> > +             shift
> > +             command_arguments=""
> > +             for arg in "$@"; do
>
> That 'in "$@"' is distracting.
>
>         for arg
>         do
>                 ...
>
> is enough.
>
> > +                     if [ `expr index "$arg" ' '` -gt 0 ]; then
>
> Do we ever use `expr index` anywhere else?  I thought we fixed
> that non-portability.
>
> > +                             arg="\"$arg\""
> > +                     fi
> > +                     command_arguments="$command_arguments $arg"
> > +             done
> > +             if [ $recurse_verbose -eq 1 ]; then
> > +                     echo GIT Command git-$git_command with arguments\($#\) "$command_arguments"
> > +             fi
> > +             main_command="git-$git_command $command_arguments"
> > +             eval $main_command
>
> This feels like a sloppy quoting, although I haven't looked at
> the code very deeply.  Does it work when $arg has funny letters
> like backslash, double-quote and or backquote?
>
> > @@ -286,6 +359,17 @@ do
> >       -*)
> >               usage
> >               ;;
> > +     recurse)
> > +             recurse=1
> > +             case "$2" in
> > +                     -v)
> > +                             recurse_verbose=1
> > +                             shift
> > +                             ;;
> > +             esac
> > +             shift
> > +             break
> > +             ;;
>
> I was ahead of myself earlier but I think it makes more sense to
> pretend (at the command line syntax level) that --recursive is
> an option to other commands.
>
> > @@ -303,17 +387,21 @@ case "$add,$branch" in
> >       ;;
> >  esac
> >
> > -case "$add,$init,$update,$status,$cached" in
> > -1,,,,)
> > +
> > +case "$add,$init,$update,$recurse,$status,$cached" in
> > +1,,,,,)
> >       module_add "$@"
> >       ;;
> > -,1,,,)
> > +,1,,,,)
> >       modules_init "$@"
> >       ;;
> > -,,1,,)
> > +,,1,,,)
> >       modules_update "$@"
> >       ;;
> > -,,,*,*)
> > +,,,1,,)
> > +     propagate "$@"
> > +     ;;
> > +,,,,*,*)
> >       modules_list "$@"
> >       ;;
> >  *)
>
> I've always hated this part of the script.  How about a bit of
> clean-up patch first before adding $recurse or anything else, to
> introduce command variable and do:
>
>         case "$command" in
>         add)
>                 module_add "$@"
>                 ;;
>         init)
>                 module_init "$@"
>                 ;;
>                 ...
>         *)
>                 modules_list "$@"
>                 ;;
>         esac
>
> or even just a single:
>
>         "module_$command" "$@"
>
>
>



-- 
Imran M Yousuf

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-01-08  6:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-08  3:14 [PATCH] Added recurse command to git submodule Imran M Yousuf
2008-01-08  5:44 ` Junio C Hamano
2008-01-08  6:57   ` Imran M Yousuf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).