From eed6ca7c402cb51ff6cfd3ecf1f1cac788579b5d Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Sat, 14 Feb 2009 23:32:13 -0500 Subject: send-email: allow send-email to run outside a repo send-email is supposed to be able to run from outside a repo. This ability was broken by commits caf0c3d6 (make the message file name more specific) and 5df9fcf6 (interpret unknown files as revision lists). This commit provides a fix for both. Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano diff --git a/git-send-email.perl b/git-send-email.perl index 77ca8fe..9dad100 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -23,7 +23,7 @@ use Getopt::Long; use Text::ParseWords; use Data::Dumper; use Term::ANSIColor; -use File::Temp qw/ tempdir /; +use File::Temp qw/ tempdir tempfile /; use Error qw(:try); use Git; @@ -156,7 +156,10 @@ if ($@) { # Behavior modification variables my ($quiet, $dry_run) = (0, 0); my $format_patch; -my $compose_filename = $repo->repo_path() . "/.gitsendemail.msg.$$"; +my $compose_filename = ($repo ? + tempfile(".gitsendemail.msg.XXXXXX", DIR => $repo->repo_path()) : + tempfile(".gitsendemail.msg.XXXXXX", DIR => "."))[1]; + # Handle interactive edition of files. my $multiedit; @@ -267,6 +270,9 @@ unless ($rc) { usage(); } +die "Cannot run git format-patch from outside a repository\n" + if $format_patch and not $repo; + # Now, let's fill any that aren't set in with defaults: sub read_config { @@ -404,6 +410,7 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) { # returns 1 if the conflict must be solved using it as a format-patch argument sub check_file_rev_conflict($) { + return unless $repo; my $f = shift; try { $repo->command('rev-parse', '--verify', '--quiet', $f); @@ -445,6 +452,8 @@ while (defined(my $f = shift @ARGV)) { } if (@rev_list_opts) { + die "Cannot run git format-patch from outside a repository\n" + unless $repo; push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts); } -- cgit v0.10.2-6-g49f6 From 5012699d9840fe34fe0838ea0d529c2f32f76b82 Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Sat, 14 Feb 2009 23:32:14 -0500 Subject: send-email: handle multiple Cc addresses when reading mbox message When git format-patch is given multiple --cc arguments, it generates a Cc header that looks like: Cc: first@example.com, second@example.com, third@example.com Before this commit, send-email was unable to handle such a message as it did not handle folded header lines, nor multiple recipients in a Cc line. This patch: - Unfolds header lines by pre-processing the header before extracting any of its fields. - Handles Cc lines with multiple recipients. - Adds use of Mail::Address if available for splitting Cc line and the "Who should the emails be sent to?" prompt", with fall back to existing split_addrs() function. - Tests the new functionality and adds two tests for detecting whether "From:" appears correctly in message body when patch author differs from patch sender. Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano diff --git a/git-send-email.perl b/git-send-email.perl index 9dad100..a6efd1f 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -126,6 +126,7 @@ sub format_2822_time { } my $have_email_valid = eval { require Email::Valid; 1 }; +my $have_mail_address = eval { require Mail::Address; 1 }; my $smtp; my $auth; @@ -366,6 +367,14 @@ foreach my $entry (@bcclist) { die "Comma in --bcclist entry: $entry'\n" unless $entry !~ m/,/; } +sub parse_address_line { + if ($have_mail_address) { + return map { $_->format } Mail::Address->parse($_[0]); + } else { + return split_addrs($_[0]); + } +} + sub split_addrs { return quotewords('\s*,\s*', 1, @_); } @@ -602,7 +611,7 @@ if (!@to) { } my $to = $_; - push @to, split_addrs($to); + push @to, parse_address_line($to); $prompting++; } @@ -929,88 +938,98 @@ foreach my $t (@files) { @cc = @initial_cc; @xh = (); my $input_format = undef; - my $header_done = 0; + my @header = (); $message = ""; + # First unfold multiline header fields while() { - if (!$header_done) { - if (/^From /) { - $input_format = 'mbox'; - next; + last if /^\s*$/; + if (/^\s+\S/ and @header) { + chomp($header[$#header]); + s/^\s+/ /; + $header[$#header] .= $_; + } else { + push(@header, $_); + } + } + # Now parse the header + foreach(@header) { + if (/^From /) { + $input_format = 'mbox'; + next; + } + chomp; + if (!defined $input_format && /^[-A-Za-z]+:\s/) { + $input_format = 'mbox'; + } + + if (defined $input_format && $input_format eq 'mbox') { + if (/^Subject:\s+(.*)$/) { + $subject = $1; } - chomp; - if (!defined $input_format && /^[-A-Za-z]+:\s/) { - $input_format = 'mbox'; + elsif (/^From:\s+(.*)$/) { + ($author, $author_encoding) = unquote_rfc2047($1); + next if $suppress_cc{'author'}; + next if $suppress_cc{'self'} and $author eq $sender; + printf("(mbox) Adding cc: %s from line '%s'\n", + $1, $_) unless $quiet; + push @cc, $1; } - - if (defined $input_format && $input_format eq 'mbox') { - if (/^Subject:\s+(.*)$/) { - $subject = $1; - - } elsif (/^(Cc|From):\s+(.*)$/) { - if (unquote_rfc2047($2) eq $sender) { + elsif (/^Cc:\s+(.*)$/) { + foreach my $addr (parse_address_line($1)) { + if (unquote_rfc2047($addr) eq $sender) { next if ($suppress_cc{'self'}); - } - elsif ($1 eq 'From') { - ($author, $author_encoding) - = unquote_rfc2047($2); - next if ($suppress_cc{'author'}); } else { next if ($suppress_cc{'cc'}); } printf("(mbox) Adding cc: %s from line '%s'\n", - $2, $_) unless $quiet; - push @cc, $2; + $addr, $_) unless $quiet; + push @cc, $addr; } - elsif (/^Content-type:/i) { - $has_content_type = 1; - if (/charset="?([^ "]+)/) { - $body_encoding = $1; - } - push @xh, $_; - } - elsif (/^Message-Id: (.*)/i) { - $message_id = $1; - } - elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) { - push @xh, $_; - } - - } else { - # In the traditional - # "send lots of email" format, - # line 1 = cc - # line 2 = subject - # So let's support that, too. - $input_format = 'lots'; - if (@cc == 0 && !$suppress_cc{'cc'}) { - printf("(non-mbox) Adding cc: %s from line '%s'\n", - $_, $_) unless $quiet; - - push @cc, $_; - - } elsif (!defined $subject) { - $subject = $_; + } + elsif (/^Content-type:/i) { + $has_content_type = 1; + if (/charset="?([^ "]+)/) { + $body_encoding = $1; } + push @xh, $_; } - - # A whitespace line will terminate the headers - if (m/^\s*$/) { - $header_done = 1; + elsif (/^Message-Id: (.*)/i) { + $message_id = $1; } + elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) { + push @xh, $_; + } + } else { - $message .= $_; - if (/^(Signed-off-by|Cc): (.*)$/i) { - next if ($suppress_cc{'sob'}); - chomp; - my $c = $2; - chomp $c; - next if ($c eq $sender and $suppress_cc{'self'}); - push @cc, $c; - printf("(sob) Adding cc: %s from line '%s'\n", - $c, $_) unless $quiet; + # In the traditional + # "send lots of email" format, + # line 1 = cc + # line 2 = subject + # So let's support that, too. + $input_format = 'lots'; + if (@cc == 0 && !$suppress_cc{'cc'}) { + printf("(non-mbox) Adding cc: %s from line '%s'\n", + $_, $_) unless $quiet; + push @cc, $_; + } elsif (!defined $subject) { + $subject = $_; } } } + # Now parse the message body + while() { + $message .= $_; + if (/^(Signed-off-by|Cc): (.*)$/i) { + next if ($suppress_cc{'sob'}); + chomp; + my $c = $2; + chomp $c; + next if ($c eq $sender and $suppress_cc{'self'}); + push @cc, $c; + printf("(sob) Adding cc: %s from line '%s'\n", + $c, $_) unless $quiet; + } + } close F; if (defined $cc_cmd && !$suppress_cc{'cccmd'}) { @@ -1029,7 +1048,7 @@ foreach my $t (@files) { or die "(cc-cmd) failed to close pipe to '$cc_cmd'"; } - if (defined $author) { + if (defined $author and $author ne $sender) { $message = "From: $author\n\n$message"; if (defined $author_encoding) { if ($has_content_type) { diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index cb3d183..63ab88b 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -32,7 +32,7 @@ clean_fake_sendmail() { } test_expect_success 'Extract patches' ' - patches=`git format-patch -n HEAD^1` + patches=`git format-patch --cc="One " --cc=two@example.com -n HEAD^1` ' test_expect_success 'Send patches' ' @@ -42,6 +42,8 @@ test_expect_success 'Send patches' ' cat >expected <<\EOF !nobody@example.com! !author@example.com! +!one@example.com! +!two@example.com! EOF test_expect_success \ 'Verify commandline' \ @@ -50,13 +52,15 @@ test_expect_success \ cat >expected-show-all-headers <<\EOF 0001-Second.patch (mbox) Adding cc: A from line 'From: A ' +(mbox) Adding cc: One from line 'Cc: One , two@example.com' +(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com' Dry-OK. Log says: Server: relay.example.com MAIL FROM: -RCPT TO:,,, +RCPT TO:,,,,, From: Example To: to@example.com -Cc: cc@example.com, A +Cc: cc@example.com, A , One , two@example.com Subject: [PATCH 1/1] Second. Date: DATE-STRING Message-Id: MESSAGE-ID-STRING @@ -104,6 +108,28 @@ test_expect_success 'no patch was sent' ' ! test -e commandline1 ' +test_expect_success 'Author From: in message body' ' + clean_fake_sendmail && + git send-email \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $patches && + sed "1,/^$/d" < msgtxt1 > msgbody1 + grep "From: A " msgbody1 +' + +test_expect_success 'Author From: not in message body' ' + clean_fake_sendmail && + git send-email \ + --from="A " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $patches && + sed "1,/^$/d" < msgtxt1 > msgbody1 + ! grep "From: A " msgbody1 +' + test_expect_success 'allow long lines with --no-validate' ' git send-email \ --from="Example " \ @@ -170,13 +196,15 @@ test_expect_success 'second message is patch' ' cat >expected-show-all-headers <<\EOF 0001-Second.patch (mbox) Adding cc: A from line 'From: A ' +(mbox) Adding cc: One from line 'Cc: One , two@example.com' +(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com' Dry-OK. Log says: Server: relay.example.com MAIL FROM: -RCPT TO:,, +RCPT TO:,,,, From: Example To: to@example.com -Cc: cc@example.com, A +Cc: cc@example.com, A , One , two@example.com Subject: [PATCH 1/1] Second. Date: DATE-STRING Message-Id: MESSAGE-ID-STRING @@ -203,13 +231,15 @@ test_expect_success 'sendemail.cc set' ' cat >expected-show-all-headers <<\EOF 0001-Second.patch (mbox) Adding cc: A from line 'From: A ' +(mbox) Adding cc: One from line 'Cc: One , two@example.com' +(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com' Dry-OK. Log says: Server: relay.example.com MAIL FROM: -RCPT TO:, +RCPT TO:,,, From: Example To: to@example.com -Cc: A +Cc: A , One , two@example.com Subject: [PATCH 1/1] Second. Date: DATE-STRING Message-Id: MESSAGE-ID-STRING -- cgit v0.10.2-6-g49f6 From 3531e2703d8e441bfb4a6765459317b3db3f224c Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Sat, 14 Feb 2009 23:32:15 -0500 Subject: send-email: --suppress-cc improvements Since 6564828 (git-send-email: Generalize auto-cc recipient mechanism., 2007-12-25) we can suppress automatic Cc generation separately for each of the possible address sources. However, --suppress-cc=sob suppressed both SOB lines and body (but not header) Cc lines, contrary to the name. Change --suppress-cc=sob to mean only SOB lines, and add separate choices 'bodycc' (body Cc lines) and 'body' (both 'sob' and 'bodycc'). The option --no-signed-off-by-cc now acts like --suppress-cc=sob, which is not backwards compatible but matches the name of the option. Also update the documentation and add a few tests. Original patch by me. Revised by Thomas Rast, who contributed the documentation and test updates. Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index ff4aeff..164d149 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -164,14 +164,25 @@ Automating --suppress-cc:: Specify an additional category of recipients to suppress the - auto-cc of. 'self' will avoid including the sender, 'author' will - avoid including the patch author, 'cc' will avoid including anyone - mentioned in Cc lines in the patch, 'sob' will avoid including - anyone mentioned in Signed-off-by lines, and 'cccmd' will avoid - running the --cc-cmd. 'all' will suppress all auto cc values. - Default is the value of 'sendemail.suppresscc' configuration value; - if that is unspecified, default to 'self' if --suppress-from is - specified, as well as 'sob' if --no-signed-off-cc is specified. + auto-cc of: ++ +-- +- 'author' will avoid including the patch author +- 'self' will avoid including the sender +- 'cc' will avoid including anyone mentioned in Cc lines in the patch header + except for self (use 'self' for that). +- 'ccbody' will avoid including anyone mentioned in Cc lines in the + patch body (commit message) except for self (use 'self' for that). +- 'sob' will avoid including anyone mentioned in Signed-off-by lines except + for self (use 'self' for that). +- 'cccmd' will avoid running the --cc-cmd. +- 'body' is equivalent to 'sob' + 'ccbody' +- 'all' will suppress all auto cc values. +-- ++ +Default is the value of 'sendemail.suppresscc' configuration value; if +that is unspecified, default to 'self' if --suppress-from is +specified, as well as 'body' if --no-signed-off-cc is specified. --[no-]suppress-from:: If this is set, do not add the From: address to the cc: list. diff --git a/git-send-email.perl b/git-send-email.perl index a6efd1f..54e7617 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -68,9 +68,8 @@ git send-email [options] Automating: --identity * Use the sendemail. options. --cc-cmd * Email Cc: via ` \$patch_path` - --suppress-cc * author, self, sob, cccmd, all. - --[no-]signed-off-by-cc * Send to Cc: and Signed-off-by: - addresses. Default on. + --suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all. + --[no-]signed-off-by-cc * Send to Signed-off-by: addresses. Default on. --[no-]suppress-from * Send to self. Default off. --[no-]chain-reply-to * Chain In-Reply-To: fields. Default on. --[no-]thread * Use In-Reply-To: field. Default on. @@ -325,13 +324,13 @@ my(%suppress_cc); if (@suppress_cc) { foreach my $entry (@suppress_cc) { die "Unknown --suppress-cc field: '$entry'\n" - unless $entry =~ /^(all|cccmd|cc|author|self|sob)$/; + unless $entry =~ /^(all|cccmd|cc|author|self|sob|body|bodycc)$/; $suppress_cc{$entry} = 1; } } if ($suppress_cc{'all'}) { - foreach my $entry (qw (ccmd cc author self sob)) { + foreach my $entry (qw (ccmd cc author self sob body bodycc)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'all'}; @@ -341,6 +340,13 @@ if ($suppress_cc{'all'}) { $suppress_cc{'self'} = $suppress_from if defined $suppress_from; $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc; +if ($suppress_cc{'body'}) { + foreach my $entry (qw (sob bodycc)) { + $suppress_cc{$entry} = 1; + } + delete $suppress_cc{'body'}; +} + # Debugging, print out the suppressions. if (0) { print "suppressions:\n"; @@ -1020,13 +1026,17 @@ foreach my $t (@files) { while() { $message .= $_; if (/^(Signed-off-by|Cc): (.*)$/i) { - next if ($suppress_cc{'sob'}); chomp; - my $c = $2; + my ($what, $c) = ($1, $2); chomp $c; - next if ($c eq $sender and $suppress_cc{'self'}); + if ($c eq $sender) { + next if ($suppress_cc{'self'}); + } else { + next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i; + next if $suppress_cc{'bodycc'} and $what =~ /Cc/i; + } push @cc, $c; - printf("(sob) Adding cc: %s from line '%s'\n", + printf("(body) Adding cc: %s from line '%s'\n", $c, $_) unless $quiet; } } diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 63ab88b..4df4f96 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -32,11 +32,11 @@ clean_fake_sendmail() { } test_expect_success 'Extract patches' ' - patches=`git format-patch --cc="One " --cc=two@example.com -n HEAD^1` + patches=`git format-patch -s --cc="One " --cc=two@example.com -n HEAD^1` ' test_expect_success 'Send patches' ' - git send-email --from="Example " --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors + git send-email --suppress-cc=sob --from="Example " --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors ' cat >expected <<\EOF @@ -74,6 +74,7 @@ EOF test_expect_success 'Show all headers' ' git send-email \ --dry-run \ + --suppress-cc=sob \ --from="Example " \ --to=to@example.com \ --cc=cc@example.com \ @@ -193,7 +194,7 @@ test_expect_success 'second message is patch' ' grep "Subject:.*Second" msgtxt2 ' -cat >expected-show-all-headers <<\EOF +cat >expected-suppress-sob <<\EOF 0001-Second.patch (mbox) Adding cc: A from line 'From: A ' (mbox) Adding cc: One from line 'Cc: One , two@example.com' @@ -213,10 +214,10 @@ X-Mailer: X-MAILER-STRING Result: OK EOF -test_expect_success 'sendemail.cc set' ' - git config sendemail.cc cc@example.com && +test_suppression () { git send-email \ --dry-run \ + --suppress-cc=$1 \ --from="Example " \ --to=to@example.com \ --smtp-server relay.example.com \ @@ -224,11 +225,16 @@ test_expect_success 'sendemail.cc set' ' sed -e "s/^\(Date:\).*/\1 DATE-STRING/" \ -e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \ -e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \ - >actual-show-all-headers && - test_cmp expected-show-all-headers actual-show-all-headers + >actual-suppress-$1 && + test_cmp expected-suppress-$1 actual-suppress-$1 +} + +test_expect_success 'sendemail.cc set' ' + git config sendemail.cc cc@example.com && + test_suppression sob ' -cat >expected-show-all-headers <<\EOF +cat >expected-suppress-sob <<\EOF 0001-Second.patch (mbox) Adding cc: A from line 'From: A ' (mbox) Adding cc: One from line 'Cc: One , two@example.com' @@ -250,17 +256,123 @@ EOF test_expect_success 'sendemail.cc unset' ' git config --unset sendemail.cc && - git send-email \ - --dry-run \ - --from="Example " \ - --to=to@example.com \ - --smtp-server relay.example.com \ - $patches | - sed -e "s/^\(Date:\).*/\1 DATE-STRING/" \ - -e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \ - -e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \ - >actual-show-all-headers && - test_cmp expected-show-all-headers actual-show-all-headers + test_suppression sob +' + +cat >expected-suppress-all <<\EOF +0001-Second.patch +Dry-OK. Log says: +Server: relay.example.com +MAIL FROM: +RCPT TO: +From: Example +To: to@example.com +Subject: [PATCH 1/1] Second. +Date: DATE-STRING +Message-Id: MESSAGE-ID-STRING +X-Mailer: X-MAILER-STRING + +Result: OK +EOF + +test_expect_success '--suppress-cc=all' ' + test_suppression all +' + +cat >expected-suppress-body <<\EOF +0001-Second.patch +(mbox) Adding cc: A from line 'From: A ' +(mbox) Adding cc: One from line 'Cc: One , two@example.com' +(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com' +Dry-OK. Log says: +Server: relay.example.com +MAIL FROM: +RCPT TO:,,, +From: Example +To: to@example.com +Cc: A , One , two@example.com +Subject: [PATCH 1/1] Second. +Date: DATE-STRING +Message-Id: MESSAGE-ID-STRING +X-Mailer: X-MAILER-STRING + +Result: OK +EOF + +test_expect_success '--suppress-cc=body' ' + test_suppression body +' + +cat >expected-suppress-sob <<\EOF +0001-Second.patch +(mbox) Adding cc: A from line 'From: A ' +(mbox) Adding cc: One from line 'Cc: One , two@example.com' +(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com' +Dry-OK. Log says: +Server: relay.example.com +MAIL FROM: +RCPT TO:,,, +From: Example +To: to@example.com +Cc: A , One , two@example.com +Subject: [PATCH 1/1] Second. +Date: DATE-STRING +Message-Id: MESSAGE-ID-STRING +X-Mailer: X-MAILER-STRING + +Result: OK +EOF + +test_expect_success '--suppress-cc=sob' ' + test_suppression sob +' + +cat >expected-suppress-bodycc <<\EOF +0001-Second.patch +(mbox) Adding cc: A from line 'From: A ' +(mbox) Adding cc: One from line 'Cc: One , two@example.com' +(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com' +(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter ' +Dry-OK. Log says: +Server: relay.example.com +MAIL FROM: +RCPT TO:,,,, +From: Example +To: to@example.com +Cc: A , One , two@example.com, C O Mitter +Subject: [PATCH 1/1] Second. +Date: DATE-STRING +Message-Id: MESSAGE-ID-STRING +X-Mailer: X-MAILER-STRING + +Result: OK +EOF + +test_expect_success '--suppress-cc=bodycc' ' + test_suppression bodycc +' + +cat >expected-suppress-cc <<\EOF +0001-Second.patch +(mbox) Adding cc: A from line 'From: A ' +(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter ' +Dry-OK. Log says: +Server: relay.example.com +MAIL FROM: +RCPT TO:,, +From: Example +To: to@example.com +Cc: A , C O Mitter +Subject: [PATCH 1/1] Second. +Date: DATE-STRING +Message-Id: MESSAGE-ID-STRING +X-Mailer: X-MAILER-STRING + +Result: OK +EOF + +test_expect_success '--suppress-cc=cc' ' + test_suppression cc ' test_expect_success '--compose adds MIME for utf8 body' ' -- cgit v0.10.2-6-g49f6 From afe756c936334a5a374a8e0e8ee70a7f319dd71b Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Mon, 23 Feb 2009 13:51:37 -0500 Subject: send-email: don't create temporary compose file until it is needed Commit eed6ca7 caused a minor regression when it switched to using tempfile() to generate the temporary compose file. Since tempfile() creates the file at the time it generates the filename, zero-length temporary files are being left behind unless --compose is used (in which case the file is cleaned up). This patch fixes the regression by not calling tempfile() to generate the compose filename unless --compose is in use. Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano diff --git a/git-send-email.perl b/git-send-email.perl index 54e7617..adf7ecb 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -156,10 +156,7 @@ if ($@) { # Behavior modification variables my ($quiet, $dry_run) = (0, 0); my $format_patch; -my $compose_filename = ($repo ? - tempfile(".gitsendemail.msg.XXXXXX", DIR => $repo->repo_path()) : - tempfile(".gitsendemail.msg.XXXXXX", DIR => "."))[1]; - +my $compose_filename; # Handle interactive edition of files. my $multiedit; @@ -222,11 +219,13 @@ sub signal_handler { system "stty echo"; # tmp files from --compose - if (-e $compose_filename) { - print "'$compose_filename' contains an intermediate version of the email you were composing.\n"; - } - if (-e ($compose_filename . ".final")) { - print "'$compose_filename.final' contains the composed email.\n" + if (defined $compose_filename) { + if (-e $compose_filename) { + print "'$compose_filename' contains an intermediate version of the email you were composing.\n"; + } + if (-e ($compose_filename . ".final")) { + print "'$compose_filename.final' contains the composed email.\n" + } } exit; @@ -505,6 +504,9 @@ sub get_patch_subject($) { if ($compose) { # Note that this does not need to be secure, but we will make a small # effort to have it be unique + $compose_filename = ($repo ? + tempfile(".gitsendemail.msg.XXXXXX", DIR => $repo->repo_path()) : + tempfile(".gitsendemail.msg.XXXXXX", DIR => "."))[1]; open(C,">",$compose_filename) or die "Failed to open for writing $compose_filename: $!"; -- cgit v0.10.2-6-g49f6 From c1f2aa45b7bdb82c4378443ae23ad9625e782fe2 Mon Sep 17 00:00:00 2001 From: Jay Soffian Date: Mon, 2 Mar 2009 23:52:18 -0500 Subject: send-email: add --confirm option and configuration setting send-email violates the principle of least surprise by automatically cc'ing additional recipients without confirming this with the user. This patch teaches send-email a --confirm option. It takes the following values: --confirm=always always confirm before sending --confirm=never never confirm before sending --confirm=cc confirm before sending when send-email has automatically added addresses from the patch to the Cc list --confirm=compose confirm before sending the first message when using --compose. (Needed to maintain backwards compatibility with existing behavior.) --confirm=auto 'cc' + 'compose' If sendemail.confirm is unconfigured, the option defaults to 'compose' if any suppress-Cc related options have been used, otherwise it defaults to 'auto'. Unfortunately, it is impossible to introduce this patch such that it helps new users without potentially annoying some existing users. We attempt to mitigate the latter by: * Allowing the user to set 'git config sendemail.confirm never' * Allowing the user to say 'all' after the first prompt to not be prompted on remaining emails during the same invocation. * Telling the user about the 'sendemail.confirm' setting if it is unconfigured whenever we prompt due to Cc before sending. * Only prompting if no --suppress related options have been passed, as using such an option is likely to indicate an experienced send-email user. There is a slight fib in message informing the user of the sendemail.confirm setting and this is intentional. Setting 'auto' differs from leaving sendemail.confirm unset in two ways: 1) 'auto' obviously squelches the informational message; 2) 'auto' prompts when the Cc list has been expanded even in the presence of a --suppress related option, where leaving sendemail.confirm unset does not. This is intentional to keep the message simple, and to avoid adding another sendemail.confirm value ('auto-except-suppress'?). Signed-off-by: Jay Soffian Signed-off-by: Junio C Hamano diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 164d149..0335727 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -199,6 +199,22 @@ specified, as well as 'body' if --no-signed-off-cc is specified. Administering ~~~~~~~~~~~~~ +--confirm:: + Confirm just before sending: ++ +-- +- 'always' will always confirm before sending +- 'never' will never confirm before sending +- 'cc' will confirm before sending when send-email has automatically + added addresses from the patch to the Cc list +- 'compose' will confirm before sending the first message when using --compose. +- 'auto' is equivalent to 'cc' + 'compose' +-- ++ +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'. + --dry-run:: Do everything except actually send the emails. @@ -242,6 +258,11 @@ sendemail.multiedit:: summary when '--compose' is used). If false, files will be edited one after the other, spawning a new editor each time. +sendemail.confirm:: + Sets the default for whether to confirm before sending. Must be + one of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm' + in the previous section for the meaning of these values. + Author ------ diff --git a/git-send-email.perl b/git-send-email.perl index adf7ecb..57127aa 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -75,6 +75,8 @@ git send-email [options] --[no-]thread * Use In-Reply-To: field. Default on. Administering: + --confirm * Confirm recipients before sending; + auto, cc, compose, always, or never. --quiet * Output one line of info per email. --dry-run * Don't actually send the emails. --[no-]validate * Perform patch sanity checks. Default on. @@ -181,7 +183,7 @@ sub do_edit { my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc, $cc_cmd); my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_encryption); my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts); -my ($validate); +my ($validate, $confirm); my (@suppress_cc); my %config_bool_settings = ( @@ -207,6 +209,7 @@ my %config_settings = ( "suppresscc" => \@suppress_cc, "envelopesender" => \$envelope_sender, "multiedit" => \$multiedit, + "confirm" => \$confirm, ); # Handle Uncouth Termination @@ -258,6 +261,7 @@ my $rc = GetOptions("sender|from=s" => \$sender, "suppress-from!" => \$suppress_from, "suppress-cc=s" => \@suppress_cc, "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc, + "confirm=s" => \$confirm, "dry-run" => \$dry_run, "envelope-sender=s" => \$envelope_sender, "thread!" => \$thread, @@ -346,6 +350,14 @@ if ($suppress_cc{'body'}) { delete $suppress_cc{'body'}; } +# Set confirm's default value +my $confirm_unconfigured = !defined $confirm; +if ($confirm_unconfigured) { + $confirm = scalar %suppress_cc ? 'compose' : 'auto'; +}; +die "Unknown --confirm setting: '$confirm'\n" + unless $confirm =~ /^(?:auto|cc|compose|always|never)/; + # Debugging, print out the suppressions. if (0) { print "suppressions:\n"; @@ -663,25 +675,13 @@ if (!defined $smtp_server) { $smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug* } -if ($compose) { - while (1) { - $_ = $term->readline("Send this email? (y|n) "); - last if defined $_; - print "\n"; - } - - if (uc substr($_,0,1) ne 'Y') { - cleanup_compose_files(); - exit(0); - } - - if ($compose > 0) { - @files = ($compose_filename . ".final", @files); - } +if ($compose && $compose > 0) { + @files = ($compose_filename . ".final", @files); } # Variables we set as part of the loop over files -our ($message_id, %mail, $subject, $reply_to, $references, $message); +our ($message_id, %mail, $subject, $reply_to, $references, $message, + $needs_confirm, $message_num); sub extract_valid_address { my $address = shift; @@ -837,6 +837,37 @@ X-Mailer: git-send-email $gitversion unshift (@sendmail_parameters, '-f', $raw_from) if(defined $envelope_sender); + if ($needs_confirm && !$dry_run) { + print "\n$header\n"; + if ($needs_confirm eq "inform") { + $confirm_unconfigured = 0; # squelch this message for the rest of this run + print " The Cc list above has been expanded by additional\n"; + print " addresses found in the patch commit message. By default\n"; + print " send-email prompts before sending whenever this occurs.\n"; + print " This behavior is controlled by the sendemail.confirm\n"; + print " configuration setting.\n"; + print "\n"; + print " For additional information, run 'git send-email --help'.\n"; + print " To retain the current behavior, but squelch this message,\n"; + print " run 'git config --global sendemail.confirm auto'.\n\n"; + } + while (1) { + chomp ($_ = $term->readline( + "Send this email? ([y]es|[n]o|[q]uit|[a]ll): " + )); + last if /^(?:yes|y|no|n|quit|q|all|a)/i; + print "\n"; + } + if (/^n/i) { + return; + } elsif (/^q/i) { + cleanup_compose_files(); + exit(0); + } elsif (/^a/i) { + $confirm = 'never'; + } + } + if ($dry_run) { # We don't want to send the email. } elsif ($smtp_server =~ m#^/#) { @@ -935,6 +966,7 @@ X-Mailer: git-send-email $gitversion $reply_to = $initial_reply_to; $references = $initial_reply_to || ''; $subject = $initial_subject; +$message_num = 0; foreach my $t (@files) { open(F,"<",$t) or die "can't open file $t"; @@ -943,11 +975,12 @@ foreach my $t (@files) { my $author_encoding; my $has_content_type; my $body_encoding; - @cc = @initial_cc; + @cc = (); @xh = (); my $input_format = undef; my @header = (); $message = ""; + $message_num++; # First unfold multiline header fields while() { last if /^\s*$/; @@ -1080,6 +1113,14 @@ foreach my $t (@files) { } } + $needs_confirm = ( + $confirm eq "always" or + ($confirm =~ /^(?:auto|cc)$/ && @cc) or + ($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1)); + $needs_confirm = "inform" if ($needs_confirm && $confirm_unconfigured && @cc); + + @cc = (@initial_cc, @cc); + send_message(); # set up for the next message @@ -1094,13 +1135,10 @@ foreach my $t (@files) { $message_id = undef; } -if ($compose) { - cleanup_compose_files(); -} +cleanup_compose_files(); sub cleanup_compose_files() { - unlink($compose_filename, $compose_filename . ".final"); - + unlink($compose_filename, $compose_filename . ".final") if $compose; } $smtp->quit if $smtp; diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 4df4f96..08d5b91 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -35,6 +35,47 @@ test_expect_success 'Extract patches' ' patches=`git format-patch -s --cc="One " --cc=two@example.com -n HEAD^1` ' +# Test no confirm early to ensure remaining tests will not hang +test_no_confirm () { + rm -f no_confirm_okay + echo n | \ + GIT_SEND_EMAIL_NOTTY=1 \ + git send-email \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $@ \ + $patches > stdout && + test_must_fail grep "Send this email" stdout && + > no_confirm_okay +} + +# Exit immediately to prevent hang if a no-confirm test fails +check_no_confirm () { + test -f no_confirm_okay || { + say 'No confirm test failed; skipping remaining tests to prevent hanging' + test_done + } +} + +test_expect_success 'No confirm with --suppress-cc' ' + test_no_confirm --suppress-cc=sob +' +check_no_confirm + +test_expect_success 'No confirm with --confirm=never' ' + test_no_confirm --confirm=never +' +check_no_confirm + +# leave sendemail.confirm set to never after this so that none of the +# remaining tests prompt unintentionally. +test_expect_success 'No confirm with sendemail.confirm=never' ' + git config sendemail.confirm never && + test_no_confirm --compose --subject=foo +' +check_no_confirm + test_expect_success 'Send patches' ' git send-email --suppress-cc=sob --from="Example " --to=nobody@example.com --smtp-server="$(pwd)/fake.sendmail" $patches 2>errors ' @@ -175,15 +216,13 @@ test_set_editor "$(pwd)/fake-editor" test_expect_success '--compose works' ' clean_fake_sendmail && - echo y | \ - GIT_SEND_EMAIL_NOTTY=1 \ - git send-email \ - --compose --subject foo \ - --from="Example " \ - --to=nobody@example.com \ - --smtp-server="$(pwd)/fake.sendmail" \ - $patches \ - 2>errors + git send-email \ + --compose --subject foo \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $patches \ + 2>errors ' test_expect_success 'first message is compose text' ' @@ -375,15 +414,56 @@ test_expect_success '--suppress-cc=cc' ' test_suppression cc ' +test_confirm () { + echo y | \ + GIT_SEND_EMAIL_NOTTY=1 \ + git send-email \ + --from="Example " \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + $@ \ + $patches | grep "Send this email" +} + +test_expect_success '--confirm=always' ' + test_confirm --confirm=always --suppress-cc=all +' + +test_expect_success '--confirm=auto' ' + test_confirm --confirm=auto +' + +test_expect_success '--confirm=cc' ' + test_confirm --confirm=cc +' + +test_expect_success '--confirm=compose' ' + test_confirm --confirm=compose --compose +' + +test_expect_success 'confirm by default (due to cc)' ' + CONFIRM=$(git config --get sendemail.confirm) && + git config --unset sendemail.confirm && + test_confirm && + git config sendemail.confirm $CONFIRM +' + +test_expect_success 'confirm by default (due to --compose)' ' + CONFIRM=$(git config --get sendemail.confirm) && + git config --unset sendemail.confirm && + test_confirm --suppress-cc=all --compose + ret="$?" + git config sendemail.confirm ${CONFIRM:-never} + test $ret = "0" +' + test_expect_success '--compose adds MIME for utf8 body' ' clean_fake_sendmail && (echo "#!$SHELL_PATH" && echo "echo utf8 body: àéìöú >>\"\$1\"" ) >fake-editor-utf8 && chmod +x fake-editor-utf8 && - echo y | \ GIT_EDITOR="\"$(pwd)/fake-editor-utf8\"" \ - GIT_SEND_EMAIL_NOTTY=1 \ git send-email \ --compose --subject foo \ --from="Example " \ @@ -405,9 +485,7 @@ test_expect_success '--compose respects user mime type' ' echo " echo utf8 body: àéìöú) >\"\$1\"" ) >fake-editor-utf8-mime && chmod +x fake-editor-utf8-mime && - echo y | \ GIT_EDITOR="\"$(pwd)/fake-editor-utf8-mime\"" \ - GIT_SEND_EMAIL_NOTTY=1 \ git send-email \ --compose --subject foo \ --from="Example " \ @@ -421,9 +499,7 @@ test_expect_success '--compose respects user mime type' ' test_expect_success '--compose adds MIME for utf8 subject' ' clean_fake_sendmail && - echo y | \ GIT_EDITOR="\"$(pwd)/fake-editor\"" \ - GIT_SEND_EMAIL_NOTTY=1 \ git send-email \ --compose --subject utf8-sübjëct \ --from="Example " \ @@ -445,7 +521,7 @@ test_expect_success 'detects ambiguous reference/file conflict' ' test_expect_success 'feed two files' ' rm -fr outdir && git format-patch -2 -o outdir && - GIT_SEND_EMAIL_NOTTY=1 git send-email \ + git send-email \ --dry-run \ --from="Example " \ --to=nobody@example.com \ -- cgit v0.10.2-6-g49f6