git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] send-email: new option to walkaround email server limits
@ 2017-04-29 12:26 xiaoqiang zhao
  2017-05-01  1:54 ` Junio C Hamano
  2017-05-02  9:32 ` Paolo Bonzini
  0 siblings, 2 replies; 9+ messages in thread
From: xiaoqiang zhao @ 2017-04-29 12:26 UTC (permalink / raw)
  To: git; +Cc: viktorin, mst, pbonzini, mina86, artagnon

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 --split <num> option, a auto reconnection will occur when number of sended
email reaches <num> and the problem is solved.

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

diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..0de9b7058 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -81,6 +81,8 @@ git send-email --dump-aliases
                                      This setting forces to use one of the listed mechanisms.
     --smtp-debug            <0|1>  * Disable, enable Net::SMTP debug.
 
+    --split                 <int>  * send \$num message per connection.
+
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
@@ -153,6 +155,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
 my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
+my $send_count = 0;
 
 # Regexes for RFC 2047 productions.
 my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
@@ -186,6 +189,7 @@ my $format_patch;
 my $compose_filename;
 my $force = 0;
 my $dump_aliases = 0;
+my $split = 0;
 
 # Handle interactive edition of files.
 my $multiedit;
@@ -358,6 +362,7 @@ $rc = GetOptions(
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
 		    "no-xmailer" => sub {$use_xmailer = 0},
+		    "split=i" => \$split,
 	 );
 
 usage() if $help;
@@ -1158,10 +1163,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 || $send_count != 0) {
 		return 1;
 	}
 
+	if ($auth && $send_count == 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 +1197,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 +1453,15 @@ EOF
 		}
 	}
 
+	$send_count++;
+	if ($send_count == $split) {
+		$smtp->quit;
+		$smtp = undef;
+		$send_count = 0;
+		print "Reconnect SMTP server required. \n";
+
+	}
+
 	return 1;
 }
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-04-29 12:26 [PATCH] send-email: new option to walkaround email server limits xiaoqiang zhao
@ 2017-05-01  1:54 ` Junio C Hamano
  2017-05-01  6:03   ` 赵小强
  2017-05-02  9:32 ` Paolo Bonzini
  1 sibling, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2017-05-01  1:54 UTC (permalink / raw)
  To: xiaoqiang zhao; +Cc: git, viktorin, mst, pbonzini, mina86, artagnon

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.

That sounds painful.

> With --split <num> option, a auto reconnection will occur when number of sended
> email reaches <num> and the problem is solved.

Two nits (other than s/sended/sent/ and s/$send_count/$num_sent/):

 - "--split" sounds as if you are splitting a single large message
   into multiple pieces, which of course is not what is going on
   here.  We need to find a better name for the option.  Perhaps
   "--batch-size=<num>", "--max-messages-per-connection=<num>" or
   something?

 - The code seems to do the "logging out and the logging back in"
   dance without any delay in between.  Is it something we want to
   consider to add a small (possibly configurable) delay in between?

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-05-01  1:54 ` Junio C Hamano
@ 2017-05-01  6:03   ` 赵小强
  2017-05-02  9:58     ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 9+ messages in thread
From: 赵小强 @ 2017-05-01  6:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, viktorin, mst, pbonzini, mina86, artagnon


Thanks for your reply , Junio !

> 在 2017年5月1日,09:54,Junio C Hamano <gitster@pobox.com> 写道:
> 
> here.  We need to find a better name for the option.  Perhaps
>   "--batch-size=<num>", "--max-messages-per-connection=<num>" or
>   something?
> 

--batch-size is ok with me

> - The code seems to do the "logging out and the logging back in"
>   dance without any delay in between.  Is it something we want to
>   consider to add a small (possibly configurable) delay in between?

It' s a good idea to have a configurable delay.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-04-29 12:26 [PATCH] send-email: new option to walkaround email server limits xiaoqiang zhao
  2017-05-01  1:54 ` Junio C Hamano
@ 2017-05-02  9:32 ` Paolo Bonzini
  2017-05-04  7:55   ` 赵小强
  1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2017-05-02  9:32 UTC (permalink / raw)
  To: xiaoqiang zhao, git; +Cc: viktorin, mst, mina86, artagnon



On 29/04/2017 14:26, xiaoqiang zhao wrote:
> 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 --split <num> option, a auto reconnection will occur when number of sended
> email reaches <num> and the problem is solved.
> 
> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>

I think you should also add a matching configuration option, or you are
going to forget it on the command line sooner or later!

Paolo

> ---
>  git-send-email.perl | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index eea0a517f..0de9b7058 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -81,6 +81,8 @@ git send-email --dump-aliases
>                                       This setting forces to use one of the listed mechanisms.
>      --smtp-debug            <0|1>  * Disable, enable Net::SMTP debug.
>  
> +    --split                 <int>  * send \$num message per connection.
> +
>    Automating:
>      --identity              <str>  * Use the sendemail.<id> options.
>      --to-cmd                <str>  * Email To: via `<str> \$patch_path`
> @@ -153,6 +155,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
>  my $have_mail_address = eval { require Mail::Address; 1 };
>  my $smtp;
>  my $auth;
> +my $send_count = 0;
>  
>  # Regexes for RFC 2047 productions.
>  my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
> @@ -186,6 +189,7 @@ my $format_patch;
>  my $compose_filename;
>  my $force = 0;
>  my $dump_aliases = 0;
> +my $split = 0;
>  
>  # Handle interactive edition of files.
>  my $multiedit;
> @@ -358,6 +362,7 @@ $rc = GetOptions(
>  		    "force" => \$force,
>  		    "xmailer!" => \$use_xmailer,
>  		    "no-xmailer" => sub {$use_xmailer = 0},
> +		    "split=i" => \$split,
>  	 );
>  
>  usage() if $help;
> @@ -1158,10 +1163,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 || $send_count != 0) {
>  		return 1;
>  	}
>  
> +	if ($auth && $send_count == 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 +1197,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 +1453,15 @@ EOF
>  		}
>  	}
>  
> +	$send_count++;
> +	if ($send_count == $split) {
> +		$smtp->quit;
> +		$smtp = undef;
> +		$send_count = 0;
> +		print "Reconnect SMTP server required. \n";
> +
> +	}
> +
>  	return 1;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-05-01  6:03   ` 赵小强
@ 2017-05-02  9:58     ` Ævar Arnfjörð Bjarmason
  2017-05-04  1:24       ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2017-05-02  9:58 UTC (permalink / raw)
  To: 赵小强
  Cc: Junio C Hamano, Git Mailing List, viktorin, mst, pbonzini,
	mina86, Ramkumar Ramachandra

On Mon, May 1, 2017 at 8:03 AM, 赵小强 <zxq_yx_007@163.com> wrote:
>
> Thanks for your reply , Junio !
>
>> 在 2017年5月1日,09:54,Junio C Hamano <gitster@pobox.com> 写道:
>>
>> here.  We need to find a better name for the option.  Perhaps
>>   "--batch-size=<num>", "--max-messages-per-connection=<num>" or
>>   something?
>>
>
> --batch-size is ok with me
>
>> - The code seems to do the "logging out and the logging back in"
>>   dance without any delay in between.  Is it something we want to
>>   consider to add a small (possibly configurable) delay in between?
>
> It' s a good idea to have a configurable delay.

It makes sense to have a configurable delay for git-send-email
unrelated to this option, I'd use a facility like that.

A lot of mail clients just sort based on date/msgid or whatever not
date/subject, so the rapid-fire output of send-email often arrives out
of order because it's all sent at the same second. I'd use some option
where I could send a series as "all" and have it sleep(N) in between
sending mails.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-05-02  9:58     ` Ævar Arnfjörð Bjarmason
@ 2017-05-04  1:24       ` Junio C Hamano
  2017-05-04  2:18         ` Ramkumar Ramachandra
  2017-05-04  7:48         ` Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2017-05-04  1:24 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: 赵小强,
	Git Mailing List, viktorin, mst, pbonzini, mina86,
	Ramkumar Ramachandra

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> It makes sense to have a configurable delay for git-send-email
> unrelated to this option, I'd use a facility like that.
>
> A lot of mail clients just sort based on date/msgid or whatever not
> date/subject, so the rapid-fire output of send-email often arrives out
> of order because it's all sent at the same second. I'd use some option
> where I could send a series as "all" and have it sleep(N) in between
> sending mails.

Hmph.  When sending many messages, send-email first grabs the
current time, counts backwards N seconds for N message series,
and uses that timestamp that is N seconds in the past for the first
message, incrementing the timestamp by 1 second per each subsequent
ones.

I found that this trick is sufficient to cause receiving MUAs sort
messages based on date, as the Date: field will have the timestamps
that increases by 1 second in between messages in a batch.  

There might be MUAs that do not use the value of the Date: field
when told to sort by date (perhaps they use the timestamp of the
message file they received at the final hop to them?), but it is
hopeless to help such MUAs unless the mail path guarantees the order
at the originator, which is not how "store and forward" e-mails
work.

So...

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-05-04  1:24       ` Junio C Hamano
@ 2017-05-04  2:18         ` Ramkumar Ramachandra
  2017-05-04  7:48         ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 9+ messages in thread
From: Ramkumar Ramachandra @ 2017-05-04  2:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ævar Arnfjörð Bjarmason, 赵小强,
	Git Mailing List, viktorin, mst, Paolo Bonzini, mina86

For the list, in plain text:

IIUC, they use the date received to sort. I think this might stem from
a historical cruft: emails sometimes took non-trivial amounts of time
to transit, back in the old days. MUAs (especially web-based ones)
probably did not want to violate user expectation by placing a new
email under several already-read emails. I would argue that this was
reasonable behavior. Further, since email is near-instantaneous today,
it really makes no difference which way the MUA sorts. Except for git
send-email.

It might be acceptable to put in a "practical hack" to help out MUAs
in the context of "near instant forwarding". I would argue against it
on grounds of it being an ugly hack, for very little benefit. The
patch that began this thread is also ugly, but has the important
consequence of enabling some people to use git send-email at all.

p.s- We should probably allow emails from mobile devices on the list.
It usually contains a HTML subpart.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-05-04  1:24       ` Junio C Hamano
  2017-05-04  2:18         ` Ramkumar Ramachandra
@ 2017-05-04  7:48         ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 9+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2017-05-04  7:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: 赵小强,
	Git Mailing List, viktorin, mst, pbonzini, mina86,
	Ramkumar Ramachandra

On Thu, May 4, 2017 at 3:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> It makes sense to have a configurable delay for git-send-email
>> unrelated to this option, I'd use a facility like that.
>>
>> A lot of mail clients just sort based on date/msgid or whatever not
>> date/subject, so the rapid-fire output of send-email often arrives out
>> of order because it's all sent at the same second. I'd use some option
>> where I could send a series as "all" and have it sleep(N) in between
>> sending mails.
>
> Hmph.  When sending many messages, send-email first grabs the
> current time, counts backwards N seconds for N message series,
> and uses that timestamp that is N seconds in the past for the first
> message, incrementing the timestamp by 1 second per each subsequent
> ones.
>
> I found that this trick is sufficient to cause receiving MUAs sort
> messages based on date, as the Date: field will have the timestamps
> that increases by 1 second in between messages in a batch.
>
> There might be MUAs that do not use the value of the Date: field
> when told to sort by date (perhaps they use the timestamp of the
> message file they received at the final hop to them?), but it is
> hopeless to help such MUAs unless the mail path guarantees the order
> at the originator, which is not how "store and forward" e-mails
> work.

As Ramkumar points out many MUAs don't sort by Date because of mail
delays / inaccurate sender clocks. Fun fact: You happen to work for a
company making one such MUA :)

E.g. just to name one example in my GMail view (it's full of this sort
of thing) Stefan Beller's recent cache.h series starts in the order
01/03/02 (and continues out of order).

The Date headers on the messages themselves are incremented by 1
second as you note, but on those first 3x the Recieved chain ends in
this for all 3:

    Received: by 10.28.48.210 with SMTP id w201csp570755wmw;
            Tue, 2 May 2017 15:24:50 -0700 (PDT)

I.e. all Received on the same second, showing that the Date header is
ignored by GMail, just from observing it GMail's sort function seems
to be (pseudocode):

    a['Received'] <=> b['Reiceved'] || a['GMailInternalID'] <=>
b['GMailInternalID']

Not:

    a['Received'] <=> b['Reiceved'] || a['MessageID'] <=>
b['MessageID'] || a['GMailInternalID'] <=> b['GMailInternalID']

Or:

    a['Received'] <=> b['Reiceved'] || a['Subject'] <=> b['Subject']
|| a['MessageID'] <=> b['MessageID'] || a['GMailInternalID'] <=>
b['GMailInternalID']

Or:

    a['Received'] <=> b['Reiceved'] || a['Date'] <=> b['Date'] ....

Anyway, you get the idea, but all of these would cause it to show
git-send-email list traffic in order, since while the Received field
is the same both Date & MessageID is different & sortable (actually
not quite, but that's another matter).

When I send my own patches with git-send-email I tend to sit there
hitting "y" in succession instead of doing "a" because I like being
able to browse my patches in order, which I guess tells you something
about my tolerance for tedium before submitting a patch to sleep().

If you look at GMail's web view you can effectively also see how close
someone is to Google's servers by how out of order their patches are,
e.g. your patches, Stefan's etc. & other Google employees working in
SV are pretty much entirely shuffled, whereas Michael Haggerty's are
pretty much in order because his mailpath involves a transatlantic &
trans-US-costal route before being delivered.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] send-email: new option to walkaround email server limits
  2017-05-02  9:32 ` Paolo Bonzini
@ 2017-05-04  7:55   ` 赵小强
  0 siblings, 0 replies; 9+ messages in thread
From: 赵小强 @ 2017-05-04  7:55 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: git, viktorin, mst, mina86, artagnon



在 2017年5月2日,17:32,Paolo Bonzini <pbonzini@redhat.com> 写道:

>> On 29/04/2017 14:26, xiaoqiang zhao wrote:
>> 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 --split <num> option, a auto reconnection will occur when number of sended
>> email reaches <num> and the problem is solved.
>> 
>> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
> 
> I think you should also add a matching configuration option, or you are
> going to forget it on the command line sooner or later!
> 
> Paolo

Good idea!


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-05-04  7:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-29 12:26 [PATCH] send-email: new option to walkaround email server limits xiaoqiang zhao
2017-05-01  1:54 ` Junio C Hamano
2017-05-01  6:03   ` 赵小强
2017-05-02  9:58     ` Ævar Arnfjörð Bjarmason
2017-05-04  1:24       ` Junio C Hamano
2017-05-04  2:18         ` Ramkumar Ramachandra
2017-05-04  7:48         ` Ævar Arnfjörð Bjarmason
2017-05-02  9:32 ` Paolo Bonzini
2017-05-04  7:55   ` 赵小强

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).