{"thread":{"id":"45841","subject":"[PATCH v2] send-email: new options to walkaround email server limits","startedAt":"2017-05-01T13:00:35Z","lastAt":"2017-05-02T02:25:05Z","messageCount":3,"participants":["xiaoqiang zhao","Jan Viktorin","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"318367","messageId":"20170501125928.11291-1-zxq_yx_007@163.com","threadId":"45841","inReplyTo":null,"subject":"[PATCH v2] send-email: new options to walkaround email server limits","fromName":"xiaoqiang zhao","fromEmail":"zxq_yx_007@163.com","sentAt":"2017-05-01T12:59:28Z","receivedAt":"2017-05-01T13:00:35Z","isPatch":true,"sender":{"key":"zxq_yx_007@163.com","avatar":"https://avatars.githubusercontent.com/u/3981296?v=4"},"body":"Some email server(e.g. smtp.163.com) limits a fixed number emails to\nbe send per session(connection) and this will lead to a send faliure.\n\nWith --batch-size= option, an auto reconnection will occur when\nnumber of sent email reaches and the problem is solved.\n\n--relogin-delay option will make some delay between two successive\nemail server login.\n\nSigned-off-by: xiaoqiang zhao \n---\n git-send-email.perl | 26 +++++++++++++++++++++++++-\n 1 file changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex eea0a517f..cd9981cc6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -81,6 +81,10 @@ git send-email --dump-aliases\n This setting forces to use one of the listed mechanisms.\n --smtp-debug <0|1> * Disable, enable Net::SMTP debug.\n \n+ --batch-size * send max \\$num message per connection.\n+ --relogin-delay * delay \\$num seconds between two successive login, default to 1,\n+ This option can only be used with --batch-size\n+\n Automating:\n --identity * Use the sendemail. options.\n --to-cmd * Email To: via ` \\$patch_path`\n@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };\n my $have_mail_address = eval { require Mail::Address; 1 };\n my $smtp;\n my $auth;\n+my $num_sent = 0;\n \n # Regexes for RFC 2047 productions.\n my $re_token = qr/[^][()<>@,;:\\\\\"\\/?.= \\000-\\037\\177-\\377]+/;\n@@ -186,6 +191,8 @@ my $format_patch;\n my $compose_filename;\n my $force = 0;\n my $dump_aliases = 0;\n+my $batch_size = 0;\n+my $relogin_delay = 1;\n \n # Handle interactive edition of files.\n my $multiedit;\n@@ -358,6 +365,8 @@ $rc = GetOptions(\n \t\t \"force\" => \\$force,\n \t\t \"xmailer!\" => \\$use_xmailer,\n \t\t \"no-xmailer\" => sub {$use_xmailer = 0},\n+\t\t \"batch-size=i\" => \\$batch_size,\n+\t\t \"relogin-delay=i\" => \\$relogin_delay,\n \t );\n \n usage() if $help;\n@@ -1158,10 +1167,15 @@ sub smtp_host_string {\n # (smtp_user was not specified), and 0 otherwise.\n \n sub smtp_auth_maybe {\n-\tif (!defined $smtp_authuser || $auth) {\n+\tif (!defined $smtp_authuser || $num_sent != 0) {\n \t\treturn 1;\n \t}\n \n+\tif ($auth && $num_sent == 0) {\n+\t\tprint \"Auth use saved password. \\n\";\n+\t\treturn !!$smtp->auth($smtp_authuser, $smtp_authpass);\n+\t}\n+\n \t# Workaround AUTH PLAIN/LOGIN interaction defect\n \t# with Authen::SASL::Cyrus\n \teval {\n@@ -1187,6 +1201,7 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n+\t\t$smtp_authpass = $cred->{'password'};\n \n \t\tif ($smtp_auth) {\n \t\t\tmy $sasl = Authen::SASL->new(\n@@ -1442,6 +1457,15 @@ EOF\n \t\t}\n \t}\n \n+\t$num_sent++;\n+\tif ($num_sent == $batch_size) {\n+\t\t$smtp->quit;\n+\t\t$smtp = undef;\n+\t\t$num_sent = 0;\n+\t\tprint \"Reconnect SMTP server required. \\n\";\n+\t\tsleep($relogin_delay);\n+\t}\n+\n \treturn 1;\n }\n \n-- \n2.13.0.rc1.16.g49e904895\n\n\n"},{"id":"318369","messageId":"20170501134057.5877840.92158.13927@rehivetech.com","threadId":"45841","inReplyTo":"20170501125928.11291-1-zxq_yx_007@163.com","subject":"Re: [PATCH v2] send-email: new options to walkaround email server limits","fromName":"Jan Viktorin","fromEmail":"viktorin@rehivetech.com","sentAt":"2017-05-01T13:40:57Z","receivedAt":"2017-05-01T13:49:59Z","isPatch":true,"sender":{"key":"viktorin@rehivetech.com","avatar":null},"body":"Hello, thank you for posting this improvement. I've been missing such feature in git. I hope to test it soon.\n\nJan Viktorin\nRehiveTech\nSent from a mobile device\n  Původní zpráva  \nOd: xiaoqiang zhao\nOdesláno: pondělí, 1. května 2017 15:00\nKomu: git@vger.kernel.org\nKopie: gitster@pobox.com; viktorin@rehivetech.com; mst@kernel.org; pbonzini@redhat.com; mina86@mina86.com; artagnon@gmail.com\nPředmět: [PATCH v2] send-email: new options to walkaround email server limits\n\nSome email server(e.g. smtp.163.com) limits a fixed number emails to\nbe send per session(connection) and this will lead to a send faliure.\n\nWith --batch-size= option, an auto reconnection will occur when\nnumber of sent email reaches and the problem is solved.\n\n--relogin-delay option will make some delay between two successive\nemail server login.\n\nSigned-off-by: xiaoqiang zhao \n---\ngit-send-email.perl | 26 +++++++++++++++++++++++++-\n1 file changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex eea0a517f..cd9981cc6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -81,6 +81,10 @@ git send-email --dump-aliases\nThis setting forces to use one of the listed mechanisms.\n--smtp-debug <0|1> * Disable, enable Net::SMTP debug.\n\n+ --batch-size * send max \\$num message per connection.\n+ --relogin-delay * delay \\$num seconds between two successive login, default to 1,\n+ This option can only be used with --batch-size\n+\nAutomating:\n--identity * Use the sendemail. options.\n--to-cmd * Email To: via ` \\$patch_path`\n@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };\nmy $have_mail_address = eval { require Mail::Address; 1 };\nmy $smtp;\nmy $auth;\n+my $num_sent = 0;\n\n# Regexes for RFC 2047 productions.\nmy $re_token = qr/[^][()<>@,;:\\\\\"\\/?.= \\000-\\037\\177-\\377]+/;\n@@ -186,6 +191,8 @@ my $format_patch;\nmy $compose_filename;\nmy $force = 0;\nmy $dump_aliases = 0;\n+my $batch_size = 0;\n+my $relogin_delay = 1;\n\n# Handle interactive edition of files.\nmy $multiedit;\n@@ -358,6 +365,8 @@ $rc = GetOptions(\n\"force\" => \\$force,\n\"xmailer!\" => \\$use_xmailer,\n\"no-xmailer\" => sub {$use_xmailer = 0},\n+\t \"batch-size=i\" => \\$batch_size,\n+\t \"relogin-delay=i\" => \\$relogin_delay,\n);\n\nusage() if $help;\n@@ -1158,10 +1167,15 @@ sub smtp_host_string {\n# (smtp_user was not specified), and 0 otherwise.\n\nsub smtp_auth_maybe {\n-\tif (!defined $smtp_authuser || $auth) {\n+\tif (!defined $smtp_authuser || $num_sent != 0) {\nreturn 1;\n}\n\n+\tif ($auth && $num_sent == 0) {\n+\t print \"Auth use saved password. \\n\";\n+\t return !!$smtp->auth($smtp_authuser, $smtp_authpass);\n+\t}\n+\n# Workaround AUTH PLAIN/LOGIN interaction defect\n# with Authen::SASL::Cyrus\neval {\n@@ -1187,6 +1201,7 @@ sub smtp_auth_maybe {\n'password' => $smtp_authpass\n}, sub {\nmy $cred = shift;\n+\t $smtp_authpass = $cred->{'password'};\n\nif ($smtp_auth) {\nmy $sasl = Authen::SASL->new(\n@@ -1442,6 +1457,15 @@ EOF\n}\n}\n\n+\t$num_sent++;\n+\tif ($num_sent == $batch_size) {\n+\t $smtp->quit;\n+\t $smtp = undef;\n+\t $num_sent = 0;\n+\t print \"Reconnect SMTP server required. \\n\";\n+\t sleep($relogin_delay);\n+\t}\n+\nreturn 1;\n}\n\n-- \n2.13.0.rc1.16.g49e904895\n\n\n"},{"id":"318451","messageId":"xmqqtw54dmh3.fsf@gitster.mtv.corp.google.com","threadId":"45841","inReplyTo":"20170501125928.11291-1-zxq_yx_007@163.com","subject":"Re: [PATCH v2] send-email: new options to walkaround email server limits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-02T02:24:56Z","receivedAt":"2017-05-02T02:25:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"xiaoqiang zhao writes:\n\n> Some email server(e.g. smtp.163.com) limits a fixed number emails to\n> be send per session(connection) and this will lead to a send faliure.\n>\n> With --batch-size= option, an auto reconnection will occur when\n> number of sent email reaches and the problem is solved.\n>\n> --relogin-delay option will make some delay between two successive\n> email server login.\n\nHere is how I would have written the above..\n\n send-email: --batch-size to work around some SMTP server limit\n\n Some email servers (e.g. smtp.163.com) limit the number emails to be\n sent per session(connection) and this will lead to a faliure when\n sending many messages.\n\n Teach send-email to disconnect after sending a number of messages\n (configurable via the --batch-size= option), wait for a few\n seconds (configurable via the --relogin-delay= option) and\n reconnect, to work around such a limit.\n\n\nBut I am having a huge problem seeing how this patch is correct. It\nalways is troubling to see a patch that makes the behaviour of a\nprogram change even when the optional feature it implements is not\nbeing used at all. Why does it even have to touch smtp_auth_maybe?\nWhy does the updated smtp_auth_maybe have to do quite different\nthings even when batch-size is not defined from the original?\nWhat is that new \"Auth use saved password. \\n\" message about?\n\nAfter reading the problem description in the proposed log message,\nthe most natural update to achieve the stated goal is to add code to\nthe loop that has the only caller to send_message() function, I\nwould think. The loop goes over the input files and prepares the\nvariables used in send_message() and have the function send a single\nmessage, initializing $smtp as necessary but otherwise reusing $smtp\nthe previous round has prepared. So just after $message_id is\nundefed in the loop, I expected that you would count \"number of\nmessages sent so far during this session\", and when that number\nexceeds the batch size, disconnect $smtp and unset the variable,\nand sleep for a bit, without having to change anything else.\n\nPuzzled.\n\n> sub smtp_auth_maybe {\n> -\tif (!defined $smtp_authuser || $auth) {\n> +\tif (!defined $smtp_authuser || $num_sent != 0) {\n> \t\treturn 1;\n> \t}\n> \n> +\tif ($auth && $num_sent == 0) {\n> +\t\tprint \"Auth use saved password. \\n\";\n> +\t\treturn !!$smtp->auth($smtp_authuser, $smtp_authpass);\n> +\t}\n> +\n> \t# Workaround AUTH PLAIN/LOGIN interaction defect\n> \t# with Authen::SASL::Cyrus\n> \teval {\n> @@ -1187,6 +1201,7 @@ sub smtp_auth_maybe {\n> \t\t'password' => $smtp_authpass\n> \t}, sub {\n> \t\tmy $cred = shift;\n> +\t\t$smtp_authpass = $cred->{'password'};\n> \n> \t\tif ($smtp_auth) {\n> \t\t\tmy $sasl = Authen::SASL->new(\n> @@ -1442,6 +1457,15 @@ EOF\n> \t\t}\n> \t}\n> \n> +\t$num_sent++;\n> +\tif ($num_sent == $batch_size) {\n> +\t\t$smtp->quit;\n> +\t\t$smtp = undef;\n> +\t\t$num_sent = 0;\n> +\t\tprint \"Reconnect SMTP server required. \\n\";\n> +\t\tsleep($relogin_delay);\n> +\t}\n> +\n> \treturn 1;\n> }\n"}]}