Re: [PATCH] [1/2many] - FInd the maintainer(s) for a patch - scripts/get_maintainer.pl

Joe Perches [off-list ref] writes:

On Tue, 2007-08-14 at 17:53 +0200, Rene Herman wrote:

quoted

It isn't about MODULE_FOO() tags, it is about tagging /source/ files 
to help with putting CCs on patch submissals.
If we want to link source file foo.c and the 
MAINTAINERS information, we have 3 options:
1. MAINTAINERS --> foo.c
2. foo.c --> MAINTAINERS
3. foo.c <--> some 3rd file <--> MAINTAINERS

I added git@vger.kernel.org and Junio Hamano

Another possibility is improving git to allow
some sort of "declaration of interest" in bits
of projects.

That would allow options like:

o  git-format-patch to include CCs
o  git-commit and git-branch to notify or
     take some other action

etc...

There are things git can help, and other things git does not
have any business with.

1. Finding out who the potentially interested parties are.

   Linus already gave a script to grep *-by: lines from commit
   messages.  I find this is probably be the best option, as it
   follows "yesterday's weather".  People who had dealt with the
   area are the ones who are likely to be interested.

   git records who did the work (author) and who did the
   integration to git-based patch flow (committer).  It does not
   structurally track intermediate people who touched the patch
   on e-mail, but Signed-off-by: and Acked-by: (and sometimes I
   see Cc: as well in the commit messages) are accepted social
   convention in the kernel community, and taking advantage of
   that is a good idea.


2. Making it easier to send your patches to these people.

   There are three possible places to add Signed-off-by: and
   friends in the commit messages you would mail out:

   - When you create your own commit, or commit a patch that
     came to you via e-mail.  The commit object in your tree
     will carry them --- you can send format-patch output as-is
     to Linus or Andrew and you are done.

   - When you run format-patch; your commit will not have extra
     Cc: or "interested parties" information, you will use the
     result of 1. and insert it near your own Signed-off-by: to
     the format-patch output.

   - When you send format-patch output, via git-send-email
     perhaps.

   To make the result useful for "yesterday's weather" approach,
   I think it would be the best to do the first.  After all,
   your commit may propagate via "git pull" not over e-mail, and
   no postprocessing approach would work in such a case.

   The second one is my least favorite.  format-patch output is
   designed to record author/committer (i.e. origin) and not to
   record recipient at all.  "Who's interested in this" does not
   simply belong there.

   On the other hand, git-send-email _is_ all about sending it
   out, and it needs to know who your patch should reach.  I
   think it makes sense to have one script that, given a set of
   paths that are affected, gives a list of potentially
   interested people (that is "Finding" part -- and I see there
   are 600+ patches to implement this on the list), and a new
   option to git-send-email to (1) inspect the patch to see what
   paths are affected, and (2) call that "Find" script to figure
   out whom to send it to, and probably asking for confirmation.

On Tue, 2007-08-14 at 18:31 -0700, Junio C Hamano wrote:

   On the other hand, git-send-email _is_ all about sending it
   out, and it needs to know who your patch should reach.  I
   think it makes sense to have one script that, given a set of
   paths that are affected, gives a list of potentially
   interested people (that is "Finding" part -- and I see there
   are 600+ patches to implement this on the list), and a new
   option to git-send-email to (1) inspect the patch to see what
   paths are affected, and (2) call that "Find" script to figure
   out whom to send it to, and probably asking for confirmation.

Yes please.

The LK MAINTAINERS file is ugly.

Might there be a git portable way to "find"?

Rene Herman had an idea about using some git
metadata that might be useful.  The completely
external data approach suggested by Al Viro 
might be OK too in that it wouldn't tie listeners
to git requiring more content in git metadata.

Perhaps both via something like:

	--external-find "cmd @filelist"

Thanks,  Joe

Joe Perches wrote:

On Tue, 2007-08-14 at 18:31 -0700, Junio C Hamano wrote:

quoted

   On the other hand, git-send-email _is_ all about sending it
   out, and it needs to know who your patch should reach.  I
   think it makes sense to have one script that,

[...]

Yes please.

The LK MAINTAINERS file is ugly.

Might there be a git portable way to "find"?

Note, maintainer contacts
  - should be available to patch submitters and
  - must be available to *problem reporters*
without having to have git and a .git repo.
-- 
Stefan Richter
-=====-=-=== =--- -====
http://arcgraph.de/sr/

On 08/15/2007 11:39 AM, Stefan Richter wrote:

Note, maintainer contacts
  - should be available to patch submitters and
  - must be available to *problem reporters*
without having to have git and a .git repo.

That "must" seems rather strong. But those few non-developer users that 
could care are served by a MAINTAINERS file generated into releases.

Rene.

On Wed, 2007-08-15 at 13:44 +0200, Rene Herman wrote:

On 08/15/2007 11:39 AM, Stefan Richter wrote:

quoted

Note, maintainer contacts
  - should be available to patch submitters and
  - must be available to *problem reporters*
without having to have git and a .git repo.

That "must" seems rather strong. But those few non-developer users that 
could care are served by a MAINTAINERS file generated into releases.

Good idea for scripts to help kernel bug reporters.
REPORTING-BUGS is underutilized as a guide.

I think Bug reporting is a separate issue from patch CC'ing.
I'd rather have MAINTAINERS disappear altogether.

On Tue, 2007-08-14 at 18:31 -0700, Junio C Hamano wrote:

   On the other hand, git-send-email _is_ all about sending it
   out, and it needs to know who your patch should reach.  I
   think it makes sense to have one script that, given a set of
   paths that are affected, gives a list of potentially
   interested people (that is "Finding" part -- and I see there
   are 600+ patches to implement this on the list), and a new
   option to git-send-email to (1) inspect the patch to see what
   paths are affected, and (2) call that "Find" script to figure
   out whom to send it to, and probably asking for confirmation.

Sorry, not a git developer, so the paths are wrong.
This seems to work:

Example:

git-send-email \
   --cc-cmd "perl scripts/get_maintainers.pl -non -multiline" foo.diff

--- git-send-email.pl	2007-08-16 19:06:07.000000000 -0700
+++ /usr/local/bin/git-send-email	2007-05-01 11:59:14.000000000 -0700

@@ -47,9 +47,6 @@ Options:
    --cc           Specify an initial "Cc:" list for the entire series
                   of emails.
 
-   --cc-cmd       Specify a command to execute per file which adds
-                  per file specific cc address entries
-
    --bcc          Specify a list of email addresses that should be Bcc:
 		  on all the emails.
 

@@ -143,7 +140,7 @@ my (@to,@cc,@initial_cc,@bcclist,@xh,
 
 # Behavior modification variables
 my ($chain_reply_to, $quiet, $suppress_from, $no_signed_off_cc,
-	$dry_run, $cc_cmd) = (1, 0, 0, 0, 0, 0);
+	$dry_run) = (1, 0, 0, 0, 0);
 my $smtp_server;
 my $envelope_sender;
 

@@ -176,7 +173,6 @@ my $rc = GetOptions("from=s" => \$from,
 		    "subject=s" => \$initial_subject,
 		    "to=s" => \@to,
 		    "cc=s" => \@initial_cc,
-		    "cc-cmd=s" => \$cc_cmd,
 		    "bcc=s" => \@bcclist,
 		    "chain-reply-to!" => \$chain_reply_to,
 		    "smtp-server=s" => \$smtp_server,

@@ -611,16 +607,6 @@ foreach my $t (@files) {
 		}
 	}
 	close F;
-
-	if (${cc_cmd} ne "") {
-	    my $output = `${cc_cmd} $t`;
-	    my @lines = split("\n", $output);
-	    foreach my $c (@lines) {
-		push @cc, $c;
-		printf("(sob) Adding cc: %s from cc-cmd: '%s'\n", $c, $t) unless $quiet;
-	    }
-	}
-
 	if (defined $author_not_sender) {
 		$author_not_sender = unquote_rfc2047($author_not_sender);
 		$message = "From: $author_not_sender\n\n$message";

On Thu, 2007-08-16 at 19:13 -0700, Joe Perches wrote:

Sorry, not a git developer, so the paths are wrong.
This seems to work:

Sorry.  Patch reversed too.

--- /usr/local/bin/git-send-email	2007-05-01 11:59:14.000000000 -0700
+++ /home/joe/bin/git-send-email.pl	2007-08-16 19:25:53.000000000 -0700

@@ -47,6 +47,9 @@ Options:
    --cc           Specify an initial "Cc:" list for the entire series
                   of emails.
 
+   --cc-cmd       Specify a command to execute per file which adds
+                  per file specific cc address entries
+
    --bcc          Specify a list of email addresses that should be Bcc:
 		  on all the emails.
 

@@ -140,7 +143,7 @@ my (@to,@cc,@initial_cc,@bcclist,@xh,
 
 # Behavior modification variables
 my ($chain_reply_to, $quiet, $suppress_from, $no_signed_off_cc,
-	$dry_run) = (1, 0, 0, 0, 0);
+	$dry_run, $cc_cmd) = (1, 0, 0, 0, 0, "");
 my $smtp_server;
 my $envelope_sender;
 

@@ -173,6 +176,7 @@ my $rc = GetOptions("from=s" => \$from,
 		    "subject=s" => \$initial_subject,
 		    "to=s" => \@to,
 		    "cc=s" => \@initial_cc,
+		    "cc-cmd=s" => \$cc_cmd,
 		    "bcc=s" => \@bcclist,
 		    "chain-reply-to!" => \$chain_reply_to,
 		    "smtp-server=s" => \$smtp_server,

@@ -607,6 +611,16 @@ foreach my $t (@files) {
 		}
 	}
 	close F;
+
+	if (${cc_cmd} ne "") {
+	    my $output = `${cc_cmd} $t`;
+	    my @lines = split("\n", $output);
+	    foreach my $c (@lines) {
+		push @cc, $c;
+		printf("(sob) Adding cc: %s from cc-cmd: '%s'\n", $c, $t) unless $quiet;
+	    }
+	}
+
 	if (defined $author_not_sender) {
 		$author_not_sender = unquote_rfc2047($author_not_sender);
 		$message = "From: $author_not_sender\n\n$message";

Here's a path to enable a command line option
that takes a string argument

	cc-cmd

This modifies the @cc array to include whatever
output is produced by cc_cmd $patchfile

cccmd can be stored in a config settings file

previous versions of this patch were submitted
against an older version of git-send-email.perl

diff --git a/git-send-email.perl b/git-send-email.perl
index 69559b2..828a77a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl

@@ -46,6 +46,9 @@ Options:
    --cc           Specify an initial "Cc:" list for the entire series
                   of emails.
 
+   --cc-cmd       Specify a command to execute per file which adds
+                  per file specific cc address entries
+
    --bcc          Specify a list of email addresses that should be Bcc:
 		  on all the emails.
 

@@ -157,13 +160,14 @@ if ($@) {
 my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc);
+my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
 
 my %config_settings = (
     "thread" => [\$thread, 1],
     "chainreplyto" => [\$chain_reply_to, 1],
     "suppressfrom" => [\$suppress_from, 0],
     "signedoffcc" => [\$signed_off_cc, 1],
+    "cccmd" => [\$cc_cmd, ""],
 );
 
 foreach my $setting (keys %config_settings) {

@@ -189,6 +193,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-server=s" => \$smtp_server,
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
+		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_cc,
 		    "dry-run" => \$dry_run,

@@ -652,11 +657,21 @@ foreach my $t (@files) {
 		}
 	}
 	close F;
+
+	if (${cc_cmd} ne "") {
+	    my $output = `${cc_cmd} $t`;
+	    my @lines = split("\n", $output);
+	    foreach my $c (@lines) {
+		push @cc, $c;
+		printf("(cc-cmd) Adding cc: %s from: '%s'\n", $c, $cc_cmd)
+		    unless $quiet;
+	    }
+	}
+
 	if (defined $author) {
 		$message = "From: $author\n\n$message";
 	}
 
-
 	send_message();
 
 	# set up for the next message

Joe Perches [off-list ref] writes:

Here's a path to enable a command line option
that takes a string argument

	cc-cmd

This modifies the @cc array to include whatever
output is produced by cc_cmd $patchfile

cccmd can be stored in a config settings file

previous versions of this patch were submitted
against an older version of git-send-email.perl

... Signed-off-by: ...

I do not see a patch to "Documentation/git-send-email.txt" here...

Something like this, with appropriate error checking, perhaps?

	open my $cc, "${cc_cmd} $t |";
        while (my $c = <$cc>) {
        	...
	}
        close $cc;

On Fri, 2007-08-17 at 16:38 -0700, Junio C Hamano wrote:

Joe Perches [off-list ref] writes:
... Signed-off-by: ...
I do not see a patch to "Documentation/git-send-email.txt" here...
Something like this, with appropriate error checking, perhaps?

	open my $cc, "${cc_cmd} $t |";
        while (my $c = <$cc>) {
        	...
	}
        close $cc;

Add --cc-cmd, the ability to execute an arbitrary "cmd" to
generate per patch file specific "Cc:"s to git-send-email.perl

Signed-off-by: Joe Perches <joe@perches.com>

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index d243ed1..9a48847 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt

@@ -34,6 +34,12 @@ The --bcc option must be repeated for each user you want on the bcc list.
 +
 The --cc option must be repeated for each user you want on the cc list.
 
+--cc-cmd::
+	Specify a command to execute once per patch file which
+	should generate patch file specific "Cc:" entries.
+	Output of this command must be single email address per line.
+	Default is the value of 'sendemail.cccmd' configuration value.
+	
 --chain-reply-to, --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

@@ -124,6 +130,9 @@ sendemail.aliasfiletype::
 	Format of the file(s) specified in sendemail.aliasesfile. Must be
 	one of 'mutt', 'mailrc', 'pine', or 'gnus'.
 
+sendemail.cccmd::
+	Command to execute to generate per patch file specific "Cc:"s.
+
 sendemail.bcc::
 	Email address (or alias) to always bcc.
 

diff --git a/git-send-email.perl b/git-send-email.perl
index 69559b2..d49947c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl

@@ -46,6 +46,9 @@ Options:
    --cc           Specify an initial "Cc:" list for the entire series
                   of emails.
 
+   --cc-cmd       Specify a command to execute per file which adds
+                  per file specific cc address entries
+
    --bcc          Specify a list of email addresses that should be Bcc:
 		  on all the emails.
 

@@ -157,13 +160,14 @@ if ($@) {
 my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc);
+my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
 
 my %config_settings = (
     "thread" => [\$thread, 1],
     "chainreplyto" => [\$chain_reply_to, 1],
     "suppressfrom" => [\$suppress_from, 0],
     "signedoffcc" => [\$signed_off_cc, 1],
+    "cccmd" => [\$cc_cmd, ""],
 );
 
 foreach my $setting (keys %config_settings) {

@@ -189,6 +193,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "smtp-server=s" => \$smtp_server,
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
+		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_cc,
 		    "dry-run" => \$dry_run,

@@ -652,11 +657,25 @@ foreach my $t (@files) {
 		}
 	}
 	close F;
+
+	if (${cc_cmd} ne "") {
+	    open(F, "${cc_cmd} $t |")
+		or die "(cc-cmd) Could not execute '${cc_cmd}'\n";
+	    while(<F>) {
+		my $c = $_;
+		$c =~ s/^\s*//g;
+		$c =~ s/\n$//g;
+		push @cc, $c;
+		printf("(cc-cmd) Adding cc: %s from: '%s'\n", $c, $cc_cmd)
+		    unless $quiet;
+	    }
+	    close F;
+	}
+
 	if (defined $author) {
 		$message = "From: $author\n\n$message";
 	}
 
-
 	send_message();
 
 	# set up for the next message