Re: [PATCH v3 06/20] t: introduce PERL_TEST_HELPERS prerequisite

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 1, 2025, 18:26 UTC
Message-ID: <ee5e082f-6ab5-b996-9a0d-f7e5192c01be@gmx.de>
In-Reply-To: <20250327-b4-pks-t-perlless-v3-6-b436de9da1b8@pks.im>

Hi Patrick,

On Thu, 27 Mar 2025, Patrick Steinhardt wrote:

Show 11 quoted lines

> In the early days of Git, Perl was used quite prominently throughout the
> project. This has changed significantly as almost all of the executables
> we ship nowadays have eventually been rewritten in C. Only a handful of
> subsystems remain that require Perl:
>
>   - gitweb, a read-only web interface.
>
>   - A couple of scripts that allow importing repositories from GNU Arch,
>     CVS and Subversion.
>
>   - git-send-email(1), which can be used to send mails.

There is also `git request-pull` which is a _shell_ script that runs `perl` to parse the output of `ls-remote`, and there is `git filter-branch` (which was apparently not yet dropped?) that uses Perl if the `--state-branch` option is in use.

Show 33 quoted lines

>   - Our Perl bindings for Git.
>
>   - The netrc Git credential helper.
>
> None of these subsystems can really be considered to be part of the
> "core" of Git, and an installation without them is fully functional.
> It is more likely than not that an end user wouldn't even notice that
> any features are missing if those tools weren't installed. But while
> Perl nowadays very much is an optional dependency of Git, there is a
> significant limitation when Perl isn't available: developers cannot run
> our test suite.
>
> Preceding commits have started to lift this restriction by removing the
> strict dependency on Perl in many central parts of the test library. But
> there are still many tests that rely on small Perl helpers to do various
> different things.
>
> Introduce a new PERL_TEST_HELPERS prerequisite that guards all tests
> that require Perl. This prerequisite is explicitly different than the
> preexisting PERL prerequisite:
>
>   - PERL records whether or not features depending on the Perl
>     interpreter are built.
>
>   - PERL_TEST_HELPERS records whether or not a Perl interpreter is
>     available for our tests.
>
> By having these two separate prerequisites we can thus distinguish
> between tests that inherently depend on Perl because the underlying
> feature does, and those tests that depend on Perl because the test
> itself is using Perl.
>
> Adapt all tests to set the PERL_TEST_HELPERS prerequisite as needed.

The patch looks good, in particular when fetching the `b4/pks-t-perlless` branch from https://gitlab.com/gitlab-org/git and inspecting 8fc639f99d9f manually, as it is a rather large patch that is pretty much unreviewable on a mailing list.

Using several write-only `sed` invocations, I identified that there are only three hunks that are neither adding a stand-alone `PERL_TEST_HELPERS` prereq nor adding a test preamble of this form:

	if ! test_have_prereq PERL_TEST_HELPERS
	then
		skip_all='skipping <something>; Perl not available'
		test_done
	fi

Skipping to only the affected files, these three instances are:

Show 13 quoted lines

> diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
> index c91a62b77af..342d0423c92 100755
> --- a/t/t5534-push-signed.sh
> +++ b/t/t5534-push-signed.sh
> @@ -177,7 +177,7 @@ test_expect_success GPGSSH 'ssh signed push sends push certificate' '
>  	test_cmp expect dst/push-cert-status
>  '
>
> -test_expect_success GPG 'inconsistent push options in signed push not allowed' '
> +test_expect_success GPG,PERL_TEST_HELPERS 'inconsistent push options in signed push not allowed' '
>  	# First, invoke receive-pack with dummy input to obtain its preamble.
>  	prepare_dst &&
>  	git -C dst config receive.certnonceseed sekrit &&

Here, that prereq is appended.

Show 22 quoted lines

> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index d0c18660e33..d743d986c40 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -649,7 +649,7 @@ test_expect_success 'GIT_TRACE_PACKFILE produces a usable pack' '
>  	git -C replay.git index-pack -v --stdin <tmp.pack
>  '
>
> -test_expect_success 'clone on case-insensitive fs' '
> +test_expect_success PERL_TEST_HELPERS 'clone on case-insensitive fs' '
>  	git init icasefs &&
>  	(
>  		cd icasefs &&
> @@ -662,7 +662,7 @@ test_expect_success 'clone on case-insensitive fs' '
>  	)
>  '
>
> -test_expect_success CASE_INSENSITIVE_FS 'colliding file detection' '
> +test_expect_success PERL_TEST_HELPERS,CASE_INSENSITIVE_FS 'colliding file detection' '
>  	grep X icasefs/warning &&
>  	grep x icasefs/warning &&
>  	test_grep "the following paths have collided" icasefs/warning

Here, too.

Show 12 quoted lines

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index a62699d6c79..59162a3c834 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1706,6 +1706,7 @@ test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2
>  test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
>  test -n "$SANITIZE_LEAK" && test_set_prereq SANITIZE_LEAK
>  test -n "$GIT_VALGRIND_ENABLED" && test_set_prereq VALGRIND
> +test -n "$PERL_PATH" && test_set_prereq PERL_TEST_HELPERS
>
>  if test -z "$GIT_TEST_CHECK_CACHE_TREE"
>  then

And this is obviously correct, too.

Thank you, Johannes