Re: [PATCH 2/6] git-submodule.sh: rename unused variables

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 12, 2018, 21:24 UTC
Message-ID: <xmqq601kuozm.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180712194754.71979-3-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> The 'mode' variable is not used in cmd_update for its original purpose,
> rename it to 'dummy' as it only serves the purpose to abort quickly
> documenting this knowledge.

It seems that

 (1) the while loop in git-submodule.sh we see in this patch is the
     only thing that read from submodule--helper update-clone; and

 (2) the mode/sha1/stage output from prepare_to_clone_next_submodule()
     is shown only when update_clone_get_next_task which in turn is
     run only during update-clone task

so this conversion will not have unintended bad effect on other codepaths that use similarly formatted (but already different) output used by the users of module_list.

Which tells us that this step is safe.

I am not sure how much it buys us not having to format mode into hex and not having to call ce_stage(), when we have cache entry anyway in the codeflow, though. IOW, it is unclear at this second step in the six-patch series if we get to the end of the tunnel sooner by having this step here. Let's keep reading ;-).

Show 46 quoted lines

> The variable 'stage' is also not used any more in cmd_update, so remove it.
>
> This went unnoticed as first each function used the commonly used
> submodule listing, which was converted in 74703a1e4df (submodule: rewrite
> `module_list` shell function in C, 2015-09-02). When cmd_update was
> using its own function starting in 48308681b07 (git submodule update:
> have a dedicated helper for cloning, 2016-02-29), its removal was missed.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  builtin/submodule--helper.c | 5 ++---
>  git-submodule.sh            | 4 ++--
>  2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 20ae9191ca3..ebcfe85bfa9 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1571,9 +1571,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>  	needs_cloning = !file_exists(sb.buf);
>  
>  	strbuf_reset(&sb);
> -	strbuf_addf(&sb, "%06o %s %d %d\t%s\n", ce->ce_mode,
> -			oid_to_hex(&ce->oid), ce_stage(ce),
> -			needs_cloning, ce->name);
> +	strbuf_addf(&sb, "dummy %s %d\t%s\n",
> +		    oid_to_hex(&ce->oid), needs_cloning, ce->name);
>  	string_list_append(&suc->projectlines, sb.buf);
>  
>  	if (!needs_cloning)
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 8a611865397..56588aa304d 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -531,9 +531,9 @@ cmd_update()
>  		"$@" || echo "#unmatched" $?
>  	} | {
>  	err=
> -	while read -r mode sha1 stage just_cloned sm_path
> +	while read -r quickabort sha1 just_cloned sm_path
>  	do
> -		die_if_unmatched "$mode" "$sha1"
> +		die_if_unmatched "$quickabort" "$sha1"
>  
>  		name=$(git submodule--helper name "$sm_path") || exit
>  		if ! test -z "$update"