"Imran M Yousuf" [off-list ref] 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.
quoted hunk
@@ -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?
quoted hunk
@@ -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.
quoted hunk
@@ -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" "$@"
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.
quoted
+ 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 [off-list ref] wrote:
"Imran M Yousuf" [off-list ref] writes:
quoted
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.
quoted
@@ -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".
quoted
+# 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.
quoted
+ 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
)
quoted
+}
+
+# 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.
quoted
+ if [ `expr index "$arg" ' '` -gt 0 ]; then
Do we ever use `expr index` anywhere else? I thought we fixed
that non-portability.
quoted
+ 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?
quoted
@@ -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.
quoted
@@ -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