Add git-send-email-script - tool to send emails from git-format-patch-script
Subsystems (2)
PATCH 1/3 Add git-send-email-script - tool to send emails from git-format-patch-script
This is based off of GregKH's script, send-lots-of-email.pl, and strives to do all the nice things a good subsystem maintainer does when forwarding a patch or 50 upstream:
All the prior handlers of the patch, as determined by the
Signed-off-by: lines, and/or the author of the commit, are cc:ed on the email.
All emails are sent as a reply to the previous email, making it easy to skip a collection of emails that are uninteresting.
---
Makefile | 2 git-send-email-script | 265 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+), 1 deletions(-) create mode 100755 git-send-email-script
55d4b5b7a11448d60eb00b5a7081954663842b06
Makefile +1 -1
@@ -62,7 +62,7 @@ SCRIPTS=git git-apply-patch-script git-m git-format-patch-script git-sh-setup-script git-push-script \ git-branch-script git-parse-remote git-verify-tag-script \ git-ls-remote-script git-clone-dumb-http git-rename-script \ - git-request-pull-script + git-request-pull-script git-send-email-script PROG= git-update-cache git-diff-files git-init-db git-write-tree \ git-read-tree git-commit-tree git-cat-file git-fsck-cache \
git-send-email-script new file +265 -0
265 lines not on this page · raw message
PATCH 2/3 Add documentation for git-send-email-script
---
Documentation/git-send-email-script.txt | 61 +++++++++++++++++++++++++++++++ 1 files changed, 61 insertions(+), 0 deletions(-) create mode 100644 Documentation/git-send-email-script.txt
799a6320d3b07347869093beec303afbc005cf26
Documentation/git-send-email-script.txt new file +61 -0
@@ -0,0 +1,61 @@ +git-send-email-script(1) +======================= +v0.1, July 2005 + +NAME +---- +git-send-email-script - Send a collection of patches as emails + + +SYNOPSIS +-------- +'git-send-email-script' [options] <file|directory> [... file|directory] + + + +DESCRIPTION +----------- +Takes the patches given on the command line and emails them out. + +The header of the email is configurable by command line options. If not +specified on the command line, the user will be prompted with a ReadLine +enabled interface to provide the necessary information. + +The options available are: + + --to + Specify the primary recipient of the emails generated. + Generally, this will be the upstream maintainer of the + project involved. + + --from + Specify the sender of the emails. This will default to + the value GIT_COMMITTER_IDENT, as returned by "git-var -l". + The user will still be prompted to confirm this entry. + + --subject + Specify the initial subject of the email thread. + + --in-reply-to + Specify the contents of the first In-Reply-To header. + Subsequent emails will refer to the previous email + instead of this. + When overriding on the command line, it may be necessary + to set this to a space. For example + --in-reply-to=" " + +Author +------ +Written by Ryan Anderson <[email protected]> + +git-send-email-script is originally based upon +send_lots_of_email.pl by Greg Kroah-Hartman. + +Documentation +-------------- +Documentation by Ryan Anderson + +GIT +--- +Part of the link:git.html[git] suite +
PATCH 3/3 Add new dependencies caused by git-send-email-script to debian/control
---
debian/control | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
6dbf602b0931608831888e779612fcc89b90d16f
debian/control +1 -1
@@ -8,7 +8,7 @@ Standards-Version: 3.6.1 Package: git-core Architecture: any Depends: ${shlibs:Depends}, ${misc:Depends}, patch, diff, rcs -Recommends: rsync, curl, ssh +Recommends: rsync, curl, ssh, libmail-sendmail-perl, libemail-valid-perl Conflicts: git Description: The git content addressable filesystem GIT comes in two layers. The bottom layer is merely an extremely fast
Quoted 4 lines · Ryan Anderson
This is based off of GregKH's script, send-lots-of-email.pl, and strives to do all the nice things a good subsystem maintainer does when forwarding a patch or 50 upstream:
Quoted 8 lines
All the prior handlers of the patch, as determined by the
Signed-off-by: lines, and/or the author of the commit, are cc:ed on the email.All emails are sent as a reply to the previous email, making it easy to skip a collection of emails that are uninteresting.
Signed-off-by Ryan Anderson <[email protected]>
And yes, I did generate this thread with this script - so I have proof that it works nicely.
Actually, with this:
git format-patch -n --mbox -o ../pending/ origin git send-email ../pending/
<answer some questions>
In 2-3 minutes I should have my git tree at rsync://h4x0r5.com/git-ryan.git/ updated with these changes, and my SYNOPSIS draft. (I haven't added the short tutorial that Sam suggested yet, that's a task for another 4am hackfest.)
Signature · 2 lines
Ryan Anderson sometimes Pug Majere
Quoted 2 lines · Ryan Anderson
And yes, I did generate this thread with this script - so I have proof that it works nicely.
It might make sense to create a "Patch 0/N" with a short explanation, and have the actual patches be replies to that -- or to patch 1/N if that's not necessary.
As it is, patch N hangs off patch N-1 in my email threading view, which gets slightly cumbersome if N>10.
Signature · 4 lines
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | [email protected]
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
Nothing makes a person more productive than the last minute.Quoted 10 lines · On Sun, 31 Jul 2005 04:17:25 -0400 Ryan Anderson wrote:
This is based off of GregKH's script, send-lots-of-email.pl, and strives to do all the nice things a good subsystem maintainer does when forwarding a patch or 50 upstream:
All the prior handlers of the patch, as determined by the
Signed-off-by: lines, and/or the author of the commit, are cc:ed on the email.All emails are sent as a reply to the previous email, making it easy to skip a collection of emails that are uninteresting.
Actually, this is the part of GregKH's script which I hate ;)
50 patches sent this way produce an enormous email thread; if someone then tries to comment on a 40th patch, this part of the thread ends up far behind the right edge of the message list window in almost any mail client (and if someone comments on the first patch, the comments are far from that patch).
Sending a [PATCH 0/N] message with the overall description of the patchset and then all patches as replies to that message looks much better in that respect. However, missing messages in this form are less obvious.
Quoted 2 lines · Ryan Anderson
All emails are sent as a reply to the previous email, making it easy to skip a collection of emails that are uninteresting.
I understand why _some_ people consider this preferable, but wonder if this should have a knob to be tweaked.
For example, I myself often find it very hard to read when a cascading thread goes very deep like this:
[PATCH 0/9] cover
[PATCH 1/9] first one
[PATCH 2/9] second one
[PATCH 3/9] third one in the series
...
and prefer to see this instead (this assumes your MUA is half-way decent and lets you sort by subject):
[PATCH 0/9] cover
[PATCH 1/9] first one
[PATCH 2/9] second one
[PATCH 3/9] third one in the series
...
git-send-email-script line 5
Quoted 4 lines
git-send-email-script new file +4
-0+# horrible hack of a script to send off a large number of email messages, one after +# each other, all chained together. This is useful for large numbers of patches. +# +# Use at your own risk!!!!
Well, if it is "Use at your own risk" maybe it should stay outside the official distribution for a while until it gets safer ;-).
git-send-email-script line 68
Quoted 2 lines
git-send-email-script new file +2
-0+ my @fields = split /\s+/, $data; + my $ident = join(" ", @fields[0...(@fields-3)]);
Wouldn't "s/>.*/>/" be easier than splitting and joining?
git-send-email-script line 82
Quoted 4 lines
git-send-email-script new file +4
-0+if (!defined $from) { + $from = $author || $committer; + 1 while (!defined ($_ = $term->readline("Who should the emails appear to be from? ", + $from)));
Judging from your past patches, you seem to really like statement modifiers[*]. While they _are_ valid Perl constructs, it is extremely hard to read when used outside very simple idiomatic use. Please consider rewriting the above and the like using compound statements[*] (I am using these terms according to the definition in perlsyn.pod). Remember, there are people Perl is not their native language, but are intelligent enough to be of great help fixing problems in programs you write in Perl. To most of them, compound statements are more familiar, so try to be gentle to them.
git-send-email-script line 116
Quoted 4 lines
git-send-email-script new file +4
-0+ opendir(DH,$f) + or die "Failed to opendir $f: $!"; + push @files, map { +$f . "/" . $_ } grep !/^\.{1,2}$/, + sort readdir(DH);
Maybe skip potential subdirs while you are at it, something like this?
push @files, sort grep { -f $_ } map { "$f/$_" } readdir(DH)git-send-email-script line 157
Quoted 3 lines
git-send-email-script new file +3
-0+ my $pseudo_rand = int (rand(4200)); + $message_id = "<$date$pseudo_rand\@foobar.com>"; + print "new message id = $message_id\n";
I doubt this hardcoded foobar.com is a good idea. Did you mean to print it, by the way?
git-send-email-script line 169
Quoted 2 lines
git-send-email-script new file +2
-0+ $to{lc(Email::Valid->address($_))}++ for (@to); + my $to = join(",", keys %to);
Is this the culprit that produced this mechanical-looking line?
To: [email protected],[email protected]
Interestingly enough, you do not seem to do it for the From:
line.
From: Ryan Anderson <[email protected]>
Also you seem to be losing the ordering in @to and @cc by the use of uniquefying "keys %to" and "keys %cc". I can not offhand tell if it matters, but you probably would care, at least for the primary recipients listed in @to array.
git-send-email-script line 182
Quoted 1 line
git-send-email-script new file +1
-0+ $mail{smtp} = 'localhost';
I suspect this probably need to be configurable. I may be a minority, but my outgoing messages are directly handed to my local ISP smtp server from my MUA, and the smtp server running on the locahost does not talk to the outside world.
git-send-email-script line 262
Quoted 2 lines
git-send-email-script new file +2
-0+ # set up for the next message + $reply_to = $message_id;
Making chaining policy configurable would be just one liner change here, I suppose.
Since there are always 47 different ways to do the same thing in Perl, Perl style varies a lot more than Shell style which in turn varies a lot more than C. You should be prepared to be nitpicked a lot when you post Perl ;-). And I was in nitpicking mood tonight.
Thanks for the patch. Overall, very good intent. Slightly troublesome details.
-jc
Quoted 7 lines · Junio C Hamano
Quoted 2 lines · Ryan Anderson
All emails are sent as a reply to the previous email, making it easy to skip a collection of emails that are uninteresting.
I understand why _some_ people consider this preferable, but wonder if this should have a knob to be tweaked.
Hmm, fair enough.
I'll send a few more patches in a minute that deal with the things in this email but for now:
--chain-reply-to (or --no-chain-reply-to)
Will toggle between these two behaviors. (This will not be prompted for by the ReadLine interface, btw.)
Quoted 8 lines
git-send-email-script line 5
Quoted 4 lines
git-send-email-script new file +4
-0+# horrible hack of a script to send off a large number of email messages, one after +# each other, all chained together. This is useful for large numbers of patches. +# +# Use at your own risk!!!!Well, if it is "Use at your own risk" maybe it should stay outside the official distribution for a while until it gets safer ;-).
Heh. I missed some comments that I meant to clean up that were in Greg's original script. One of the patches will clean up the comments.
Quoted 4 lines
git-send-email-script line 68
Quoted 2 lines
git-send-email-script new file +2
-0+ my @fields = split /\s+/, $data; + my $ident = join(" ", @fields[0...(@fields-3)]);Wouldn't "s/>.*/>/" be easier than splitting and joining?
Most of GIT_COMMITTER_IDENT (and GIT_AUTHOR_IDENT) is use controllable, except for the "date" section of it. I know we delimit that with spaces, so the above is guaranteed to work unless we change the format that git-var returns.
If I hope that nobody has done something like:
GIT_AUTHOR="Ryan <> Anderson"
GIT_AUTHOR_EMAIL="[email protected]"
I get more confusing results. (I suddenly have to think about what that regular expression does in this case - and I'm pretty sure that the one you gave would do bad things.)
Probably the best fix for this would be to take libgit.a, make a shared library out of it, and then interface the Perl scripts directly with it via a .xs module. I was thinking that I'd rather have direct access to the git_ident* functions than calling out to git-var, anyway. Consider that a plan for a revamp after the core seems to have settled down a bit more.
Quoted 7 lines
git-send-email-script line 82
Quoted 4 lines
git-send-email-script new file +4
-0+if (!defined $from) { + $from = $author || $committer; + 1 while (!defined ($_ = $term->readline("Who should the emails appear to be from? ", + $from)));Judging from your past patches, you seem to really like statement modifiers[*]. While they _are_ valid Perl constructs,
Quoted 8 lines
it is extremely hard to read when used outside very simple idiomatic use. Please consider rewriting the above and the like using compound statements[*] (I am using these terms according to the definition in perlsyn.pod). Remember, there are people Perl is not their native language, but are intelligent enough to be of great help fixing problems in programs you write in Perl. To most of them, compound statements are more familiar, so try to be gentle to them.
I copied this from another program of mine, and I'm *sure* I copied the style directly from a ReadLine example. But, I can't find a current example that says this is good, so, I'll fix this, too. It is rather ugly. (The other uses of this style are... less bad, IMO, than this abuse here.)
Quoted 9 lines
git-send-email-script line 116
Quoted 5 lines
git-send-email-script new file +4
-0+ opendir(DH,$f) + or die "Failed to opendir $f: $!"; + push @files, map { +$f . "/" . $_ } grep !/^\.{1,2}$/, + sort readdir(DH);Maybe skip potential subdirs while you are at it, something like this?
push @files, sort grep { -f $_ } map { "$f/$_" } readdir(DH)
Good point. One one hand I'd say, "Let it break for people who do strange things like that", but I'll make it safer anyway.
(Someone is going to reply and ask for it to recurse into subdirectories now. Maybe Andrew Morton would find that useful with his rather massive collection of patches in -mm kernels. But that's a feature for next week.)
Quoted 6 lines
git-send-email-script line 157
Quoted 3 lines
git-send-email-script new file +3
-0+ my $pseudo_rand = int (rand(4200)); + $message_id = "<$date$pseudo_rand\@foobar.com>"; + print "new message id = $message_id\n";I doubt this hardcoded foobar.com is a good idea. Did you mean to print it, by the way?
I'll convert this to something that is based off the $from address instead. It's probably better that way, anyway.
Quoted 6 lines
git-send-email-script line 169
Quoted 2 lines
git-send-email-script new file +2
-0+ $to{lc(Email::Valid->address($_))}++ for (@to); + my $to = join(",", keys %to);Is this the culprit that produced this mechanical-looking line?
No, that line was exactly what I put into the readline entry.
Quoted 9 lines
Interestingly enough, you do not seem to do it for the From:
line.From: Ryan Anderson <[email protected]>
Also you seem to be losing the ordering in @to and @cc by the use of uniquefying "keys %to" and "keys %cc". I can not offhand tell if it matters, but you probably would care, at least for the primary recipients listed in @to array.
Well, it was kind of annoying to see the same email address appear 2-3 times in the email, because of the way I pull in all the relevant emails from various places. So I really needed a way to cull the duplicates. I don't believe ordering is really significant in To: or Cc: lines, for really anyone. I could do soemthing like this, instead, I suppose:
my @clean_to = ();
my %dupe_check_to = ();
foreach my $to_entry (@to) {
if (!$dupe_check_to{Email::Valid->address($to_entry)}++) {
push @clean_to, $to_entry;
}
}
my $to = join(", ", @clean_to);I just like the first one a little better (though, I can't really pin down why).
Quoted 6 lines
git-send-email-script line 182
Quoted 1 line
git-send-email-script new file +1
-0+ $mail{smtp} = 'localhost';I suspect this probably need to be configurable. I may be a minority, but my outgoing messages are directly handed to my local ISP smtp server from my MUA, and the smtp server running on the locahost does not talk to the outside world.
Oddly, I think this is even the wrong setting after having read over the Mail::Sendmail docs to figure out how to change the MIME settings.
Quoted 5 lines
Since there are always 47 different ways to do the same thing in Perl, Perl style varies a lot more than Shell style which in turn varies a lot more than C. You should be prepared to be nitpicked a lot when you post Perl ;-). And I was in nitpicking mood tonight.
It's Perl. There does seem to be a lot of nitpicking when people write Perl.
(If you write it in idiomatic Perl, the non-Perl users hate you. If you write it like C, the Perl users complain that you write it like C. *sigh*)
Quoted 2 lines
Thanks for the patch. Overall, very good intent. Slightly troublesome details.
Thanks.
I'll have a series of patches out in a few minutes that will update what I originally sent to fix your issues here.
Signature · 2 lines
Ryan Anderson sometimes Pug Majere
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [OT] Perl-ish perl vs. C-ish perl.
Quoted 1 line · Junio C Hamano
Noel Maddy <[email protected]> writes:
(silly perl stuff)
Quoted 2 lines
Please refrain from making this thread "I know more Perl than you do"; thank you.
Sorry. Just trying to help, but suitably chastened.
Thanks
Signature · 4 lines
Time is an illusion. Lunchtime doubly so. -- Ford Prefect +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ Noel Maddy <[email protected]>
Re: [OT] Perl-ish perl vs. C-ish perl.
Quoted 4 lines · Noel Maddy
Quoted 2 lines
Please refrain from making this thread "I know more Perl than you do"; thank you.
Sorry. Just trying to help, but suitably chastened.
I realize that what you sent was not _too_ Perlish and being helpful. If you feel I overreacted, I am sorry; I _do_ think I did overreact, attempting to be preemptive.
Re: [OT] Perl-ish perl vs. C-ish perl.
Quoted 10 lines · Junio C Hamano
Quoted 4 lines · Noel Maddy
Quoted 2 lines
Please refrain from making this thread "I know more Perl than you do"; thank you.
Sorry. Just trying to help, but suitably chastened.
I realize that what you sent was not _too_ Perlish and being helpful. If you feel I overreacted, I am sorry; I _do_ think I did overreact, attempting to be preemptive.
No, I don't think you overreacted. Well, maybe a _little_ bit. ;)
I think you're focused on making git good (as in reliable and maintainable), and that strong focus is admirable.
Your clarification on the expected developer profile, and how to target the code to those developers helps, too.
For those of us who've spent years living in Perl, the idioms come much easier than those who are coming from other languages like C. But if the expected maintainers are not perl weenies (like me, I guess), then you're right, it's better to stay away from the more perl-y stuffy.
Again, thanks.
Quoted 1 line
Signature · 5 lines
If we can't define the user experience of Windows so that all Windows machines operate the same way, then the Windows brand is meaningless. -- Bill Gates +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+ Noel Maddy <[email protected]>
Quoted 5 lines · Ryan Anderson
If I hope that nobody has done something like:
GIT_AUTHOR="Ryan <> Anderson"
GIT_AUTHOR_EMAIL="[email protected]"I get more confusing results.
The function git_author_info() would remove <> from the above GIT_AUTHOR_* environment values by calling ident.c:copy(), so I think you would get more-or-less what you _should_ expect without getting confused.
$ GIT_AUTHOR_NAME="Ryan <> Anderson" \
GIT_AUTHOR_EMAIL="[email protected]" git-var -l
GIT_COMMITTER_IDENT=Junio C Hamano <[email protected]> 1122855849 -0700
GIT_AUTHOR_IDENT=Ryan Anderson <[email protected]> 1122855849 -0700Quoted 5 lines
Quoted 3 lines
Is this the culprit that produced this mechanical-looking line?
No, that line was exactly what I put into the readline entry.
I was mostly talking about Email::Valid seeming to be stripping out the display-name[*] part and keeping only addr-spec[*] part, like this:
$ cat j.perl
#!/usr/bin/perl
use Email::Valid;
for ('Junio C Hamano <[email protected]>',
'Ryan Anderson <[email protected]>') {
print $_, " => ", lc(Email::Valid->address($_)), "\n";
}
$ perl j.perl
Junio C Hamano <[email protected]> => [email protected]
Ryan Anderson <[email protected]> => [email protected]Also, I wonder if running lc() to downcase the local-part[*] is safe/allowed/correct; domain[*] part is case insensitive and should be OK to downcase, though.
Quoted 3 lines
..., because of the way I pull in all the relevant emails from various places. So I really needed a way to cull the duplicates. ... I could do soemthing like this, instead, I suppose:
I understand your needs, and you can make it a "sub filter_dups", which I think would make things a lot more pleasant to read.
[Footnote]
Terms marked [*] are from RFC2822, section 3.4.
Quoted 2 lines · Junio C Hamano
Also, I wonder if running
lc()to downcase the local-part[*] is safe/allowed/correct
mostly/no/no.
It's unlikely to be a real-life problem, but we still shouldn't do it.
Signature · 6 lines
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | [email protected]
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
Amusements to virtue are like breezes of air to the flame -- gentle ones will
fan it, but strong ones will put it out.
-- David ThomasQuoted 17 lines · Junio C Hamano
Quoted 2 lines · Ryan Anderson
All emails are sent as a reply to the previous email, making it easy to skip a collection of emails that are uninteresting.
I understand why _some_ people consider this preferable, but wonder if this should have a knob to be tweaked.
For example, I myself often find it very hard to read when a cascading thread goes very deep like this:
[PATCH 0/9] cover
[PATCH 1/9] first one
[PATCH 2/9] second one
[PATCH 3/9] third one in the series
...
Quoted 8 lines
and prefer to see this instead (this assumes your MUA is half-way decent and lets you sort by subject):
[PATCH 0/9] cover
[PATCH 1/9] first one
[PATCH 2/9] second one
[PATCH 3/9] third one in the series
...
In testing this, I think I figured out *why* the former is preferred - it guarantees the sorting when in the standard mode of "order by date".
Since these emails are sent *very* fast, delivery order tends to be the dominating factor in how they sort in your inbox, as they will all have the same time. So that's a trifle annoying.
Yes, sorting by subject will fix it, but "threaded + subject" is actually something I'm not sure much of anything supports. (It's not instantly obvious how to do it in mutt, for example - though, admittedly, Mutt may very well have a way to do it.)
I noticed this while testing my 2 new fixes - those will sent in a few seconds.
Signature · 2 lines
Ryan Anderson sometimes Pug Majere
Quoted 3 lines · Ryan Anderson
Since these emails are sent *very* fast, delivery order tends to be the dominating factor in how they sort in your inbox, as they will all have the same time. So that's a trifle annoying.
That's trivially fixable: just generate your own Date: header and add a second for each email.
Signature · 4 lines
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | [email protected]
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
Be careful whilst playing under the anvil tree.Oh, another thing. Could you refrain from doing
quoted-printable when possible? Thanks.
Hi,
wouldn't it be a good idea to make $from and $to required parameters? At least you could infer a sensible default of $from from GIT_* environment variables, no? I am not quite comfortable with a hard coded sender in a script possibly deployed into a multi-user environment.
Ciao,
Dscho
Quoted 6 lines · Johannes Schindelin
Hi,
wouldn't it be a good idea to make $from and $to required parameters? At least you could infer a sensible default of $from from GIT_* environment variables, no? I am not quite comfortable with a hard coded sender in a script possibly deployed into a multi-user environment.
The sender isn't hardcoded.
If it isn't sent on the command line, the return value from git-var -l is used to set a default, which is then confirmed by the ReadLine interface.
Some of the comments were leftover from Greg's original, before I remembered to remove them - it should be better now if you take another look. (See my, soon to be sent, set of additional changes.)
Signature · 2 lines
Ryan Anderson sometimes Pug Majere