[PATCH v2] send-email: new options to walkaround email server limits

[PATCH v2] send-email: new options to walkaround email server limits

[xiaoqiang zhao]

Some email server(e.g. smtp.163.com) limits a fixed number emails to
be send per session(connection) and this will lead to a send faliure.

With --batch-size=<num> option, an auto reconnection will occur when
number of sent email reaches <num> and the problem is solved.

--relogin-delay option will make some delay between two successive
email server login.

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 git-send-email.perl | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..cd9981cc6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -81,6 +81,10 @@ git send-email --dump-aliases
                                      This setting forces to use one of the listed mechanisms.
     --smtp-debug            <0|1>  * Disable, enable Net::SMTP debug.
 
+    --batch-size            <int>  * send max \$num message per connection.
+    --relogin-delay         <int>  * delay \$num seconds between two successive login, default to 1,
+                                     This option can only be used with --batch-size
+
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
 my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
+my $num_sent = 0;
 
 # Regexes for RFC 2047 productions.
 my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
@@ -186,6 +191,8 @@ my $format_patch;
 my $compose_filename;
 my $force = 0;
 my $dump_aliases = 0;
+my $batch_size = 0;
+my $relogin_delay = 1;
 
 # Handle interactive edition of files.
 my $multiedit;
@@ -358,6 +365,8 @@ $rc = GetOptions(
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
 		    "no-xmailer" => sub {$use_xmailer = 0},
+		    "batch-size=i" => \$batch_size,
+		    "relogin-delay=i" => \$relogin_delay,
 	 );
 
 usage() if $help;
@@ -1158,10 +1167,15 @@ sub smtp_host_string {
 # (smtp_user was not specified), and 0 otherwise.
 
 sub smtp_auth_maybe {
-	if (!defined $smtp_authuser || $auth) {
+	if (!defined $smtp_authuser || $num_sent != 0) {
 		return 1;
 	}
 
+	if ($auth && $num_sent == 0) {
+		print "Auth use saved password. \n";
+		return !!$smtp->auth($smtp_authuser, $smtp_authpass);
+	}
+
 	# Workaround AUTH PLAIN/LOGIN interaction defect
 	# with Authen::SASL::Cyrus
 	eval {
@@ -1187,6 +1201,7 @@ sub smtp_auth_maybe {
 		'password' => $smtp_authpass
 	}, sub {
 		my $cred = shift;
+		$smtp_authpass = $cred->{'password'};
 
 		if ($smtp_auth) {
 			my $sasl = Authen::SASL->new(
@@ -1442,6 +1457,15 @@ EOF
 		}
 	}
 
+	$num_sent++;
+	if ($num_sent == $batch_size) {
+		$smtp->quit;
+		$smtp = undef;
+		$num_sent = 0;
+		print "Reconnect SMTP server required. \n";
+		sleep($relogin_delay);
+	}
+
 	return 1;
 }
 
-- 
2.13.0.rc1.16.g49e904895

Re: [PATCH v2] send-email: new options to walkaround email server limits

[Jan Viktorin]

Hello, thank you for posting this improvement. I've been missing such feature in git. I hope to test it soon.

Jan Viktorin
RehiveTech
Sent from a mobile device
  Původní zpráva  
Od: xiaoqiang zhao
Odesláno: pondělí, 1. května 2017 15:00
Komu: git@vger.kernel.org
Kopie: gitster@pobox.com; viktorin@rehivetech.com; mst@kernel.org; pbonzini@redhat.com; mina86@mina86.com; artagnon@gmail.com
Předmět: [PATCH v2] send-email: new options to walkaround email server limits

Some email server(e.g. smtp.163.com) limits a fixed number emails to
be send per session(connection) and this will lead to a send faliure.

With --batch-size=<num> option, an auto reconnection will occur when
number of sent email reaches <num> and the problem is solved.

--relogin-delay option will make some delay between two successive
email server login.

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
git-send-email.perl | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..cd9981cc6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -81,6 +81,10 @@ git send-email --dump-aliases
This setting forces to use one of the listed mechanisms.
--smtp-debug <0|1> * Disable, enable Net::SMTP debug.

+ --batch-size <int> * send max \$num message per connection.
+ --relogin-delay <int> * delay \$num seconds between two successive login, default to 1,
+ This option can only be used with --batch-size
+
Automating:
--identity <str> * Use the sendemail.<id> options.
--to-cmd <str> * Email To: via `<str> \$patch_path`
@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
my $have_mail_address = eval { require Mail::Address; 1 };
my $smtp;
my $auth;
+my $num_sent = 0;

# Regexes for RFC 2047 productions.
my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
@@ -186,6 +191,8 @@ my $format_patch;
my $compose_filename;
my $force = 0;
my $dump_aliases = 0;
+my $batch_size = 0;
+my $relogin_delay = 1;

# Handle interactive edition of files.
my $multiedit;
@@ -358,6 +365,8 @@ $rc = GetOptions(
"force" => \$force,
"xmailer!" => \$use_xmailer,
"no-xmailer" => sub {$use_xmailer = 0},
+	 "batch-size=i" => \$batch_size,
+	 "relogin-delay=i" => \$relogin_delay,
);

usage() if $help;
@@ -1158,10 +1167,15 @@ sub smtp_host_string {
# (smtp_user was not specified), and 0 otherwise.

sub smtp_auth_maybe {
-	if (!defined $smtp_authuser || $auth) {
+	if (!defined $smtp_authuser || $num_sent != 0) {
return 1;
}

+	if ($auth && $num_sent == 0) {
+	 print "Auth use saved password. \n";
+	 return !!$smtp->auth($smtp_authuser, $smtp_authpass);
+	}
+
# Workaround AUTH PLAIN/LOGIN interaction defect
# with Authen::SASL::Cyrus
eval {
@@ -1187,6 +1201,7 @@ sub smtp_auth_maybe {
'password' => $smtp_authpass
}, sub {
my $cred = shift;
+	 $smtp_authpass = $cred->{'password'};

if ($smtp_auth) {
my $sasl = Authen::SASL->new(
@@ -1442,6 +1457,15 @@ EOF
}
}

+	$num_sent++;
+	if ($num_sent == $batch_size) {
+	 $smtp->quit;
+	 $smtp = undef;
+	 $num_sent = 0;
+	 print "Reconnect SMTP server required. \n";
+	 sleep($relogin_delay);
+	}
+
return 1;
}

-- 
2.13.0.rc1.16.g49e904895

Re: [PATCH v2] send-email: new options to walkaround email server limits

[Junio C Hamano]

xiaoqiang zhao <zxq_yx_007@163.com> writes:

> Some email server(e.g. smtp.163.com) limits a fixed number emails to
> be send per session(connection) and this will lead to a send faliure.
>
> With --batch-size=<num> option, an auto reconnection will occur when
> number of sent email reaches <num> and the problem is solved.
>
> --relogin-delay option will make some delay between two successive
> email server login.

Here is how I would have written the above..

    send-email: --batch-size to work around some SMTP server limit

    Some email servers (e.g. smtp.163.com) limit the number emails to be
    sent per session(connection) and this will lead to a faliure when
    sending many messages.

    Teach send-email to disconnect after sending a number of messages
    (configurable via the --batch-size=<num> option), wait for a few
    seconds (configurable via the --relogin-delay=<seconds> option) and
    reconnect, to work around such a limit.


But I am having a huge problem seeing how this patch is correct.  It
always is troubling to see a patch that makes the behaviour of a
program change even when the optional feature it implements is not
being used at all.  Why does it even have to touch smtp_auth_maybe?
Why does the updated smtp_auth_maybe have to do quite different
things even when batch-size is not defined from the original?
What is that new "Auth use saved password. \n" message about?

After reading the problem description in the proposed log message,
the most natural update to achieve the stated goal is to add code to
the loop that has the only caller to send_message() function, I
would think.  The loop goes over the input files and prepares the
variables used in send_message() and have the function send a single
message, initializing $smtp as necessary but otherwise reusing $smtp
the previous round has prepared.  So just after $message_id is
undefed in the loop, I expected that you would count "number of
messages sent so far during this session", and when that number
exceeds the batch size, disconnect $smtp and unset the variable,
and sleep for a bit, without having to change anything else.

Puzzled.

>  sub smtp_auth_maybe {
> -	if (!defined $smtp_authuser || $auth) {
> +	if (!defined $smtp_authuser || $num_sent != 0) {
>  		return 1;
>  	}
>  
> +	if ($auth && $num_sent == 0) {
> +		print "Auth use saved password. \n";
> +		return !!$smtp->auth($smtp_authuser, $smtp_authpass);
> +	}
> +
>  	# Workaround AUTH PLAIN/LOGIN interaction defect
>  	# with Authen::SASL::Cyrus
>  	eval {
> @@ -1187,6 +1201,7 @@ sub smtp_auth_maybe {
>  		'password' => $smtp_authpass
>  	}, sub {
>  		my $cred = shift;
> +		$smtp_authpass = $cred->{'password'};
>  
>  		if ($smtp_auth) {
>  			my $sasl = Authen::SASL->new(
> @@ -1442,6 +1457,15 @@ EOF
>  		}
>  	}
>  
> +	$num_sent++;
> +	if ($num_sent == $batch_size) {
> +		$smtp->quit;
> +		$smtp = undef;
> +		$num_sent = 0;
> +		print "Reconnect SMTP server required. \n";
> +		sleep($relogin_delay);
> +	}
> +
>  	return 1;
>  }