imyousuf@xxxxxxxxx writes:
> The recurse commands behavior can be customized with several arguments
> that it accepts. The synopsis for the recurse command is:
>
> git-submodule recurse [-q|--quiet] [-e|--exit-after-error]
> [-d|--depth <recursion depth>] [-b|--breadth-first]
> <git command> [<arguments> ...]
Is there a reason to limit the command that can be run per submodule to
only "git" commands? To me, this "recurse" looks like a glorified "find"
command that can trigger its action only to submodule directories, but
limits what can be given to its -exec option to "git" commands. While it
would not make sense to give certain git command to recurse (e.g. neither
"git show 65ea3b8" nor "git clone $there" would make any sense), it would
be handy if we can give certain non-git commands to it (e.g. "du -sh").
> @@ -580,6 +585,129 @@ cmd_status()
> done
> }
>
> +# Check whether the submodule is initialized or not
> +initialize_sub_module()
Everybody else seems to spell "<do-something>_submodule"; should this be
any different?
> +{
> + if test ! -d "$1"/.git
> + then
> + say "Submodule $1 is not initialized and skipped"
> + return 1
> + # Returns true if submodule is already initialized
Micronit; s/Returns/Return/. A sentence that begins with a capitalized
verb in comments is almost always in imperative mood, not third-person
singular present.
> + elif test -d "$1"/.git
> + then
> + return 0
> + fi
> +}
Otherwise, what does it return? Do you need elif there, or just "else"?
> +# This function simply checks whether the depth is traverseable in terms of
> +# depth and if so then it sequentially traverses its submodules
> +traverse_submodules()
> +{
> + # If current depth is the range specified than it will continue
> + # else return with success
> + if test "$depth" -gt 0 &&
> + test "$current_depth" -ge "$depth"
> + then
> + return 0;
> + fi
> + # If submodules exists than it will traverse over them
> + if test -f .gitmodules
> + then
> + # Incrementing the depth for the next level of submodules
> + current_depth=$(($current_depth + 1))
> + for mod_path in `sed -n -e 's/path = //p' .gitmodules`; do
> + traverse_module "$mod_path" "$@"
> + done
> + # Decremented the depth to bring it back to the depth of
> + # the current submodule
> + current_depth=$(($current_depth - 1))
> + fi
> +}
This makes me wonder if you should be iterating over .gitmodules, or
perhaps you may want to iterate over output of git-ls-files (picking
entries of gitlink type). How should a local change that adds a new
submodule or removes an existing submodule, or moves an existing submodule
interact with "submodule recurse"?
Also the same micronits (s/Incrementing/Increment/; s/Decremented/Decrement/).
Even if iterating over .gitmodules entries is a good idea, I suspect that
sed script is too fragile. Doesn't .gitmodules use the same format as git
configuration files, allowing spaces around values, value quoting and
trailing comments on the same line?
> +# This actually traverses a submodule; checks whether the its initialized
> +# or not, does nothing if not initialized.
s/the //;?
> +traverse_module()
> +{
> + # Will work in the submodule if and only if its initialized
> + initialize_sub_module "$1" &&
"initialize_sub_module" does not sound like a function that checks if it
is initialized, but more like a function to, eh, initialize the submodule.
Perhaps the function should be renamed to make it clearer that it is a
predicate?
> + (
> + submod_path="$1"
> + shift
> + cd "$submod_path"
> + # If depth-first is specified in that case submodules are
> + # are traversed before executing the command on this submodule
> + test -n "$depth_first" && traverse_submodules "$@"
> + # pwd is mentioned in order to enable the ser to distinguish
> + # between same name modules, e.g. a/lib and b/lib.
> + say "git submodule recurse $submod_path $*"
> + git "$@"
> + # if exit on error is specifed than script will exit if any
> + # command fails. As there is no transaction there will be
> + # no rollback either
s/than/then/;?
> + # TODO - If possible facilitate transaction
> + if test "$?" -ne 0 && test -n "$on_error"
> + then
> + die "FAILED: git submodule $submod_path $*"
Dying before doing further damage to the repository tree may be a good
idea, but I did not see the calling loop in traverse_submodules pay
attention to the exit code from here.
> + fi
> + # If depth-first is not specified in that case submodules are
> + # are traversed after executing the command on this submodule
> + test -z "$depth_first" && traverse_submodules "$@"
> + )
> +}
> +
> +# 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.
Is "git-clone" a good example to give here? What would that mean to
recurse into each submodule directories in a superproject to run "clone"?
> +cmd_recurse() {
> + while :
> + do
> + case "$1" in
> + -q|--quiet)
> + quiet=1
> + ;;
> + -d|--depth)
> + shift
> + if test -z "$1"
> + then
> + echo "No <recursion depth> specified"
> + usage
> + # Arithmatic operation will give an error if depth is not number
> + # thus chose to check intergerness with regular expression.
> + # $1 is underquoted becuase the expr is in quotation
> + elif test "$(expr $1 : '[1-9][0-9]*')" -eq "$(expr $1 : '.*')"
Huh?
$ a='1 2 3'
$ expr $a : '[1-9]'
expr: syntax error
$ expr "$a" : '[1-9]'
1
$ z=$(expr $a : '[1-9]')
expr: syntax error
$ z=$(expr "$a" : '[1-9]')
$ echo $z
1
$ echo "$(expr $a : '[1-9]')"
expr: syntax error
$ echo "$(expr "$a" : '[1-9]')"
1
If you want to make sure that $(( ... )) would not choke with given "$1",
you can check by attempting to do a simple $(( ... )) to see if it errors
out, which would be simpler.
if test -z "$1"
then
...
elif ! echo $(( "$1" + 0 )) >/dev/null
then
die "$1 is not an integer"
...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
