Skip to content

Commit 4791fc0

Browse files
peffdscho
authored andcommitted
send-email: avoid creating more than one Term::ReadLine object
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: hirooih/perl-trg#16 Backported from c016726 (send-email: avoid creating more than one Term::ReadLine object, 2023-08-08). Signed-off-by: Jeff King <peff@peff.net> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
1 parent ad2a135 commit 4791fc0

File tree

2 files changed

+16
-7
lines changed

2 files changed

+16
-7
lines changed

git-send-email.perl

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -917,11 +917,19 @@ sub get_patch_subject {
917917
do_edit(@files);
918918
}
919919
920-
sub term {
921-
require Term::ReadLine;
922-
return $ENV{"GIT_SEND_EMAIL_NOTTY"}
923-
? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT)
924-
: Term::ReadLine->new('git-send-email');
920+
{
921+
# Only instantiate one $term per program run, since some
922+
# Term::ReadLine providers refuse to create a second instance.
923+
my $term;
924+
sub term {
925+
require Term::ReadLine;
926+
if (!defined $term) {
927+
$term = $ENV{"GIT_SEND_EMAIL_NOTTY"}
928+
? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT)
929+
: Term::ReadLine->new('git-send-email');
930+
}
931+
return $term;
932+
}
925933
}
926934
927935
sub ask {

t/t9001-send-email.sh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,13 +337,14 @@ test_expect_success $PREREQ 'Show all headers' '
337337
test_expect_success $PREREQ 'Prompting works' '
338338
clean_fake_sendmail &&
339339
(echo "to@example.com" &&
340-
echo ""
340+
echo "my-message-id@example.com"
341341
) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
342342
--smtp-server="$(pwd)/fake.sendmail" \
343343
$patches \
344344
2>errors &&
345345
grep "^From: A U Thor <author@example.com>\$" msgtxt1 &&
346-
grep "^To: to@example.com\$" msgtxt1
346+
grep "^To: to@example.com\$" msgtxt1 &&
347+
grep "^In-Reply-To: <my-message-id@example.com>" msgtxt1
347348
'
348349

349350
test_expect_success $PREREQ,AUTOIDENT 'implicit ident is allowed' '

0 commit comments

Comments
 (0)