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

From: Jens Lehmann <jens.lehmann@web.de>
Date: Oct 10, 2011, 19:34 UTC
Message-ID: <4E9348A8.5000500@web.de>
In-Reply-To: <1317978295-4796-2-git-send-email-rctay89@gmail.com>

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?

Show 18 quoted lines

> 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)