{"thread":{"id":"27099","subject":"RFC: git send-email and error handling","startedAt":"2011-04-14T20:10:12Z","lastAt":"2011-05-07T13:21:05Z","messageCount":9,"participants":["Paul Gortmaker","Jeff King","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"165836","messageId":"4DA754A4.3090709@windriver.com","threadId":"27099","inReplyTo":null,"subject":"RFC: git send-email and error handling","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2011-04-14T20:10:12Z","receivedAt":"2011-04-14T20:10:12Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"I came across a situation today where the behaviour of git send-email\nkind of surprised me -- in that it seemed definitely less than ideal\nfor the use case I had.\n\nFor stable linux kernel releases, it is very common to send\nhundreds of patches at once, to people you've never contacted before,\nsimply because the are called out in CC: or SOB: lines of a patch\nthat has been cherry picked. So it is highly likely that you may\nhit full inboxes, expired accounts and so on.\n\nThe command line (git 1.7.4.4) is typically something like:\n\ngit send-email --to stable@kernel.org --to linux-kernel@vger.kernel.org \\\n --cc stable-review@kernel.org some_patch_dir\n\nSo, let me get to what happened today: After sending 113 out of 209\npatches, it came to the 114th patch, and gave me this:\n\n(mbox) Adding cc: Dmitry Torokhov from line 'From: Dmitry Torokhov '\n(body) Adding cc: Dmitry Torokhov from line 'Signed-off-by: Dmitry Torokhov '\n(body) Adding cc: Paul Gortmaker from line 'Signed-off-by: Paul Gortmaker '\n5.2.1 ... Mailbox disabled for this recipient\n\nThen, taking that as a hard error, it simply exited,\nleaving me scrambling to figure out how to quickly fix the\noffending patch and continue with the unsent queue.\n\n From my point of view, the right thing to do here would have\nbeen to ignore the error on the harvested mail address, and continue\non through the rest of the queue. Or even interactively ask me what\nto do when it saw the 5.2.1 failure. But maybe that wouldn't be\nright for everyone. I didn't see anything in the GSE man page\nthat would let me configure this behaviour either.\n\nAnyway, I thought I'd mention it and see where the discussion went.\n\nThanks,\nPaul.\n"},{"id":"165848","messageId":"20110414210913.GC6525@sigill.intra.peff.net","threadId":"27099","inReplyTo":"4DA754A4.3090709@windriver.com","subject":"Re: RFC: git send-email and error handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-14T21:09:13Z","receivedAt":"2011-04-14T21:09:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 14, 2011 at 04:10:12PM -0400, Paul Gortmaker wrote:\n\n> The command line (git 1.7.4.4) is typically something like:\n> \n> git send-email --to stable@kernel.org --to linux-kernel@vger.kernel.org \\\n> --cc stable-review@kernel.org some_patch_dir\n> \n> So, let me get to what happened today: After sending 113 out of 209\n> patches, it came to the 114th patch, and gave me this:\n> \n> (mbox) Adding cc: Dmitry Torokhov from line 'From: Dmitry Torokhov '\n> (body) Adding cc: Dmitry Torokhov from line 'Signed-off-by: Dmitry Torokhov '\n> (body) Adding cc: Paul Gortmaker from line 'Signed-off-by: Paul Gortmaker '\n> 5.2.1 ... Mailbox disabled for this recipient\n> \n> Then, taking that as a hard error, it simply exited,\n> leaving me scrambling to figure out how to quickly fix the\n> offending patch and continue with the unsent queue.\n\nI suspect part of the issue is that your mail setup is unlike that of\nmost people. Usually, we would deliver to some local MTA (either by SMTP\ndirectly, by sendmail that speaks SMTP to a smarthost, or by sendmail\nthat queues directly); that MTA would queue the message in a spool for\nyou, and attempt delivery asynchronously. So the errors generally come\nall or nothing. You can queue mail, or you can't, and trying again and\nagain after each error is just wasteful and annoying. Eventual errors\nare reported back to you as bounces.\n\nYour setup seems different; it looks like your sendmail (or the SMTP\nserver you connect to) actually routes the mail without queueing at all,\nand you synchronously get the final word on whether it can be delivered.\nOr maybe it just connects to the recipient site and checks that \"RCPT\nTO\" works before actually queueing. It's hard to say from the snippet\nabove. What's your MTA?\n\nI can see in your case how it would be preferable to keep going through\nthe list and then assemble a set of errors at the end. But that should\nbe configurable, because most setups won't want that behavior.\n\n> From my point of view, the right thing to do here would have\n> been to ignore the error on the harvested mail address, and continue\n> on through the rest of the queue.\n\nIt's a little tricky, because send-email may not know the details of\nwhat happened, especially if this behavior comes from a sendmail\nwrapper rather than SMTP. We dump the message and a list of recipients\nto an external program, and then get back a \"yes it was sent\" or \"no it\nwas not\" code. So we can't do anything clever, like say \"Well, it was\nsent, but not to one particular address, but that's OK because that\naddress was auto-harvested from a signed-off-by line\".\n\nWhether we can do better depends on your MTA. _If_ you are sending via\nSMTP, and _if_ it will reject particular recipients at the time of \"rcpt\nto\", then we could do something that clever. Given that the 5.2.1\nmessage appeared on your terminal, and that it should not have been\ngenerated by git-send-email, that implies to me you are using the local\nsendmail binary.\n\n> Or even interactively ask me what to do when it saw the 5.2.1 failure.\n> But maybe that wouldn't be right for everyone. I didn't see anything\n> in the GSE man page that would let me configure this behaviour either.\n\nThe problem there is that the message probably was not actually sent (or\nat least, sendmail presumably returned an error code to git, which is\nwhy git stopped). And you, as a human, saw that the error was something\nsurvivable. But you can't just tell git \"it's OK to continue\". You need\nto actually fix the problem and re-send, which means telling git that\nthe one particular address is not interesting. And that is a lot more\ninterface than just yes/no.\n\nI would think what you really want is a system that tries to send\neverything, keeps track of which recipients are to receive which\nmessage, and then marks success or failure for each. At the end, you\nwould find that dtor@mail.ru didn't receive anything, and realize you\ndon't care. And you don't have to worry about restarting a failed\nsend-email and sending duplicates. You know who got what.\n\nIf that sounds good, then you should consider switching MTAs, because\nthat is exactly what the job of an MTA is. :)\n\n-Peff\n"},{"id":"165865","messageId":"4DA791A2.3010901@windriver.com","threadId":"27099","inReplyTo":"20110414210913.GC6525@sigill.intra.peff.net","subject":"Re: RFC: git send-email and error handling","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2011-04-15T00:30:26Z","receivedAt":"2011-04-15T00:30:26Z","isPatch":false,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"On 11-04-14 05:09 PM, Jeff King wrote:\n> On Thu, Apr 14, 2011 at 04:10:12PM -0400, Paul Gortmaker wrote:\n> \n>> The command line (git 1.7.4.4) is typically something like:\n>>\n>> git send-email --to stable@kernel.org --to linux-kernel@vger.kernel.org \\\n>> --cc stable-review@kernel.org some_patch_dir\n>>\n>> So, let me get to what happened today: After sending 113 out of 209\n>> patches, it came to the 114th patch, and gave me this:\n>>\n>> (mbox) Adding cc: Dmitry Torokhov from line 'From: Dmitry Torokhov '\n>> (body) Adding cc: Dmitry Torokhov from line 'Signed-off-by: Dmitry Torokhov '\n>> (body) Adding cc: Paul Gortmaker from line 'Signed-off-by: Paul Gortmaker '\n>> 5.2.1 ... Mailbox disabled for this recipient\n>>\n>> Then, taking that as a hard error, it simply exited,\n>> leaving me scrambling to figure out how to quickly fix the\n>> offending patch and continue with the unsent queue.\n> \n> I suspect part of the issue is that your mail setup is unlike that of\n> most people. Usually, we would deliver to some local MTA (either by SMTP\n> directly, by sendmail that speaks SMTP to a smarthost, or by sendmail\n> that queues directly); that MTA would queue the message in a spool for\n> you, and attempt delivery asynchronously. So the errors generally come\n> all or nothing. You can queue mail, or you can't, and trying again and\n> again after each error is just wasteful and annoying. Eventual errors\n> are reported back to you as bounces.\n> \n> Your setup seems different; it looks like your sendmail (or the SMTP\n> server you connect to) actually routes the mail without queueing at all,\n> and you synchronously get the final word on whether it can be delivered.\n> Or maybe it just connects to the recipient site and checks that \"RCPT\n> TO\" works before actually queueing. It's hard to say from the snippet\n> above. What's your MTA?\n\nYes, the above is true -- I'm not queuing anything locally or involving\na local MTA. I've set sendemail.smtpserver in my ~/.gitconfig to the\nhostname of an infrastructural server running sendmail (telnet 25 doesnt\nshow me what version, but I'm told it is sendmail). This configuration\ngives me the most portability to run on any random machine within our\norg without having to wonder if it has a locally installed and correctly\nconfigured MTA -- it works so well, I've even abused git send-email to\ndispatch random (non-patch) mails from ad-hoc scripts on occasions, simply\nbecause I know everyone has git installed somewhere in $PATH.\n\n> \n> I can see in your case how it would be preferable to keep going through\n> the list and then assemble a set of errors at the end. But that should\n> be configurable, because most setups won't want that behavior.\n> \n>> From my point of view, the right thing to do here would have\n>> been to ignore the error on the harvested mail address, and continue\n>> on through the rest of the queue.\n> \n> It's a little tricky, because send-email may not know the details of\n> what happened, especially if this behavior comes from a sendmail\n> wrapper rather than SMTP. We dump the message and a list of recipients\n> to an external program, and then get back a \"yes it was sent\" or \"no it\n> was not\" code. So we can't do anything clever, like say \"Well, it was\n> sent, but not to one particular address, but that's OK because that\n> address was auto-harvested from a signed-off-by line\".\n\nTrue. I wonder if there is some flexibility in what we do, depending\non whether the setting is a local binary like /usr/bin/sendmail, vs.\na hostname of a server, like it was in my case...\n\n> \n> Whether we can do better depends on your MTA. _If_ you are sending via\n> SMTP, and _if_ it will reject particular recipients at the time of \"rcpt\n> to\", then we could do something that clever. Given that the 5.2.1\n> message appeared on your terminal, and that it should not have been\n> generated by git-send-email, that implies to me you are using the local\n> sendmail binary.\n> \n>> Or even interactively ask me what to do when it saw the 5.2.1 failure.\n>> But maybe that wouldn't be right for everyone. I didn't see anything\n>> in the GSE man page that would let me configure this behaviour either.\n> \n> The problem there is that the message probably was not actually sent (or\n> at least, sendmail presumably returned an error code to git, which is\n> why git stopped). And you, as a human, saw that the error was something\n> survivable. But you can't just tell git \"it's OK to continue\". You need\n> to actually fix the problem and re-send, which means telling git that\n> the one particular address is not interesting. And that is a lot more\n> interface than just yes/no.\n> \n> I would think what you really want is a system that tries to send\n> everything, keeps track of which recipients are to receive which\n> message, and then marks success or failure for each. At the end, you\n> would find that dtor@mail.ru didn't receive anything, and realize you\n> don't care. And you don't have to worry about restarting a failed\n> send-email and sending duplicates. You know who got what.\n> \n> If that sounds good, then you should consider switching MTAs, because\n> that is exactly what the job of an MTA is. :)\n\nI could set up a local queue, and for the big batches of patches I\nmay do that, but the portability of having sendemail.smtpserver pointing\nat a host instead of a program is too nice for me to be willing to\ngive that up globally.\n\nP.\n\n> \n> -Peff\n"},{"id":"165877","messageId":"20110415034251.GC19621@sigill.intra.peff.net","threadId":"27099","inReplyTo":"4DA791A2.3010901@windriver.com","subject":"Re: RFC: git send-email and error handling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-15T03:42:51Z","receivedAt":"2011-04-15T03:42:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 14, 2011 at 08:30:26PM -0400, Paul Gortmaker wrote:\n\n> > Your setup seems different; it looks like your sendmail (or the SMTP\n> > server you connect to) actually routes the mail without queueing at all,\n> > and you synchronously get the final word on whether it can be delivered.\n> > Or maybe it just connects to the recipient site and checks that \"RCPT\n> > TO\" works before actually queueing. It's hard to say from the snippet\n> > above. What's your MTA?\n> \n> Yes, the above is true -- I'm not queuing anything locally or involving\n> a local MTA. I've set sendemail.smtpserver in my ~/.gitconfig to the\n> hostname of an infrastructural server running sendmail (telnet 25 doesnt\n> show me what version, but I'm told it is sendmail).\n\nAh. I'm not up on my sendmail config, but googling around, there are\napparently milters that will do this kind of \"call-ahead\" rcpt checking.\n\n> This configuration gives me the most portability to run on any random\n> machine within our org without having to wonder if it has a locally\n> installed and correctly configured MTA -- it works so well, I've even\n> abused git send-email to dispatch random (non-patch) mails from ad-hoc\n> scripts on occasions, simply because I know everyone has git installed\n> somewhere in $PATH.\n\nYeah, I think submitting to a central server is a very sane config if\nyou don't have reliably local delivery.\n\n> > It's a little tricky, because send-email may not know the details of\n> > what happened, especially if this behavior comes from a sendmail\n> > wrapper rather than SMTP. We dump the message and a list of recipients\n> > to an external program, and then get back a \"yes it was sent\" or \"no it\n> > was not\" code. So we can't do anything clever, like say \"Well, it was\n> > sent, but not to one particular address, but that's OK because that\n> > address was auto-harvested from a signed-off-by line\".\n> \n> True. I wonder if there is some flexibility in what we do, depending\n> on whether the setting is a local binary like /usr/bin/sendmail, vs.\n> a hostname of a server, like it was in my case...\n\nSure. Since you are actually doing SMTP, you have much more flexibility\nin knowing what errors happen. Look in git-send-email.perl's\nsend_message, around line 1118. We use the Mail::SMTP module, but we\njust feed it the whole recipient list and barf if any of them is\nrejected. You could probably remember which recipients are \"important\"\n(i.e., given on the command line) and which were pulled automatically\nfrom the commit information, and then feed each recipient individually.\nIf important ones fail, abort the message. If an unimportant one fails,\nsend the message anyway, but remember the bad address and report the\nerror at the end.\n\nThat wouldn't help people using a sendmail binary, but there's nothing\nwe can do. That transport simply doesn't supply as much information, so\nit can't take advantage of the new feature. But it will be no worse off\nfor you adding the feature for SMTP users.\n\n-Peff\n"},{"id":"167026","messageId":"1304525528-24757-1-git-send-email-jnareb@gmail.com","threadId":"27099","inReplyTo":"20110415034251.GC19621@sigill.intra.peff.net","subject":"[RFC/PATCH] git-send-email: Remember sources of Cc addresses","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-04T16:12:08Z","receivedAt":"2011-05-04T16:12:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Instead of using @initial_cc to remember --cc=
command line\noptions, and @cc to remember Cc addresses derived from message body\nand --cc-cmd= and ultimately gather all Cc addresses,\nuse %cc hash to remember from where Cc addresses came from:\n * command line --cc=
('initial'),\n * \"From:\" email header in patch ('from'),\n * \"Cc:\" email header in patch ('cc'),\n * signoff lines and Cc lines in message body ('body'),\n * result of running from --cc-cmd= ('cc-cmd').\n\nThis is pure refactoring: we don't use this information, but gather\ntogether all of those in @cc variable local to send_message()\nsubroutine. No changes in behavior.\n\nWhile at it make assignment to $to variable in send_message()\nup, to make it more clear that @recipients is just @to then.\n\nSigned-off-by: Jakub Narebski \n---\nOn Fri, 15 Apr 2011, Jeff King wrote:\n> On Thu, Apr 14, 2011 at 08:30:26PM -0400, Paul Gortmaker wrote:\n\n>> True. I wonder if there is some flexibility in what we do, depending\n>> on whether the setting is a local binary like /usr/bin/sendmail, vs.\n>> a hostname of a server, like it was in my case...\n> \n> Sure. Since you are actually doing SMTP, you have much more flexibility\n> in knowing what errors happen. Look in git-send-email.perl's\n> send_message, around line 1118. We use the Mail::SMTP module, but we\n> just feed it the whole recipient list and barf if any of them is\n> rejected. You could probably remember which recipients are \"important\"\n> (i.e., given on the command line) and which were pulled automatically\n> from the commit information, and then feed each recipient individually.\n> If important ones fail, abort the message. If an unimportant one fails,\n> send the message anyway, but remember the bad address and report the\n> error at the end.\n> \n> That wouldn't help people using a sendmail binary, but there's nothing\n> we can do. That transport simply doesn't supply as much information, so\n> it can't take advantage of the new feature. But it will be no worse off\n> for you adding the feature for SMTP users.\n\nThis is an RFC patch preparing the way, so to speak, by remembering\nwhere each Cc address came from. We could in the future treat\n$cc{'body'} / all_cc('body') differently from the rest of all_cc().\n\nIs the approach taken here sane?\n\n git-send-email.perl | 52 ++++++++++++++++++++++++++++++--------------------\n 1 files changed, 31 insertions(+), 21 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 1c6b1a8..7d75a1e 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -140,10 +140,18 @@ my $smtp;\n my $auth;\n \n # Variables we fill in automatically, or via prompting:\n-my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n+my (@to,$no_to,@initial_to,%cc,$no_cc,@bcclist,$no_bcc,@xh,\n \t$initial_reply_to,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$compose,$time);\n \n+sub all_cc {\n+\tmy @keys = @_;\n+\t@keys = qw(initial from cc body cc-cmd) unless @keys;\n+\treturn map { ref($_) ? @$_ : () } @cc{@keys};\n+\n+\t#return map { ref($_) ? @$_ : () } values %cc;\n+}\n+\n my $envelope_sender;\n \n # Example reply to:\n@@ -221,7 +229,7 @@ my %config_settings = (\n \"smtpdomain\" => \\$smtp_domain,\n \"to\" => \\@initial_to,\n \"tocmd\" => \\$to_cmd,\n- \"cc\" => \\@initial_cc,\n+ \"cc\" => \\@{$cc{'initial'}},\n \"cccmd\" => \\$cc_cmd,\n \"aliasfiletype\" => \\$aliasfiletype,\n \"bcc\" => \\@bcclist,\n@@ -281,7 +289,7 @@ my $rc = GetOptions(\"sender|from=s\" => \\$sender,\n \t\t \"to=s\" => \\@initial_to,\n \t\t \"to-cmd=s\" => \\$to_cmd,\n \t\t \"no-to\" => \\$no_to,\n-\t\t \"cc=s\" => \\@initial_cc,\n+\t\t \"cc=s\" => \\@{$cc{'initial'}},\n \t\t \"no-cc\" => \\$no_cc,\n \t\t \"bcc=s\" => \\@bcclist,\n \t\t \"no-bcc\" => \\$no_bcc,\n@@ -423,7 +431,7 @@ foreach my $entry (@initial_to) {\n \tdie \"Comma in --to entry: $entry'\\n\" unless $entry !~ m/,/;\n }\n \n-foreach my $entry (@initial_cc) {\n+foreach my $entry (@{$cc{'initial'}}) {\n \tdie \"Comma in --cc entry: $entry'\\n\" unless $entry !~ m/,/;\n }\n \n@@ -753,7 +761,7 @@ sub expand_one_alias {\n \n @initial_to = expand_aliases(@initial_to);\n @initial_to = (map { sanitize_address($_) } @initial_to);\n-@initial_cc = expand_aliases(@initial_cc);\n+@{$cc{'initial'}} = expand_aliases(@{$cc{'initial'}});\n @bcclist = expand_aliases(@bcclist);\n \n if ($thread && !defined $initial_reply_to && $prompting) {\n@@ -959,12 +967,14 @@ sub maildomain {\n \n sub send_message {\n \tmy @recipients = unique_email_list(@to);\n-\t@cc = (grep { my $cc = extract_valid_address($_);\n-\t\t not grep { $cc eq $_ || $_ =~ /<\\Q${cc}\\E>$/ } @recipients\n-\t\t }\n-\t map { sanitize_address($_) }\n-\t @cc);\n-\tmy $to = join (\",\\n\\t\", @recipients);\n+\tmy $to = join(\",\\n\\t\", @recipients);\n+\tmy @cc =\n+\t\tgrep {\n+\t\t\tmy $cc = extract_valid_address($_);\n+\t\t\tnot grep { $cc eq $_ || $_ =~ /<\\Q${cc}\\E>$/ } @recipients\n+\t\t}\n+\t\tmap { sanitize_address($_) }\n+\t\tall_cc();\n \t@recipients = unique_email_list(@recipients,@cc,@bcclist);\n \t@recipients = (map { extract_valid_address($_) } @recipients);\n \tmy $date = format_2822_time($time++);\n@@ -1159,7 +1169,7 @@ foreach my $t (@files) {\n \tmy $has_content_type;\n \tmy $body_encoding;\n \t@to = ();\n-\t@cc = ();\n+\t$cc{$_} = [] foreach (qw(from cc body));\n \t@xh = ();\n \tmy $input_format = undef;\n \tmy @header = ();\n@@ -1197,7 +1207,7 @@ foreach my $t (@files) {\n \t\t\t\tnext if $suppress_cc{'self'} and $author eq $sender;\n \t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t$1, $_) unless $quiet;\n-\t\t\t\tpush @cc, $1;\n+\t\t\t\tpush @{$cc{'from'}}, $1;\n \t\t\t}\n \t\t\telsif (/^To:\\s+(.*)$/) {\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n@@ -1215,7 +1225,7 @@ foreach my $t (@files) {\n \t\t\t\t\t}\n \t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t\t$addr, $_) unless $quiet;\n-\t\t\t\t\tpush @cc, $addr;\n+\t\t\t\t\tpush @{$cc{'cc'}}, $addr;\n \t\t\t\t}\n \t\t\t}\n \t\t\telsif (/^Content-type:/i) {\n@@ -1239,10 +1249,10 @@ foreach my $t (@files) {\n \t\t\t# line 2 = subject\n \t\t\t# So let's support that, too.\n \t\t\t$input_format = 'lots';\n-\t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n+\t\t\tif (all_cc() == 0 && !$suppress_cc{'cc'}) {\n \t\t\t\tprintf(\"(non-mbox) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t\t$_, $_) unless $quiet;\n-\t\t\t\tpush @cc, $_;\n+\t\t\t\tpush @{$cc{'cc'}}, $_;\n \t\t\t} elsif (!defined $subject) {\n \t\t\t\t$subject = $_;\n \t\t\t}\n@@ -1261,7 +1271,7 @@ foreach my $t (@files) {\n \t\t\t\tnext if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;\n \t\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n \t\t\t}\n-\t\t\tpush @cc, $c;\n+\t\t\tpush @{$cc{'body'}}, $c;\n \t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n \t\t\t\t$c, $_) unless $quiet;\n \t\t}\n@@ -1270,7 +1280,7 @@ foreach my $t (@files) {\n \n \tpush @to, recipients_cmd(\"to-cmd\", \"to\", $to_cmd, $t)\n \t\tif defined $to_cmd;\n-\tpush @cc, recipients_cmd(\"cc-cmd\", \"cc\", $cc_cmd, $t)\n+\tpush @{$cc{'cc-cmd'}}, recipients_cmd(\"cc-cmd\", \"cc\", $cc_cmd, $t)\n \t\tif defined $cc_cmd && !$suppress_cc{'cccmd'};\n \n \tif ($broken_encoding{$t} && !$has_content_type) {\n@@ -1308,14 +1318,14 @@ foreach my $t (@files) {\n \t\t}\n \t}\n \n+\tmy $has_cc = all_cc();\n \t$needs_confirm = (\n \t\t$confirm eq \"always\" or\n-\t\t($confirm =~ /^(?:auto|cc)$/ && @cc) or\n+\t\t($confirm =~ /^(?:auto|cc)$/ && $has_cc) or\n \t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n-\t$needs_confirm = \"inform\" if ($needs_confirm && $confirm_unconfigured && @cc);\n+\t$needs_confirm = \"inform\" if ($needs_confirm && $confirm_unconfigured && $has_cc);\n \n \t@to = (@initial_to, @to);\n-\t@cc = (@initial_cc, @cc);\n \n \tmy $message_was_sent = send_message();\n \n-- \n1.7.5\n"},{"id":"167047","messageId":"20110504213535.GB27779@sigill.intra.peff.net","threadId":"27099","inReplyTo":"1304525528-24757-1-git-send-email-jnareb@gmail.com","subject":"Re: [RFC/PATCH] git-send-email: Remember sources of Cc addresses","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-04T21:35:35Z","receivedAt":"2011-05-04T21:35:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 04, 2011 at 06:12:08PM +0200, Jakub Narebski wrote:\n\n> > Sure. Since you are actually doing SMTP, you have much more flexibility\n> > in knowing what errors happen. Look in git-send-email.perl's\n> > send_message, around line 1118. We use the Mail::SMTP module, but we\n> > just feed it the whole recipient list and barf if any of them is\n> > rejected. You could probably remember which recipients are \"important\"\n> > (i.e., given on the command line) and which were pulled automatically\n> > from the commit information, and then feed each recipient individually.\n> > If important ones fail, abort the message. If an unimportant one fails,\n> > send the message anyway, but remember the bad address and report the\n> > error at the end.\n> [...]\n> This is an RFC patch preparing the way, so to speak, by remembering\n> where each Cc address came from. We could in the future treat\n> $cc{'body'} / all_cc('body') differently from the rest of all_cc().\n> \n> Is the approach taken here sane?\n\nYeah, from my cursory read, it looks like a good step forward, and I\ndidn't see any obvious bugs.\n\nYou'll need still more refactoring in send_message to treat them\ndifferently at the SMTP level. We collapse all of the addresses down to\na single list via unique_email_list (and we obviously want to keep this\nunique-ifying step), but that final list will have to remember where\neach address came from.\n\n> +sub all_cc {\n> +\tmy @keys = @_;\n> +\t@keys = qw(initial from cc body cc-cmd) unless @keys;\n> +\treturn map { ref($_) ? @$_ : () } @cc{@keys};\n> +\n> +\t#return map { ref($_) ? @$_ : () } values %cc;\n> +}\n\nNit: debugging cruft. :)\n\n-Peff\n"},{"id":"167106","messageId":"201105051601.46012.jnareb@gmail.com","threadId":"27099","inReplyTo":"20110504213535.GB27779@sigill.intra.peff.net","subject":"[RFC/PATCH 2/2] git-send-email: Do not require that addresses added from body be valid","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-05T14:01:44Z","receivedAt":"2011-05-05T14:01:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 4 May 2011, Jeff King wrote:\n> On Wed, May 04, 2011 at 06:12:08PM +0200, Jakub Narebski wrote:\n> > On Fri, 15 Apr 2011, Jeff King wrote:\n> > >\n> > > Sure. Since you are actually doing SMTP, you have much more flexibility\n> > > in knowing what errors happen. Look in git-send-email.perl's\n> > > send_message, around line 1118. We use the Mail::SMTP module, but we\n> > > just feed it the whole recipient list and barf if any of them is\n> > > rejected. You could probably remember which recipients are \"important\"\n> > > (i.e., given on the command line) and which were pulled automatically\n> > > from the commit information, and then feed each recipient individually.\n> > > If important ones fail, abort the message. If an unimportant one fails,\n> > > send the message anyway, but remember the bad address and report the\n> > > error at the end.\n> > [...]\n> > This is an RFC patch preparing the way, so to speak, by remembering\n> > where each Cc address came from. We could in the future treat\n> > $cc{'body'} / all_cc('body') differently from the rest of all_cc().\n> > \n> > Is the approach taken here sane?\n> \n> Yeah, from my cursory read, it looks like a good step forward, and I\n> didn't see any obvious bugs.\n> \n> You'll need still more refactoring in send_message to treat them\n> differently at the SMTP level. We collapse all of the addresses down to\n> a single list via unique_email_list (and we obviously want to keep this\n> unique-ifying step), but that final list will have to remember where\n> each address came from.\n\nBelow there is RFC patch that implements it by separating @cc and @cc_extra\nand later @recipients and @recipients_extra.\n\nHow about it?\n\nIt does not warn about bad addresses from body, and there are no tests yet!\n\n> > +sub all_cc {\n> > +\tmy @keys = @_;\n> > +\t@keys = qw(initial from cc body cc-cmd) unless @keys;\n> > +\treturn map { ref($_) ? @$_ : () } @cc{@keys};\n> > +\n> > +\t#return map { ref($_) ? @$_ : () } values %cc;\n> > +}\n> \n> Nit: debugging cruft. :)\n\nRight, I'll fix it in final version.\n\n-- >8 ---------- >8 --\nSubject: [PATCH] git-send-email: Do not require that addresses added from\n body be valid\n\nDo not barf if any of recipients that was pulled automatically from\ncommit information is rejected. This covers [currently] addresses\nfrom 'Cc:' and 'Signed-off-by:' lines in message body.\n\nThis is possible only if we are using Net::SMTP or Net::SMTP::SSL\n(it means that --smtp-server / sendemail.smtpserver is set to\ninternet address of outgoing SMTP server to use), because only then we\nhave control over which addresses are to be checked.\n\nCurrently no warnings that \"unimportant\" addresses were rejected are\nshown; we should report errors at the end.\n\nNo test for this new behavior: perhaps we could adapt some test from\nNet::SMTP module distribution...\n\nCc: Jeff King \nCc: Paul Gortmaker \nSigned-off-by: Jakub Narebski \n---\n git-send-email.perl | 40 +++++++++++++++++++++++++---------------\n 1 files changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 7d75a1e..e758fd9 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -968,22 +968,32 @@ sub maildomain {\n sub send_message {\n \tmy @recipients = unique_email_list(@to);\n \tmy $to = join(\",\\n\\t\", @recipients);\n-\tmy @cc =\n-\t\tgrep {\n-\t\t\tmy $cc = extract_valid_address($_);\n-\t\t\tnot grep { $cc eq $_ || $_ =~ /<\\Q${cc}\\E>$/ } @recipients\n-\t\t}\n-\t\tmap { sanitize_address($_) }\n-\t\tall_cc();\n-\t@recipients = unique_email_list(@recipients,@cc,@bcclist);\n+\tmy $sanitize_cc = sub {\n+\t\treturn\n+\t\t\tgrep {\n+\t\t\t\tmy $cc = extract_valid_address($_);\n+\t\t\t\tnot grep { $cc eq $_ || $_ =~ /<\\Q${cc}\\E>$/ } @recipients\n+\t\t\t}\n+\t\t\tmap { sanitize_address($_) }\n+\t\t\tall_cc(@_);\n+\t};\n+\tmy @cc = $sanitize_cc->(qw(initial from cc cc-cmd));\n+\tmy @cc_extra = $sanitize_cc->(qw(body));\n+\n+\tmy (%seen, @recipients_extra);\n+\t@recipients = unique_email_list(\\%seen,@recipients,@cc,@bcclist);\n \t@recipients = (map { extract_valid_address($_) } @recipients);\n+\t@recipients_extra =\n+\t\tmap { extract_valid_address($_) }\n+\t\tunique_email_list(\\%seen,@cc_extra);\n+\n \tmy $date = format_2822_time($time++);\n \tmy $gitversion = '@@GIT_VERSION@@';\n \tif ($gitversion =~ m/..GIT_VERSION../) {\n \t $gitversion = Git::version();\n \t}\n \n-\tmy $cc = join(\",\\n\\t\", unique_email_list(@cc));\n+\tmy $cc = join(\",\\n\\t\", unique_email_list(@cc,@cc_extra));\n \tmy $ccline = \"\";\n \tif ($cc ne '') {\n \t\t$ccline = \"\\nCc: $cc\";\n@@ -1007,7 +1017,7 @@ X-Mailer: git-send-email $gitversion\n \t\t$header .= join(\"\\n\", @xh) . \"\\n\";\n \t}\n \n-\tmy @sendmail_parameters = ('-i', @recipients);\n+\tmy @sendmail_parameters = ('-i', @recipients,@recipients_extra);\n \tmy $raw_from = $sanitized_sender;\n \tif (defined $envelope_sender && $envelope_sender ne \"auto\") {\n \t\t$raw_from = $envelope_sender;\n@@ -1126,6 +1136,7 @@ X-Mailer: git-send-email $gitversion\n \n \t\t$smtp->mail( $raw_from ) or die $smtp->message;\n \t\t$smtp->to( @recipients ) or die $smtp->message;\n+\t\t$smtp->to( @recipients_extra, { Notify => ['NEVER'], SkipBad => 1 });\n \t\t$smtp->data or die $smtp->message;\n \t\t$smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n \t\t$smtp->dataend() or die $smtp->message;\n@@ -1138,8 +1149,8 @@ X-Mailer: git-send-email $gitversion\n \t\tif ($smtp_server !~ m#^/#) {\n \t\t\tprint \"Server: $smtp_server\\n\";\n \t\t\tprint \"MAIL FROM:<$raw_from>\\n\";\n-\t\t\tforeach my $entry (@recipients) {\n-\t\t\t print \"RCPT TO:<$entry>\\n\";\n+\t\t\tforeach my $entry (@recipients,@recipients_extra) {\n+\t\t\t\tprint \"RCPT TO:<$entry>\\n\";\n \t\t\t}\n \t\t} else {\n \t\t\tprint \"Sendmail: $smtp_server \".join(' ',@sendmail_parameters).\"\\n\";\n@@ -1375,13 +1386,12 @@ sub cleanup_compose_files {\n $smtp->quit if $smtp;\n \n sub unique_email_list {\n-\tmy %seen;\n+\tmy $seen = ref $_[0] eq 'HASH' ? shift : {};\n \tmy @emails;\n \n \tforeach my $entry (@_) {\n \t\tif (my $clean = extract_valid_address($entry)) {\n-\t\t\t$seen{$clean} ||= 0;\n-\t\t\tnext if $seen{$clean}++;\n+\t\t\tnext if $seen->{$clean}++;\n \t\t\tpush @emails, $entry;\n \t\t} else {\n \t\t\tprint STDERR \"W: unable to extract a valid address\",\n-- \n1.7.5\n"},{"id":"167244","messageId":"201105061322.24736.jnareb@gmail.com","threadId":"27099","inReplyTo":"201105051601.46012.jnareb@gmail.com","subject":"[RFC/PATCH 3/2 (squash!)] git-send-email: Warn about rejected automatically added recipients","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-06T11:22:23Z","receivedAt":"2011-05-06T11:22:23Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Signed-off-by: Jakub Narebski \n---\nOn Thu, 5 May 2011, Jakub Narebski wrote:\n> On Wed, 4 May 2011, Jeff King wrote:\n>> On Wed, May 04, 2011 at 06:12:08PM +0200, Jakub Narebski wrote:\n>>> On Fri, 15 Apr 2011, Jeff King wrote:\n\n>>>> [...] You could probably remember which recipients are \"important\"\n>>>> (i.e., given on the command line) and which were pulled automatically\n>>>> from the commit information, and then feed each recipient individually.\n>>>> If important ones fail, abort the message. If an unimportant one fails,\n>>>> send the message anyway, but remember the bad address and report the\n>>>> error at the end.\n> \n> It does not warn about bad addresses from body, and there are no tests yet!\n\nNow it does warn, though I don't know if we should warn after each message,\nor all together at the end, and if we should warn only about _new_ addresses.\n\nStill no tests, and no idea how to write one...\n\nAlso if it is to be standalone commit, it needs better commit message.\nBut if it is to squashed with previous, it doesn't ;-)\n\n git-send-email.perl | 11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex e758fd9..b8d4fb9 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1136,11 +1136,20 @@ X-Mailer: git-send-email $gitversion\n \n \t\t$smtp->mail( $raw_from ) or die $smtp->message;\n \t\t$smtp->to( @recipients ) or die $smtp->message;\n-\t\t$smtp->to( @recipients_extra, { Notify => ['NEVER'], SkipBad => 1 });\n+\t\tmy @good_recips =\n+\t\t\t$smtp->to( @recipients_extra, { Notify => ['NEVER'], SkipBad => 1 });\n \t\t$smtp->data or die $smtp->message;\n \t\t$smtp->datasend(\"$header\\n$message\") or die $smtp->message;\n \t\t$smtp->dataend() or die $smtp->message;\n \t\t$smtp->code =~ /250|200/ or die \"Failed to send $subject\\n\".$smtp->message;\n+\n+\t\t%seen = ();\n+\t\tunique_email_list(\\%seen, @good_recips);\n+\t\t# bad recipients are those not seen on list of good recipients\n+\t\tmy @bad_recips = unique_email_list(\\%seen, @recipients_extra);\n+\t\t@bad_recips and\n+\t\t\twarn \"W: The following addresses added from body were rejected:\\n\\t\".\n+\t\t\t\tjoin(\"\\n\\t\", @bad_recips).\"\\n\";\n \t}\n \tif ($quiet) {\n \t\tprintf (($dry_run ? \"Dry-\" : \"\").\"Sent %s\\n\", $subject);\n-- \n1.7.5\n"},{"id":"167323","messageId":"201105071521.06344.jnareb@gmail.com","threadId":"27099","inReplyTo":"201105051601.46012.jnareb@gmail.com","subject":"Re: [RFC/PATCH 2/2] git-send-email: Do not require that addresses added from body be valid","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-07T13:21:05Z","receivedAt":"2011-05-07T13:21:05Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 5 May 2011, Jakub Narebski wrote:\n\n>                 $smtp->to( @recipients ) or die $smtp->message;\n> +               $smtp->to( @recipients_extra, { Notify => ['NEVER'], SkipBad => 1 });\n ^^^^^^^^^^^^^^^^^^^^\n\nNote: contrary to what I thought this doesn't mean to not send any\nnotification request, but to send DSN (Delivery Status Notification)\nrequest of 'NEVER'. For example when using smtp.gmail.com gives:\n\n Net::SMTP::recipient: DSN option not supported by host at ./git-send-email line 1140\n\nSo the underlined part has to be removed.\n\n-- \nJakub Narebski\nPoland\n"}]}