Re: [[PATCH v2]] Fix bug when more than one readline instance is used

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 10, 2023, 14:31 UTC
Message-ID: <xmqqmsyzhsto.fsf@gitster.g>
In-Reply-To: <20230810011831.1423208-1-wesleys@opperschaap.net>

Wesley Schwengle <wesleys@opperschaap.net> writes:

> Subject: Re: [[PATCH v2]] Fix bug when more than one readline instance is used

Thanks. Again, our convention is to make sure that, even only with the title, readers would know what the commit is about. The above does not even hint which part of the system the bug was about. By stealing from what Peff already has done, we can call this

    Subject: [PATCH v2] git-svn: avoid creating more than one Term::ReadLine object

to mimic c016726c (send-email: avoid creating more than one Term::ReadLine object, 2023-08-08). Also, please do not double the [brackets] around the "PATCH".

> A followup[^1] for git-svn.perl on d42e4ca9f8 where this bug was solved
> for git-send-email.perl
>
> [^1]: https://lore.kernel.org/git/20230810004956.GA816605@coredump.intra.peff.net/T/#t

Once a commit is in 'next', its commit object name will generally be stable, hence, taken as a whole, something like:

    git-svn: avoid creating more than one than one Term::ReadLine object

    Newer (v1.46) Term::ReadLine::Gnu would not like us to ask it to
    create multiple readline instances.  c016726c (send-email: avoid
    creating more than one Term::ReadLine object, 2023-08-08)
    adjusted git-send-email to this change.  Make the same
    adjustment to git-svn.

    While at it, drop the same FakeTerm hack, just like dfd46bae
    (send-email: drop FakeTerm hack, 2023-08-08) did, for exactly
    the same reason.

I'll queue the patch with the above commit log message for tonight, so unless you have improvements over it, there is no need to resend.

Thanks.

Show 45 quoted lines

> Signed-off-by: Wesley Schwengle <wesleys@opperschaap.net>
> ---
>  git-svn.perl | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index be987e316f..93f6538d61 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -297,27 +297,18 @@ sub _req_svn {
>  		{} ],
>  );
>  
> -package FakeTerm;
> -sub new {
> -	my ($class, $reason) = @_;
> -	return bless \$reason, shift;
> -}
> -sub readline {
> -	my $self = shift;
> -	die "Cannot use readline on FakeTerm: $$self";
> -}
>  package main;
>  
> -my $term;
> -sub term_init {
> -	$term = eval {
> +{
> +	my $term;
> +	sub term_init {
> +		return $term if $term;
>  		require Term::ReadLine;
> -		$ENV{"GIT_SVN_NOTTY"}
> -			? new Term::ReadLine 'git-svn', \*STDIN, \*STDOUT
> -			: new Term::ReadLine 'git-svn';
> -	};
> -	if ($@) {
> -		$term = new FakeTerm "$@: going non-interactive";
> +		$term = $ENV{"GIT_SVN_NOTTY"}
> +				? new Term::ReadLine 'git-svn', \*STDIN, \*STDOUT
> +				: new Term::ReadLine 'git-svn';
> +		};
> +		return $term;
>  	}
>  }