Apology/Bug report: git-send-email sends everything on Ctrl+C

Expect a mail storm shortly, sorry folks.

Sam

Sam Vilain wrote:

Expect a mail storm shortly, sorry folks.
  

I just had a look at patching git-send-email.perl; but it looks like it
is checking $? correctly at least since beece9da;

            system('sh', '-c', $editor.' "$@"', $editor, $_);
            if (($? & 127) || ($? >> 8)) {
                die("the editor exited uncleanly, aborting everything");
            }

I think I must have actually hit Ctrl-Z, not Ctrl-C.  So, it's all my
fault and I apologize profusely.

Sam

On Wed, 2010-05-26 at 18:50 +1200, Sam Vilain wrote:

Sam Vilain wrote:

quoted

Expect a mail storm shortly, sorry folks.
  

I just had a look at patching git-send-email.perl; but it looks like it
is checking $? correctly at least since beece9da;

            system('sh', '-c', $editor.' "$@"', $editor, $_);
            if (($? & 127) || ($? >> 8)) {
                die("the editor exited uncleanly, aborting everything");
            }

I think I must have actually hit Ctrl-Z, not Ctrl-C.  So, it's all my
fault and I apologize profusely.

Sam

I'd consider anything that can make "accidentally sending 100+ patches"
possible to be a bug. How does "it was Ctrl-Z, not Ctrl-C" make a
difference here?

Sam Vilain venit, vidit, dixit 26.05.2010 08:50:

Sam Vilain wrote:

quoted

Expect a mail storm shortly, sorry folks.
  

I just had a look at patching git-send-email.perl; but it looks like it
is checking $? correctly at least since beece9da;

            system('sh', '-c', $editor.' "$@"', $editor, $_);
            if (($? & 127) || ($? >> 8)) {
                die("the editor exited uncleanly, aborting everything");
            }

I think I must have actually hit Ctrl-Z, not Ctrl-C.  So, it's all my
fault and I apologize profusely.

Sam

I don't want to make you feel even worse, but could you tell us what
mistake you made so that we can avoid it? Wrong base rev for
format-patch baserev.. and piping directly into send-email?

Michael

Will Palmer wrote:

quoted

I think I must have actually hit Ctrl-Z, not Ctrl-C.  So, it's all my
fault and I apologize profusely.
    

I'd consider anything that can make "accidentally sending 100+ patches"
possible to be a bug. How does "it was Ctrl-Z, not Ctrl-C" make a
difference here?

This was what I did;

on git.git next or so; I was thinking "hmm, how does git send-email
format the cover letter for editing by the user?  I know, I'll just check":

 $ git send-email --compose HEAD~5..

(tab around a bit, I think I hit ctrl+z because my history has "jobs" in
it just after that command)

(poke around in the git-send-email source a bit)

(inspect the file it produced, oh look, HEAD~5.. matches an awful lot of
commits)

(close edit window without changes)

(notice hundreds of e-mail headers being spewed out)

Ctrl+C.  nothing (it's in the background)

Ctrl+C again.  argh!  What MTA am I running?

 $ sudo /etc/init.d/postfix stop
 $ sudo /etc/init.d/exim4 stop
 $ cat .git/config

Oh no, it's going to a mail server I don't have root on.  It was fully
configured so it didn't need to ask any questions, just fire away.  Why
did I test that on my git.git checkout?

Quick, find a friendly sysadmin and mailq | grep sam.vilain | cut -d" "
-f1 | sudo xargs -n1 postsuper -d

Hard to know what could have possibly stopped this from happening.  PEBKAC.

Sam

Heya,

[sorry for my earlier reply to your other accidental mail, feel free
to ignore it as I had not read this thread yet]

On Wed, May 26, 2010 at 10:51, Sam Vilain [off-list ref] wrote:

Hard to know what could have possibly stopped this from happening.  PEBKAC.

Perhaps git can be smart and ask for confirmation for huge (>20 ?) patchsets.

-- 
Cheers,

Sverre Rabbelier

Sverre Rabbelier venit, vidit, dixit 26.05.2010 11:07:

Heya,

[sorry for my earlier reply to your other accidental mail, feel free
to ignore it as I had not read this thread yet]

On Wed, May 26, 2010 at 10:51, Sam Vilain [off-list ref] wrote:

quoted

Hard to know what could have possibly stopped this from happening.  PEBKAC.

Perhaps git can be smart and ask for confirmation for huge (>20 ?) patchsets.

to be overriden by an option "--no-villain"...

Sorry, bad joke, but couldn't resist. Feel free to pick on my name ;)

Michael

Michael J Gruber wrote:

quoted

quoted

Hard to know what could have possibly stopped this from happening.
PEBKAC.

Perhaps git can be smart and ask for confirmation for huge (>20 ?)
patchsets.

to be overriden by an option "--no-villain"...

Sorry, bad joke, but couldn't resist. Feel free to pick on my name ;)

I went with the simpler and pre-existing --confirm=never :-)

Subject: [PATCH] send-email: confirm for really large patch sets

It is possible to send a lot of e-mails using quite simple user errors
with git send-email; if they were not intended this could be quite a
lot of spam.  Ask a final question if there are a lot of messages
before sending.

Signed-off-by: Sam Vilain <redacted>
---
 untested: after what just happens there is no way I'm testing this :-)

 git-send-email.perl |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 12622fc..093b7f2 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt

@@ -257,6 +257,11 @@ Administering
 Default is the value of 'sendemail.confirm' configuration value; if that
 is unspecified, default to 'auto' unless any of the suppress options
 have been specified, in which case default to 'compose'.
++
+If you are sending a very large patch sequence (more than 20 parts),
+you will be asked for a second final confirmation before anything is
+sent.  This can be suppressed by setting this configuration value to
+'never'.
 
 --dry-run::
        Do everything except actually send the emails.

diff --git a/git-send-email.perl b/git-send-email.perl
index 111c981..2775506 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl

@@ -1096,6 +1096,19 @@ $references = $initial_reply_to || '';
 $subject = $initial_subject;
 $message_num = 0;
 
+if ($needs_confirm ne "never" and @files > 20) {
+	$_ = ask(
+		"That's a big patch set - ".@files." parts.  "
+		."Are you sure you want to do this? ([y]es|[n]o): ",
+		valid_re => qr/^(?:yes|y|no|n)/i,
+		default => "n",
+	       );
+	if (/^n/i) {
+		cleanup_compose_files();
+		exit(0);
+	}
+}
+
 foreach my $t (@files) {
 	open(F,"<",$t) or die "can't open file $t";
 

-- 
1.7.1.rc2.333.gb2668