| Message ID | 20240521195659.870714-2-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> > > Back in 280242d1cc (send-email: do not barf when Term::ReadLine does > not > like your terminal, 2006-07-02), we added a fallback for when > Term::ReadLine's constructor failed: we'd have a FakeTerm object > instead, which would then die if anybody actually tried to call > readline() on it. Since we instantiated the $term variable at program > startup, we needed this workaround to let the program run in modes when > we did not prompt the user. > > But later, in f4dc9432fd (send-email: lazily load modules for a big > speedup, 2021-05-28), we started loading Term::ReadLine lazily only > when > ask() is called. So at that point we know we're trying to prompt the > user, and we can just die if ReadLine instantiation fails, rather than > making this fake object to lazily delay showing the error. > > This should be OK even if there is no tty (e.g., we're in a cron job), > because Term::ReadLine will return a stub object in that case whose > "IN" > and "OUT" functions return undef. And since 5906f54e47 (send-email: > don't attempt to prompt if tty is closed, 2009-03-31), we check for > that > case and skip prompting. > > And we can be sure that FakeTerm was not kicking in for such a > situation, because it has actually been broken since that commit! It > does not define "IN" or "OUT" methods, so perl would barf with an > error. > If FakeTerm was in use, we were neither honoring what 5906f54e47 tried > to do, nor producing the readable message that 280242d1cc intended. > > So we're better off just dropping FakeTerm entirely, and letting the > error reported by constructing Term::ReadLine through. > > [jc: cherry-picked from v2.42.0-rc2~6^2~1] > > 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 | 22 ++-------------------- > 1 file changed, 2 insertions(+), 20 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index 5861e99a6e..72d876f0a0 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -26,18 +26,6 @@ > > Getopt::Long::Configure qw/ pass_through /; > > -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; > - > - > sub usage { > print <<EOT; > git send-email' [<options>] <file|directory> > @@ -930,16 +918,10 @@ sub get_patch_subject { > } > > sub term { > - my $term = eval { > - require Term::ReadLine; > - $ENV{"GIT_SEND_EMAIL_NOTTY"} > + require Term::ReadLine; > + return $ENV{"GIT_SEND_EMAIL_NOTTY"} > ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) > : Term::ReadLine->new('git-send-email'); > - }; > - if ($@) { > - $term = FakeTerm->new("$@: going non-interactive"); > - } > - return $term; > } > > sub ask {
diff --git a/git-send-email.perl b/git-send-email.perl index 5861e99a6e..72d876f0a0 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -26,18 +26,6 @@ Getopt::Long::Configure qw/ pass_through /; -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; - - sub usage { print <<EOT; git send-email' [<options>] <file|directory> @@ -930,16 +918,10 @@ sub get_patch_subject { } sub term { - my $term = eval { - require Term::ReadLine; - $ENV{"GIT_SEND_EMAIL_NOTTY"} + require Term::ReadLine; + return $ENV{"GIT_SEND_EMAIL_NOTTY"} ? Term::ReadLine->new('git-send-email', \*STDIN, \*STDOUT) : Term::ReadLine->new('git-send-email'); - }; - if ($@) { - $term = FakeTerm->new("$@: going non-interactive"); - } - return $term; } sub ask {