threads / patch / 32078

patch, 2 partssend-email: new series-cc-cmd option

Subject: [PATCH 0/2] send-email: new series-cc-cmd option

## tl;dr

11 messages between Nov 11, 2012 and Nov 13, 2012. Diffs are folded; open one to read it.

replies: 10people: 4as markdown or json

Felipe Contreras· Nov 11, 2012, 17:04 UTC · lore

cc-cmd is probably not that useful, what we really want is to make a list of people to Cc on a per-patch-series basis.

This patch series allows just that.
Felipe Contreras (2):
  send-email: refactor recipients_cmd()
  send-email: add series-cc-cmd option
 Documentation/git-send-email.txt       |  7 +++++++
 contrib/completion/git-completion.bash |  2 +-
 git-send-email.perl                    | 13 ++++++++++---
 3 files changed, 18 insertions(+), 4 deletions(-)
-- 
1.8.0
Felipe Contreras· Nov 11, 2012, 17:04 UTC · re: Felipe Contreras · lore

[PATCH 1/2] send-email: refactor recipients_cmd()

If we pass the full command to run, we should be able to use it in more extensible ways--see next patch.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 git-send-email.perl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to git-send-email.perl +2 −2
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..26d4477 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1373,11 +1373,11 @@ foreach my $t (@files) {
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
-	my ($prefix, $what, $cmd, $file) = @_;
+	my ($prefix, $what, $cmd, @args) = @_;
 
 	my $sanitized_sender = sanitize_address($sender);
 	my @addresses = ();
-	open my $fh, "$cmd \Q$file\E |"
+	open my $fh, "-|", $cmd, @args
 	    or die "($prefix) Could not execute '$cmd'";
 	while (my $address = <$fh>) {
 		$address =~ s/^\s*//g;
-- 
1.8.0
Felipe Contreras· Nov 11, 2012, 17:04 UTC · re: Felipe Contreras · lore

[PATCH 2/2] send-email: add series-cc-cmd option

cc-cmd is only per-file, and many times receipients get lost without seing the full patch series.

So, add an option for series-cc-cmd, which receives as an argument rev-list options, just like format-patch.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-send-email.txt       | 7 +++++++
 contrib/completion/git-completion.bash | 2 +-
 git-send-email.perl                    | 9 ++++++++-
 3 files changed, 16 insertions(+), 2 deletions(-)
Show changes to 3 files +16 −2

Documentation/git-send-email.txt, contrib/completion/git-completion.bash, git-send-email.perl

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3241170..1161d3e 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -218,6 +218,13 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of 'sendemail.cccmd' configuration value.
 
+--series-cc-cmd=<command>::
+	Specify a command to execute to generate "Cc:" entries for the whole
+	patch series.
+	The arguments would be the same rev-list options the user specified.
+	Output of this command must be single email address per line.
+	Default is the value of 'sendemail.seriescccmd' configuration value.
+
 --[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index be800e0..0b04229 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1555,7 +1555,7 @@ _git_send_email ()
 		return
 		;;
 	--*)
-		__gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
+		__gitcomp "--annotate --bcc --cc --cc-cmd --series-cc-cmd --chain-reply-to
 			--compose --confirm= --dry-run --envelope-sender
 			--from --identity
 			--in-reply-to --no-chain-reply-to --no-signed-off-by-cc
diff --git a/git-send-email.perl b/git-send-email.perl
index 26d4477..da89ac6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -191,7 +191,7 @@ sub do_edit {
 
 # Variables with corresponding config settings
 my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
-my ($to_cmd, $cc_cmd);
+my ($to_cmd, $cc_cmd, $series_cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
@@ -224,6 +224,7 @@ my %config_settings = (
     "tocmd" => \$to_cmd,
     "cc" => \@initial_cc,
     "cccmd" => \$cc_cmd,
+    "seriescccmd" => \$series_cc_cmd,
     "aliasfiletype" => \$aliasfiletype,
     "bcc" => \@bcclist,
     "suppresscc" => \@suppress_cc,
@@ -305,6 +306,7 @@ my $rc = GetOptions("h" => \$help,
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
+		    "series-cc-cmd=s" => \$series_cc_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "suppress-cc=s" => \@suppress_cc,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_by_cc,
@@ -766,6 +768,11 @@ if (!@initial_to && !defined $to_cmd) {
 	$prompting++;
 }
 
+if (@rev_list_opts) {
+	push @initial_cc, recipients_cmd("series-cc-cmd", "cc", $series_cc_cmd, @rev_list_opts)
+		if defined $series_cc_cmd;
+}
+
 sub expand_aliases {
 	return map { expand_one_alias($_) } @_;
 }
-- 
1.8.0
Joe Perches· Nov 12, 2012, 22:52 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
Show 9 quoted lines
> Felipe Contreras wrote:
> > cc-cmd is only per-file, and many times receipients get lost without
> > seing the full patch series.
> 
> s/seing/seeing
> 
> > [...]
> 
> Looks good otherwise.
s/receipients/recipients/ too
Practically this is ok but I think it's unnecessary.

Output from git format-patch is always in a single directory.

My work flow is to use a script for --to and --cc lines that can be set to emit the same addresses for all files in a patch series or generate different addresses per patch file.

Felipe Contreras· Nov 12, 2012, 23:03 UTC · re: Joe Perches · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
Show 17 quoted lines
> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> Felipe Contreras wrote:
>> > cc-cmd is only per-file, and many times receipients get lost without
>> > seing the full patch series.
>>
>> s/seing/seeing
>>
>> > [...]
>>
>> Looks good otherwise.
>
> s/receipients/recipients/ too
>
> Practically this is ok but I think it's unnecessary.
>
> Output from git format-patch is always in a single
> directory.
A temporary directory.
> My work flow is to use a script for --to and --cc
> lines that can be set to emit the same addresses for
> all files in a patch series or generate different
> addresses per patch file.

For --to-cmd and --cc-cmd? So basically you check the dirname of the argument passed?

While that works, it means you have to run the same command multiple times, one for each mail.

If the command is using something expensive such as 'git blame' and you have many patches, this is particularly bad. Also, it's not elegant :)

-- 
Felipe Contreras
Joe Perches· Nov 12, 2012, 23:13 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
Show 28 quoted lines
> On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
> >> Felipe Contreras wrote:
> >> > cc-cmd is only per-file, and many times receipients get lost without
> >> > seing the full patch series.
> >>
> >> s/seing/seeing
> >>
> >> > [...]
> >>
> >> Looks good otherwise.
> >
> > s/receipients/recipients/ too
> >
> > Practically this is ok but I think it's unnecessary.
> >
> > Output from git format-patch is always in a single
> > directory.
> 
> A temporary directory.
> 
> > My work flow is to use a script for --to and --cc
> > lines that can be set to emit the same addresses for
> > all files in a patch series or generate different
> > addresses per patch file.
> 
> For --to-cmd and --cc-cmd? So basically you check the dirname of the
> argument passed?
yes. basename and dirname
> While that works, it means you have to run the same command multiple
> times, one for each mail.

Shrug. it's not a generally significant cost. The script could also output the addresses to yet another file.

> If the command is using something expensive such as 'git blame' and
> you have many patches, this is particularly bad. Also, it's not
> elegant :)
Elegant is a beholder viewpoint.
cheers, Joe
Felipe Contreras· Nov 12, 2012, 23:37 UTC · re: Joe Perches · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

On Tue, Nov 13, 2012 at 12:13 AM, Joe Perches <joe@perches.com> wrote:
Show 31 quoted lines
> On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
>> On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches <joe@perches.com> wrote:
>> > On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> >> Felipe Contreras wrote:
>> >> > cc-cmd is only per-file, and many times receipients get lost without
>> >> > seing the full patch series.
>> >>
>> >> s/seing/seeing
>> >>
>> >> > [...]
>> >>
>> >> Looks good otherwise.
>> >
>> > s/receipients/recipients/ too
>> >
>> > Practically this is ok but I think it's unnecessary.
>> >
>> > Output from git format-patch is always in a single
>> > directory.
>>
>> A temporary directory.
>>
>> > My work flow is to use a script for --to and --cc
>> > lines that can be set to emit the same addresses for
>> > all files in a patch series or generate different
>> > addresses per patch file.
>>
>> For --to-cmd and --cc-cmd? So basically you check the dirname of the
>> argument passed?
>
> yes. basename and dirname

Well, the basename is irrelevant, because you don't care witch particular patch is being sent, you are going to process all of them every time.

>> While that works, it means you have to run the same command multiple
>> times, one for each mail.
>
> Shrug.  it's not a generally significant cost.
It is when you use 'git blame' and there's a lot of history:

% time git cc-cmd -1 1266686 Peter Krefting <peter@softwolves.pp.se> junio Tran Ngoc Quan <vnwildman@gmail.com> Jiang Xin <worldhello.net@gmail.com> git cc-cmd -1 1266686 0.23s user 0.16s system 1% cpu 19.991 total

> The script could also output the addresses to yet another file.
Which would be even more hacky.
Show 5 quoted lines
>> If the command is using something expensive such as 'git blame' and
>> you have many patches, this is particularly bad. Also, it's not
>> elegant :)
>
> Elegant is a beholder viewpoint.

You think a solution that runs the same instructions multiple times unnecessarily is elegant? All right.

-- 
Felipe Contreras
Joe Perches· Nov 12, 2012, 23:40 UTC · re: Felipe Contreras · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

On Tue, 2012-11-13 at 00:37 +0100, Felipe Contreras wrote:
> On Tue, Nov 13, 2012 at 12:13 AM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote:
[]
Show 8 quoted lines
> >> For --to-cmd and --cc-cmd? So basically you check the dirname of the
> >> argument passed?
> >
> > yes. basename and dirname
> 
> Well, the basename is irrelevant, because you don't care witch
> particular patch is being sent, you are going to process all of them
> every time.

Well, I do different actions on cover letter patches than other patches because for patch sets that touch lots of files, the cc list can be _very_ long and that can run afoul of other issues like maximum recipient counts for various mailing lists.

cheers, Joe
Junio C Hamano· Nov 13, 2012, 00:37 UTC · re: Joe Perches · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

Joe Perches <joe@perches.com> writes:
Show 17 quoted lines
> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>> Felipe Contreras wrote:
>> > cc-cmd is only per-file, and many times receipients get lost without
>> > seing the full patch series.
>> 
>> s/seing/seeing
>> 
>> > [...]
>> 
>> Looks good otherwise.
>
> s/receipients/recipients/ too
>
> Practically this is ok but I think it's unnecessary.
>
> Output from git format-patch is always in a single
> directory.

Sorry, but I do not see how the usefulness (or necessity) of this new option is connected to the fact that you can tell the command to write the patches into a single (possibly new) directory. Care to explain?

> My work flow is to use a script for --to and --cc
> lines that can be set to emit the same addresses for
> all files in a patch series or generate different
> addresses per patch file.
Felipe Contreras· Nov 13, 2012, 01:01 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] send-email: add series-cc-cmd option

On Tue, Nov 13, 2012 at 1:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> Joe Perches <joe@perches.com> writes:
>
>> On Tue, 2012-11-13 at 03:21 +0530, Ramkumar Ramachandra wrote:
>>> Felipe Contreras wrote:
>>> > cc-cmd is only per-file, and many times receipients get lost without
>>> > seing the full patch series.
>>>
>>> s/seing/seeing
>>>
>>> > [...]
>>>
>>> Looks good otherwise.
>>
>> s/receipients/recipients/ too
>>
>> Practically this is ok but I think it's unnecessary.
>>
>> Output from git format-patch is always in a single
>> directory.
>
> Sorry, but I do not see how the usefulness (or necessity) of this
> new option is connected to the fact that you can tell the command to
> write the patches into a single (possibly new) directory.  Care to
> explain?

Basically 'git send-email' would do something like this: git format-patch -10 -o /tmp for x in /tmp/*; do cc_cmd $x; done

And the cc-cmd would do something like: cc_cmd() { do_stuff_with $(dirname $1)/*.patch }

-- 
Felipe Contreras

← back to recent threads