Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()

[Jens Lehmann <Jens.Lehmann@web.de>]

BTW: this patch applies to next

Am 07.10.2011 11:04, schrieb Tay Ray Chuan:
> The die() message that may occur in module_name() is not really relevant
> to the user when called from module_clone(); the latter handles the
> "failure" (no submodule mapping) anyway.

Makes tons of sense, especially as adding a new submodule currently always
spews out the "No submodule mapping found in .gitmodules for path 'sub'"
message right before that mapping is added there. Thanks for noticing that
and ACK on that change from my side.

> Leave other callers of module_name() unchanged, as the die() message
> shown is either relevant for user consumption (such as those that exit()
> when the call fails), or will not occur at all (when called with paths
> returned by module_list()).

Hmm, while I agree on the first reasoning I'm not sure about the second.
module_list() asks the index for the submodule paths while module_name()
gets it's input from .gitmodules, so they can (and sometimes will)
disagree. When cmd_foreach() passes an empty "name" variable to the
spawned command that might still work (and even make sense), but using the
empty name in cmd_sync() to access the config is looking like an error to
me. It might make sense to add an "|| exit" at least to the callsite in
cmd_sync(). Or am I missing something here?

> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>  git-submodule.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index ebea35b..3adab93 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -130,7 +130,7 @@ module_clone()
>  
>  	gitdir=
>  	gitdir_base=
> -	name=$(module_name "$path")
> +	name=$(module_name "$path" 2>/dev/null)
>  	base_path=$(dirname "$path")
>  
>  	gitdir=$(git rev-parse --git-dir)