[2/3] t3701: test the built-in `add -i` regardless of NO_PERL

Commit Message

Johannes Schindelin Aug. 30, 2022, 1:54 p.m. UTC

From: Johannes Schindelin <[e-mail]>

The built-in `git add --interactive` does not require Perl, therefore we
can safely run these tests even when building with `NO_PERL=LetsDoThat`.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 t/t3701-add-interactive.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jeff King Aug. 30, 2022, 6:54 p.m. UTC | #1

On Tue, Aug 30, 2022 at 01:54:23PM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <[e-mail]>
> 
> The built-in `git add --interactive` does not require Perl, therefore we
> can safely run these tests even when building with `NO_PERL=LetsDoThat`.

Make sense. The patch is small enough that it is certainly worth doing
in the meantime, but is it time to start thinking about dropping the
perl implementation (and hence this prereq) entirely?

-Peff

Junio C Hamano Aug. 30, 2022, 7:09 p.m. UTC | #2

"Johannes Schindelin via GitGitGadget" <[e-mail]>
writes:

> From: Johannes Schindelin <[e-mail]>
>
> The built-in `git add --interactive` does not require Perl, therefore we
> can safely run these tests even when building with `NO_PERL=LetsDoThat`.
>
> Signed-off-by: Johannes Schindelin <[e-mail]>
> ---
>  t/t3701-add-interactive.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

A companion to the previous step that is obvious and very much to
the point.  Looking good.

> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index b354fb39de8..8d16cd45821 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -7,9 +7,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-terminal.sh
>  
> -if ! test_have_prereq PERL
> +if test_have_prereq !ADD_I_USE_BUILTIN,!PERL
>  then
> -	skip_all='skipping add -i tests, perl not available'
> +	skip_all='skipping add -i (scripted) tests, perl not available'
>  	test_done
>  fi

Johannes Schindelin Aug. 30, 2022, 9:03 p.m. UTC | #3

Hi Peff,

On Tue, 30 Aug 2022, Jeff King wrote:

> On Tue, Aug 30, 2022 at 01:54:23PM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <[e-mail]>
> >
> > The built-in `git add --interactive` does not require Perl, therefore we
> > can safely run these tests even when building with `NO_PERL=LetsDoThat`.
>
> Make sense. The patch is small enough that it is certainly worth doing
> in the meantime, but is it time to start thinking about dropping the
> perl implementation (and hence this prereq) entirely?

A couple of months ago, I would have said the same (and you seemed to
suggest something similar in [*1*], too), but since we flipped the default
to the built-in version in 0527ccb1b55 (add -i: default to the built-in
implementation, 2021-11-30), we discovered the need for several fixes:

- pw/add-p-hunk-split-fix
- js/add-i-delete
- js/add-p-diff-parsing-fix

Therefore I consider it premature to drop `git-add--interactive.perl` just
yet.

Ciao,
Dscho

Footnote *1*:
https://lore.kernel.org/git/[e-mail]/

Patch

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index b354fb39de8..8d16cd45821 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -7,9 +7,9 @@  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
-if ! test_have_prereq PERL
+if test_have_prereq !ADD_I_USE_BUILTIN,!PERL
 then
-	skip_all='skipping add -i tests, perl not available'
+	skip_all='skipping add -i (scripted) tests, perl not available'
 	test_done
 fi