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 2/4] request-pull: stop depending on Perl
Date: Tue, 15 Apr 2025 09:16:05 -0700 [thread overview]
Message-ID: <xmqqtt6pmmd6.fsf@gitster.g> (raw)
In-Reply-To: <20250415-b4-pks-drop-perl-v1-2-c6addf175858@pks.im> (Patrick Steinhardt's message of "Tue, 15 Apr 2025 11:57:09 +0200")
Patrick Steinhardt <ps@pks•im> writes:
> While git-request-pull(1) is written as a shell script, for it to
> function we depend on Perl being available. The script gets installed
> unconditionally though, regardless of whether or not Perl is even
> available on the system. When it's not available, the `@PERL_PATH@`
> variable may be substituted with a nonexistent executable path and thus
> cause the script to fail.
>
> Refactor the script so that it does not depend on Perl at all anymore.
>
> Signed-off-by: Patrick Steinhardt <ps@pks•im>
> ---
> git-request-pull.sh | 74 ++++++++++++++++++++++++++-----------------------
> t/t5150-request-pull.sh | 6 ----
> 2 files changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index 775ba8ea11a..59276fe265b 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -78,41 +78,47 @@ fi
> merge_base=$(git merge-base $baserev $headrev) ||
> die "fatal: No commits in common between $base and $head"
>
> -# $head is the refname from the command line.
> -# Find a ref with the same name as $head that exists at the remote
> +find_matching_ref () {
> + while read sha1 ref
> + do
> + case "$ref" in
> + *"^{}")
> + ref="${ref%"^{}"}"
> + deref=true
This got a bit tighter (the original allowed ls-remote output to be
later extended to throw "^something-else" at the end of line), which
I do not know is something we need to worry about. I think retaining
the original semantics is easy in this case, e.g.,
*"^"?*)
ref="${ref%"^*"}"
or something, if we wanted to. As this is meant to be faithful
rewrite to lose Perl, not a bugfix to correct overly loose pattern
matching in the original, I think we want to in this case.
> + ;;
> + *)
> + deref=
> + ;;
> + esac
> +
> + if test "$sha1" = "${remote:-HEAD}"
> + then
> + echo "$sha1 $sha1"
> + break
> + fi
> +
> + case "$ref" in
> + "${remote:-HEAD}"|*"/${remote:-HEAD}")
> + if test -z "$deref"
> + then
> + # Remember the matching unpeeled object on the
> + # remote side.
> + remote_sha1="$sha1"
> + fi
> +
> + if test "$sha1" = "$headrev"
> + then
> + echo "${remote_sha1:-$headrev} $ref"
> + break
> + fi
> + ;;
> + esac
> + done
> +}
> +
> +# Find a ref with the same name as $remote that exists at the remote
> # and points to the same commit as the local object.
> ...
> -set fnord $(git ls-remote "$url" | @PERL_PATH@ -e "$find_matching_ref" "${remote:-HEAD}" "$headrev")
> +set fnord $(git ls-remote "$url" | find_matching_ref)
> remote_sha1=$2
> ref=$3
This is faithful to the original, and having find_matching_ref in
the process substitution means we do not have to worry about
localizing variables in there. Good.
next prev parent reply other threads:[~2025-04-15 16:16 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 [this message]
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
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=xmqqtt6pmmd6.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