public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Patrick Steinhardt <ps@pks•im>
Cc: git@vger•kernel.org,  Johannes Schindelin <Johannes.Schindelin@gmx•de>
Subject: Re: [PATCH 4/4] Documentation: stop depending on Perl to generate command list
Date: Tue, 15 Apr 2025 09:32:33 -0700	[thread overview]
Message-ID: <xmqqmschmllq.fsf@gitster.g> (raw)
In-Reply-To: <20250415-b4-pks-drop-perl-v1-4-c6addf175858@pks.im> (Patrick Steinhardt's message of "Tue, 15 Apr 2025 11:57:11 +0200")

Patrick Steinhardt <ps@pks•im> writes:

> The "cmd-list.perl" script is used to extract the list of commands part
> of a specific category and extracts the description of each command from
> its respective manpage. The generated output is then included in git(1)
> to list all Git commands.
>
> The script is written in Perl. Refactor it to use shell scripting
> exclusively so that we can get rid of the mandatory dependency on Perl
> to build our documentation.
>
> The converted script is slower compared to its Perl implementation. But
> by being careful and not spawning external commands in `format_one ()`
> we can mitigate the performance hit to a reasonable level:
>
>     Benchmark 1: Perl
>       Time (mean ± σ):      10.3 ms ±   0.2 ms    [User: 7.0 ms, System: 3.3 ms]
>       Range (min … max):    10.0 ms …  11.1 ms    200 runs
>
>     Benchmark 2: Shell
>       Time (mean ± σ):      74.4 ms ±   0.4 ms    [User: 48.6 ms, System: 24.7 ms]
>       Range (min … max):    73.1 ms …  75.5 ms    200 runs
>
>     Summary
>       Perl ran
>         7.23 ± 0.13 times faster than Shell
>
> While a sevenfold slowdown is significant, the benefit of not requiring
> Perl for a fully-functioning Git installation outweighs waiting a couple
> of milliseconds longer during the build process.

I personally do not feel Perl such a drag but whether it is 10ms vs
75ms, as long as we won't run the script excessively (and either
meson or make should be set-up to avoid unnecessary work already), I
do not think a shell script being slightly slower than a Perl script
is a big deal.  Thanks for working on this.

> diff --git a/Documentation/cmd-list.sh b/Documentation/cmd-list.sh
> new file mode 100755
> index 00000000000..fa90781f3c7
> --- /dev/null
> +++ b/Documentation/cmd-list.sh
> @@ -0,0 +1,104 @@
> +#!/bin/sh
> +
> +set -e
> +
> +format_one () {
> +	source_dir="$1"
> +	command="$2"
> +	attributes="$3"
> +
> +	path="$source_dir/Documentation/$command.adoc"
> +	if ! test -f "$path"
> +	then
> +		echo >&2 "No such file $path"
> +		exit 1
> +	fi
> +
> +	state=0
> +	while read line
> +	do
> +		case "$state" in
> +			0)

Style. label and "case" and "esac" align, just like ...

> +				case "$line" in
> +				git*\(*\)|scalar*\(*\))
> +					mansection="${line##*\(}"

... this one.

> +					mansection="${mansection%\)}"
> +					;;
> +				NAME)
> +					state=1;;
> +				esac
> +				;;

> +	case "$description" in
> +		"$command - "*)

Likewise.

> +			text="${description#$command - }"
> +
> +			printf "linkgit:%s[%s]::\n\t" "$command" "$mansection"
> +			case "$attributes" in
> +				*" deprecated "*)

Ditto.

> +					printf "(deprecated) "
> +					;;
> +			esac
> +			printf "$text.\n\n"
> +			;;
> +		*)
> +			echo >&2 "Description does not match $command: $description"
> +			exit 1
> +			;;
> +	esac
> +}
> +
> +source_dir="$1"
> +build_dir="$2"
> +shift 2
> +
> +for out in "$@"

Let's omit 'in "$@"' when iterationg over "$@".

> +do
> +	category="${out#cmds-}"
> +	category="${category%.adoc}"
> +	path="$build_dir/$out"
> +
> +	while read command command_category attributes
> +	do
> +		case "$command" in
> +		"#"*)
> +			continue;;
> +		esac
> +
> +		case "$command_category" in
> +		"$category")
> +			format_one "$source_dir" "$command" " $attributes ";;
> +		esac
> +	done <"$source_dir/command-list.txt" >"$build_dir/$out+"
> +
> +	if cmp "$build_dir/$out+" "$build_dir/$out" >/dev/null 2>&1
> +	then
> +		rm "$build_dir/$out+"
> +	else
> +		mv "$build_dir/$out+" "$build_dir/$out"
> +	fi
> +done

OK.


  reply	other threads:[~2025-04-15 16:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-15  9:57 [PATCH 0/4] Drop Perl dependency in a couple of subsystems Patrick Steinhardt
2025-04-15  9:57 ` [PATCH 1/4] filter-branch: stop depending on Perl Patrick Steinhardt
2025-04-15 15:50   ` Junio C Hamano
2025-04-15  9:57 ` [PATCH 2/4] request-pull: " Patrick Steinhardt
2025-04-15 16:16   ` Junio C Hamano
2025-04-16 15:07     ` Patrick Steinhardt
2025-04-15  9:57 ` [PATCH 3/4] Documentation: stop depending on Perl to massage user manual Patrick Steinhardt
2025-04-15  9:57 ` [PATCH 4/4] Documentation: stop depending on Perl to generate command list Patrick Steinhardt
2025-04-15 16:32   ` Junio C Hamano [this message]
2025-04-16 12:16 ` [PATCH v2 0/4] Drop Perl dependency in a couple of subsystems Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 1/4] filter-branch: stop depending on Perl Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 2/4] request-pull: " Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 3/4] Documentation: stop depending on Perl to massage user manual Patrick Steinhardt
2025-04-16 12:16   ` [PATCH v2 4/4] Documentation: stop depending on Perl to generate command list Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmschmllq.fsf@gitster.g \
    --to=gitster@pobox$(echo .)com \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=ps@pks$(echo .)im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox