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

From: Jeff King <peff@peff.net>
Date: Aug 11, 2023, 14:51 UTC
Message-ID: <20230811145121.GB2303200@coredump.intra.peff.net>
In-Reply-To: <xmqqwmy2no2e.fsf@gitster.g>

On Thu, Aug 10, 2023 at 10:30:17PM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> And embarrassingly, the above is not sufficient, as the way $term is
> used in git-send-email and git-svn are subtly different.
> 
> I think we further need something like this on top, but my Perl is
> rusty.
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index e919c3f172..6033b97a0c 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -427,7 +427,7 @@ sub ask {
>  	my $default = $arg{default};
>  	my $resp;
>  	my $i = 0;
> -	term_init() unless $term;
> +	my $term = term_init();
>  
>  	if ( !( defined($term->IN)
>              && defined( fileno($term->IN) )

Hmm. Isn't that an indication that git-svn is OK as-is?

Looking at the version of git-svn.perl on the tip of master, I see we declare a global $term along with the initializer:

  my $term;
  sub term_init {
          $term = eval { ...etc... }

And then later in ask we call term_init() only if it's uninitialized:

  sub ask {
          ...
          term_init() unless $term;

So those are looking at the same $term, and the result should only be initialized once.

It could still benefit from cleaning up FakeTerm, since we lazily init the object since 30d45f798d (git-svn: delay term initialization, 2014-09-14). But I don't think there's a visible bug here with the new version of Term::ReadLine::Gnu.

-Peff