| Message ID | 20250415-b4-pks-drop-perl-v1-2-c6addf175858@pks.im (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Drop Perl dependency in a couple of subsystems | expand |
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.
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 + ;; + *) + 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. -find_matching_ref=' - my ($head,$headrev) = (@ARGV); - my $pattern = qr{/\Q$head\E$}; - my ($remote_sha1, $found); - - while (<STDIN>) { - chomp; - my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/; - - if ($sha1 eq $head) { - $found = $remote_sha1 = $sha1; - break; - } - - if ($ref eq $head || $ref =~ $pattern) { - if ($deref eq "") { - # Remember the matching object on the remote side - $remote_sha1 = $sha1; - } - if ($sha1 eq $headrev) { - $found = $ref; - break; - } - } - } - if ($found) { - $remote_sha1 = $headrev if ! defined $remote_sha1; - print "$remote_sha1 $found\n"; - } -' - -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 diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh index cb67bac1c47..270ce6ea487 100755 --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -7,12 +7,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh -if ! test_have_prereq PERL -then - skip_all='skipping request-pull tests, perl not available' - test_done -fi - test_expect_success 'setup' ' git init --bare upstream.git &&
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(-)