All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	"Eric Wong" <e@80x24.org>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2] send-email: add an option to impose delay sent E-Mails
Date: Tue, 14 Aug 2018 18:15:34 +0000	[thread overview]
Message-ID: <20180814181534.21234-1-avarab@gmail.com> (raw)
In-Reply-To: <20180325182803.30036-1-avarab@gmail.com>

Add a --send-delay option with a corresponding sendemail.smtpSendDelay
configuration variable. When set to e.g. 2, this causes send-email to
sleep 2 seconds before sending the next E-Mail. We'll only sleep
between sends, not before the first send, or after the last.

This option has two uses. Firstly, to be able to Ctrl+C a long send
with "all" if you have a change of heart. Secondly, as a hack in some
mail setups to, with a sufficiently high delay, force the receiving
client to sort the E-Mails correctly.

Some popular E-Mail clients completely ignore the "Date" header, which
format-patch is careful to set such that the patches will be displayed
in order, and instead sort by the time the E-mail was received.

Google's GMail is a good example of such a client. It ostensibly sorts
by some approximation of received time (although not by any "Received"
header). It's more usual than not to see patches showing out of order
in GMail. To take a few examples of orders seen on patches on the Git
mailing list:

    1 -> 3 -> 4 -> 2 -> 8 -> 7 (completion by Nguyễn Thái Ngọc Duy)
    2 -> 0 -> 1 -> 3 (pack search by Derrick Stolee)
    3 -> 2 -> 1 (fast-import by Jameson Miller)
    2 -> 3 -> 1 -> 5 -> 4 -> 6 (diff-highlight by Jeff King)

The reason to add the new "X-Mailer-Send-Delay" header is to make it
easy to tell what the imposed delay was, if any. This allows for
gathering some data on how the transfer of E-Mails with & without this
option behaves. This may not be workable without really long delays,
see [1] and [2].

The reason for why the getopt format is "send-delay=s" instead of
"send-delay=d" is because we're doing manual validation of the value
we get passed, which getopt would corrupt in cases of e.g. float
values before we could show a sensible error message.

1. https://public-inbox.org/git/20180325210132.GE74743@genre.crustytoothpaste.net/
2. https://public-inbox.org/git/xmqqpo3rehe4.fsf@gitster-ct.c.googlers.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

I submitted this back in March hoping it would solve mail ordering
problems, but the other motive I had for this is that I'm paranoid
that I'm sending out bad E-Mails, and tend to "y" to each one because
"a" is too fast.

So I'm re-sending this with an updated commit message & rationale, and
not sending 2/2 to toggle this on by default. I'd still like to have
this feature.

 Documentation/config.txt         |  6 ++++
 Documentation/git-send-email.txt |  4 +++
 git-send-email.perl              | 18 ++++++++---
 t/t9001-send-email.sh            | 55 ++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 63365dcf3d..5eb81b64a7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3216,6 +3216,12 @@ sendemail.smtpReloginDelay::
 	Seconds wait before reconnecting to smtp server.
 	See also the `--relogin-delay` option of linkgit:git-send-email[1].
 
+sendemail.smtpSendDelay::
+	Seconds wait in between message sending before sending another
+	message. Set it to 0 to impose no extra delay, defaults to 0.
++
+See also the `--send-delay` option of linkgit:git-send-email[1].
+
 showbranch.default::
 	The default set of branches for linkgit:git-show-branch[1].
 	See linkgit:git-show-branch[1].
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 465a4ecbed..98fdd9b803 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -270,6 +270,10 @@ must be used for each option.
 	with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
 	configuration variable.
 
+--send-delay=<int>::
+	Wait $<int> between sending emails. Defaults to the
+	`sendemail.smtpSendDelay` configuration variable.
+
 Automating
 ~~~~~~~~~~
 
diff --git a/git-send-email.perl b/git-send-email.perl
index 2be5dac337..65b53ee9f1 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -89,6 +89,7 @@ sub usage {
     --batch-size            <int>  * send max <int> message per connection.
     --relogin-delay         <int>  * delay <int> seconds between two successive login.
                                      This option can only be used with --batch-size
+    --send-delay            <int>  * ensure that <int> seconds pass between two successive sends.
 
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
@@ -225,7 +226,7 @@ sub do_edit {
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
-my ($batch_size, $relogin_delay);
+my ($batch_size, $relogin_delay, $send_delay);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -259,6 +260,7 @@ sub do_edit {
     "smtpauth" => \$smtp_auth,
     "smtpbatchsize" => \$batch_size,
     "smtprelogindelay" => \$relogin_delay,
+    "smtpsenddelay" => \$send_delay,
     "to" => \@initial_to,
     "tocmd" => \$to_cmd,
     "cc" => \@initial_cc,
@@ -373,6 +375,7 @@ sub signal_handler {
 		    "no-xmailer" => sub {$use_xmailer = 0},
 		    "batch-size=i" => \$batch_size,
 		    "relogin-delay=i" => \$relogin_delay,
+		    "send-delay=s" => \$send_delay,
 	 );
 
 usage() if $help;
@@ -484,6 +487,8 @@ sub read_config {
 };
 die sprintf(__("Unknown --confirm setting: '%s'\n"), $confirm)
 	unless $confirm =~ /^(?:auto|cc|compose|always|never)/;
+die sprintf(__("Invalid --send-delay setting: '%s'\n"), $send_delay)
+	if defined $send_delay and $send_delay !~ /^[0-9]+$/s;
 
 # Debugging, print out the suppressions.
 if (0) {
@@ -1562,7 +1567,8 @@ sub send_message {
 # Returns 0 if an edit was done and the function should be called again, or 1
 # otherwise.
 sub process_file {
-	my ($t) = @_;
+	my ($i) = @_;
+	my $t = $files[$i];
 
 	open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
 
@@ -1741,6 +1747,10 @@ sub process_file {
 		$message, $xfer_encoding, $target_xfer_encoding);
 	push @xh, "Content-Transfer-Encoding: $xfer_encoding";
 	unshift @xh, 'MIME-Version: 1.0' unless $has_mime_version;
+	if ($send_delay && $i > 0) {
+		push @xh, "X-Mailer-Send-Delay: $send_delay";
+		sleep $send_delay;
+	}
 
 	$needs_confirm = (
 		$confirm eq "always" or
@@ -1793,8 +1803,8 @@ sub process_file {
 	return 1;
 }
 
-foreach my $t (@files) {
-	while (!process_file($t)) {
+foreach my $i (0 .. $#files) {
+	while (!process_file($i)) {
 		# user edited the file
 	}
 }
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index b8e919e25d..791461fabd 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1759,6 +1759,61 @@ test_expect_success '--dump-aliases must be used alone' '
 	test_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting
 '
 
+test_expect_success '--send-delay expects whole non-negative seconds' '
+	test_must_fail git send-email --send-delay=-1 HEAD~ 2>errors &&
+	test_i18ngrep "Invalid --send-delay setting" errors &&
+	test_must_fail git send-email --send-delay=1.5 HEAD~ 2>errors &&
+	test_i18ngrep "Invalid --send-delay setting" errors &&
+	test_must_fail git -c sendemail.smtpSendDelay=-1 send-email HEAD~ 2>errors &&
+	test_i18ngrep "Invalid --send-delay setting" errors &&
+	test_must_fail git -c sendemail.smtpSendDelay=1.5 send-email HEAD~ 2>errors &&
+	test_i18ngrep "Invalid --send-delay setting" errors
+'
+
+test_expect_success $PREREQ "there is no default --send-delay" '
+	clean_fake_sendmail &&
+	rm -fr outdir &&
+	git format-patch -3 -o outdir &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		outdir/*.patch \
+		2>stderr >stdout &&
+	test $(grep -c "X-Mailer:" stdout) = 3 &&
+	test $(grep -c "X-Mailer-Send-Delay:" stdout) = 0
+'
+
+test_expect_success $PREREQ '--send-delay adds a X-Mailer-Send-Delay header to affected E-Mails' '
+	clean_fake_sendmail &&
+	rm -fr outdir &&
+	git format-patch -3 -o outdir &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--send-delay=2 \
+		outdir/*.patch \
+		2>stderr >stdout &&
+	test $(grep -c "X-Mailer:" stdout) = 3 &&
+	test $(grep -c "X-Mailer-Send-Delay:" stdout) = 2
+'
+
+test_expect_success $PREREQ '--send-delay=0 disables any imposed delay on E-Mail sending' '
+	clean_fake_sendmail &&
+	rm -fr outdir &&
+	git format-patch -3 -o outdir &&
+	git -c sendemail.smtpSendDelay=3 send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--send-delay=0 \
+		outdir/*.patch \
+		2>stderr >stdout &&
+	test $(grep -c "X-Mailer:" stdout) = 3 &&
+	test $(grep -c "X-Mailer-Send-Delay:" stdout) = 0
+'
+
 test_sendmail_aliases () {
 	msg="$1" && shift &&
 	expect="$@" &&
-- 
2.18.0.865.gffc8e1a3cd6


  parent reply	other threads:[~2018-08-14 18:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-25 18:28 [PATCH 0/2] send-email: impose a delay while sending to appease GMail Ævar Arnfjörð Bjarmason
2018-03-25 18:28 ` [PATCH 1/2] send-email: add an option to impose delay sent E-Mails Ævar Arnfjörð Bjarmason
2018-03-25 18:28 ` [PATCH 2/2] send-email: supply a --send-delay=1 by default Ævar Arnfjörð Bjarmason
2018-03-25 21:01   ` brian m. carlson
2018-03-25 22:01     ` Ævar Arnfjörð Bjarmason
2018-03-28  1:26       ` Eric Wong
2018-03-26  1:48     ` Junio C Hamano
2018-03-26  0:11   ` Eric Sunshine
2018-03-26  0:11     ` Eric Sunshine
2018-03-26  1:56     ` Junio C Hamano
2018-03-26  1:56       ` Junio C Hamano
2018-08-14 18:15 ` Ævar Arnfjörð Bjarmason [this message]
2018-08-14 18:39   ` [PATCH v2] send-email: add an option to impose delay sent E-Mails Stefan Beller
2018-08-14 18:45   ` Eric Wong
2018-08-14 19:53     ` Junio C Hamano
2018-08-14 21:02     ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180814181534.21234-1-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.