Re: [PATCH v2 09/10] generate-cmdlist.sh: replace "grep' invocation with a shell version

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 23, 2021, 22:19 UTC
Message-ID: <xmqqcznv9yfh.fsf@gitster.g>
In-Reply-To: <patch-v2-09.10-7903dd1f8c2-20211022T193027Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 39 quoted lines

> Replace the "grep" we run to exclude certain programs from the
> generated output with a pure-shell loop that strips out the comments,
> and sees if the "cmd" we're reading is on a list of excluded
> programs. This uses a trick similar to test_have_prereq() in
> test-lib-functions.sh.
>
> On my *nix system this makes things quite a bit slower compared to
> HEAD~, but since the generate-cmdlist.sh is already quite fast, and
> this likely helps systems where command invocations are more
> expensive (i.e. Windows) let's use this anyway.
>
>   'sh generate-cmdlist.sh.old command-list.txt' ran
>     1.56 ± 0.11 times faster than 'sh generate-cmdlist.sh command-list.txt'
>    18.00 ± 0.19 times faster than 'sh generate-cmdlist.sh.master command-list.txt'
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  generate-cmdlist.sh | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> index 9b7d6aea629..2b184bbc65f 100755
> --- a/generate-cmdlist.sh
> +++ b/generate-cmdlist.sh
> @@ -6,12 +6,27 @@ die () {
>  }
>  
>  command_list () {
> -	eval "grep -ve '^#' $exclude_programs" <"$1"
> +	while read cmd rest
> +	do
> +		case "$cmd" in
> +		"#"*)
> +			continue;
> +			;;
> +		*)
> +			case "$exclude_programs" in
> +				*":$cmd:"*)
> +				;;

Funny indentation.

Show 7 quoted lines

> +			*)
> +				echo "$cmd $rest"
> +				;;
> +			esac
> +		esac
> +	done
>  }

>  category_list () {
> -	command_list "$1" |
> +	command_list <"$1" |

This change is unnecessary if you did

	while read cmd rest
	do
		...
	done <"$1"

to keep the external interface to the command_list helper unchanged.

> -	cut -c 40- |
> +	cut -d' ' -f2- |

Is this just a subjective preference or a logical consequence of how the output from command_list looks like got somehow changed?

Show 6 quoted lines

> @@ -48,7 +63,7 @@ define_category_names () {
>  print_command_list () {
>  	echo "static struct cmdname_help command_list[] = {"
>  
> -	command_list "$1" |
> +	command_list <"$1" |

Ditto.

Show 9 quoted lines

> -exclude_programs=
> +exclude_programs=:
>  while test "--exclude-program" = "$1"
>  do
>  	shift
> -	exclude_programs="$exclude_programs -e \"^$1 \""
> +	exclude_programs="$exclude_programs$1:"
>  	shift
>  done