Control: forwarded -1 https://rt.perl.org/Ticket/Display.html?id=128020
On Thu, Apr 28, 2016 at 08:17:09PM +0100, Dominic Hargreaves wrote:
> On Thu, Apr 28, 2016 at 07:05:40PM +0300, Niko Tyni wrote:
> > On Tue, Apr 26, 2016 at 09:55:38PM +0300, Niko Tyni wrote:
> >
> > > A better fix would be be to wrap everything, but I don't have a good patch
> > > for that. Hacking build_complete_message() could work I guess, but it's
> > > not used by all the sending backends (at least _send_message_mailsend();
> > > not sure we need to care about that.)
> > >
> > > I think I need to look at this a bit more.
> >
> > Revised set of two patches attached. Eyeballs would be welcome.
>
> This looks sane to me. Thanks for taking the time to refactor!
Thank you for the review. I had to make one more modification to avoid
Text::Wrap eating empty lines from the body. Attached is the version I
sent upstream in [perl #128020].
--
Niko
>From 64986731cebb7768237489e00a15f93be660751f Mon Sep 17 00:00:00 2001
From: Niko Tyni <nt...@debian.org>
Date: Thu, 28 Apr 2016 18:50:17 +0300
Subject: [PATCH 1/2] perlbug: Refactor duplicated file reading code
_send_message_mailsend() needs to build the message itself rather than
calling build_complete_message() like the other backends, but they can
still share the file reading code, and so can the 'display report' part.
---
utils/perlbug.PL | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 885785a..6d6c780 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -834,10 +834,7 @@ EOF
if ( SaveMessage() ) { exit }
} elsif ($action =~ /^(d|l|sh)/i ) { # <D>isplay, <L>ist, <Sh>ow
# Display the message
- open(REP, '<:raw', $filename) or die "Couldn't open file '$filename': $!\n";
- binmode(REP, ':raw :crlf') if $Is_MSWin32;
- while (<REP>) { print $_ }
- close(REP) or die "Error closing report file '$filename': $!";
+ print _read_report($filename);
if ($have_attachment) {
print "\n\n---\nAttachment(s):\n";
for my $att (split /\s*,\s*/, $attachments) { print " $att\n"; }
@@ -1080,13 +1077,20 @@ ATTACHMENT
return $attach;
}
+sub _read_report {
+ my $fname = shift;
+ my $content;
+ open( REP, "<:raw", $fname ) or die "Couldn't open file '$fname': $!\n";
+ binmode(REP, ':raw :crlf') if $Is_MSWin32;
+ while (<REP>) { $content .= $_; }
+ close(REP) or die "Error closing report file '$fname': $!";
+ return $content;
+}
+
sub build_complete_message {
my $content = _build_header(%{_message_headers()}) . "\n\n";
$content .= _add_body_start() if $have_attachment;
- open( REP, "<:raw", $filename ) or die "Couldn't open file '$filename': $!\n";
- binmode(REP, ':raw :crlf') if $Is_MSWin32;
- while (<REP>) { $content .= $_; }
- close(REP) or die "Error closing report file '$filename': $!";
+ $content .= _read_report($filename);
$content .= _add_attachments() if $have_attachment;
return $content;
}
@@ -1137,10 +1141,7 @@ sub _send_message_mailsend {
$fh = $msg->open;
binmode($fh, ':raw');
print $fh _add_body_start() if $have_attachment;
- open(REP, "<:raw", $filename) or die "Couldn't open '$filename': $!\n";
- binmode(REP, ':raw :crlf') if $Is_MSWin32;
- while (<REP>) { print $fh $_ }
- close(REP) or die "Error closing $filename: $!";
+ print $fh _read_report($filename);
print $fh _add_attachments() if $have_attachment;
$fh->close or die "Error sending mail: $!";
--
2.8.0.rc3
>From 4bfb47f4d3a237934e9abe181e04c405c928067c Mon Sep 17 00:00:00 2001
From: Niko Tyni <nt...@debian.org>
Date: Mon, 25 Apr 2016 16:31:00 +0300
Subject: [PATCH 2/2] Make perlbug wrap overly long lines
Mail transport agents limit the length of message lines at SMTP time.
One observed limit is 1000 characters per line. Mail user agents typically
work around these limits by MIME-encoding the message. Since perlbug
doesn't do that, it needs to limit the length of its lines manually to
make sure bug reports get delivered.
The longest lines in perlbug reports normally come from Config::myconfig
output, particularly 'config_args', which has been observed to exceed
1000 characters on some configurations, causing report rejection. While
less likely, the list of local patches is another potential source of
overly long lines.
Use Text::Wrap (if available) to wrap the body of the report at an
arbitrarily chosen and hopefully safe limit of 900 characters. No
indentation or continuation line markers are added, though it would
be easy to add those if desired. Attachments and mail headers are not
wrapped.
Bug-Debian: https://bugs.debian.org/822463
---
utils/perlbug.PL | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/utils/perlbug.PL b/utils/perlbug.PL
index 6d6c780..6bc5bad 100644
--- a/utils/perlbug.PL
+++ b/utils/perlbug.PL
@@ -76,6 +76,8 @@ BEGIN {
$::HaveTemp = ($@ eq "");
eval { require Module::CoreList; };
$::HaveCoreList = ($@ eq "");
+ eval { require Text::Wrap; };
+ $::HaveWrap = ($@ eq "");
};
my $Version = "1.40";
@@ -1082,7 +1084,16 @@ sub _read_report {
my $content;
open( REP, "<:raw", $fname ) or die "Couldn't open file '$fname': $!\n";
binmode(REP, ':raw :crlf') if $Is_MSWin32;
- while (<REP>) { $content .= $_; }
+ # wrap long lines to make sure the report gets delivered
+ local $Text::Wrap::columns = 900;
+ local $Text::Wrap::huge = 'overflow';
+ while (<REP>) {
+ if ($::HaveWrap && /\S/) { # wrap() would remove empty lines
+ $content .= Text::Wrap::wrap(undef, undef, $_);
+ } else {
+ $content .= $_;
+ }
+ }
close(REP) or die "Error closing report file '$fname': $!";
return $content;
}
--
2.8.0.rc3
