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

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

[Sam Vilain]

Expect a mail storm shortly, sorry folks.

Sam

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

[Sam Vilain]

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

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

[Will Palmer]

On Wed, 2010-05-26 at 18:50 +1200, Sam Vilain wrote:
> 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

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?

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

[Sam Vilain]

Will Palmer wrote:
>> 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

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

[Sverre Rabbelier]

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 <sam@vilain.net> 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

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

[Michael J Gruber]

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 <sam@vilain.net> 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.
> 

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

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

Michael

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

[Sam Vilain]

Michael J Gruber 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.
> 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 <sam@vilain.net>
---
 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

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

[Michael J Gruber]

Sam Vilain venit, vidit, dixit 26.05.2010 08:50:
> 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

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