| Message ID | 20240521195659.870714-3-gitster@pobox.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix various overly aggressive protections in 2.45.1 and friends | expand |
On 2024-05-21 21:56, Junio C Hamano wrote: > From: Jeff King <peff@peff.net> > > Every time git-send-email calls its ask() function to prompt the user, > we call term(), which instantiates a new Term::ReadLine object. But in > v1.46 of Term::ReadLine::Gnu (which provides the Term::ReadLine > interface on some platforms), its constructor refuses to create a > second > instance[1]. So on systems with that version of the module, most > git-send-email instances will fail (as we usually prompt for both "to" > and "in-reply-to" unless the user provided them on the command line). > > We can fix this by keeping a single instance variable and returning it > for each call to term(). In perl 5.10 and up, we could do that with a > "state" variable. But since we only require 5.008, we'll do it the > old-fashioned way, with a lexical "my" in its own scope. > > Note that the tests in t9001 detect this problem as-is, since the > failure mode is for the program to die. But let's also beef up the > "Prompting works" test to check that it correctly handles multiple > inputs (if we had chosen to keep our FakeTerm hack in the previous > commit, then the failure mode would be incorrectly ignoring prompts > after the first). > > [1] For discussion of why multiple instances are forbidden, see: > https://github.com/hirooih/perl-trg/issues/16 > > [jc: cherry-picked from v2.42.0-rc2~6^2] > > Signed-off-by: Jeff King <peff@peff.net> > Acked-by: Taylor Blau <me@ttaylorr.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> Looking good to me. Thanks for taking care of this issue. Reviewed-by: Dragan Simic <dsimic@manjaro.org> > --- > git-send-email.perl | 18 +++++++++++++----- > t/t9001-send-email.sh | 5 +++-- > 2 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index 72d876f0a0..ad51508790 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -917,11 +917,19 @@ sub get_patch_subject { > do_edit(@files); > } > > -sub term { > - require Term::ReadLine; > - return $ENV{"GIT_SEND_EMAIL_NOTTY"} > - ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) > - : Term::ReadLine->new('git-send-email'); > +{ > + # Only instantiate one $term per program run, since some > + # Term::ReadLine providers refuse to create a second instance. > + my $term; > + sub term { > + require Term::ReadLine; > + if (!defined $term) { > + $term = $ENV{"GIT_SEND_EMAIL_NOTTY"} > + ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) > + : Term::ReadLine->new('git-send-email'); > + } > + return $term; > + } > } > > sub ask { > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index 1130ef21b3..0f08a9542b 100755 > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -337,13 +337,14 @@ test_expect_success $PREREQ 'Show all headers' ' > test_expect_success $PREREQ 'Prompting works' ' > clean_fake_sendmail && > (echo "to@example.com" && > - echo "" > + echo "my-message-id@example.com" > ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ > --smtp-server="$(pwd)/fake.sendmail" \ > $patches \ > 2>errors && > grep "^From: A U Thor <author@example.com>\$" msgtxt1 && > - grep "^To: to@example.com\$" msgtxt1 > + grep "^To: to@example.com\$" msgtxt1 && > + grep "^In-Reply-To: <my-message-id@example.com>" msgtxt1 > ' > > test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '
diff --git a/git-send-email.perl b/git-send-email.perl index 72d876f0a0..ad51508790 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -917,11 +917,19 @@ sub get_patch_subject { do_edit(@files); } -sub term { - require Term::ReadLine; - return $ENV{"GIT_SEND_EMAIL_NOTTY"} - ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) - : Term::ReadLine->new('git-send-email'); +{ + # Only instantiate one $term per program run, since some + # Term::ReadLine providers refuse to create a second instance. + my $term; + sub term { + require Term::ReadLine; + if (!defined $term) { + $term = $ENV{"GIT_SEND_EMAIL_NOTTY"} + ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) + : Term::ReadLine->new('git-send-email'); + } + return $term; + } } sub ask { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 1130ef21b3..0f08a9542b 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -337,13 +337,14 @@ test_expect_success $PREREQ 'Show all headers' ' test_expect_success $PREREQ 'Prompting works' ' clean_fake_sendmail && (echo "to@example.com" && - echo "" + echo "my-message-id@example.com" ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ --smtp-server="$(pwd)/fake.sendmail" \ $patches \ 2>errors && grep "^From: A U Thor <author@example.com>\$" msgtxt1 && - grep "^To: to@example.com\$" msgtxt1 + grep "^To: to@example.com\$" msgtxt1 && + grep "^In-Reply-To: <my-message-id@example.com>" msgtxt1 ' test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '