Thiago FarinaHi,
I'm trying to use the --cc-cmd to get the list of people who to copy when sending a patch to linux kernel.
But when I run:
$ git send-email --to linux-kernel@vger.kernel.org --cc-cmd scripts/get_maintainer.pl foo
I'm getting some lines like: Use of uninitialized value $cc in string eq at /home/tfarina/libexec/git-core/git-send-email line 964.
Any idea?
Thanks in advance.
Jonathan NiederRe: problem when using --cc-cmd
Hi,
Thiago Farina wrote:
> when I run: > > $ git send-email --to linux-kernel@vger.kernel.org --cc-cmd > scripts/get_maintainer.pl foo > > I'm getting some lines like: > Use of uninitialized value $cc in string eq at > /home/tfarina/libexec/git-core/git-send-email line 964.
Yes, sounds like a bug. Cc-ing some send-email people for tips.
On the other hand, using --cc-cmd=scripts/get_maintainer.pl does not sound like a great idea to me. On one hand the output of get_maintainer.pl is not an unadorned address per line like --cc-cmd expects. On the other hand, at least some versions of get_maintainer.pl returned more addresses than are likely to be interested people (by using --git by default).
I think get_maintainer.pl is meant to be a starting point for tracking down who might be interested in a patch and should be followed by careful investigation. (That means making sure that there is a reasonable number of people and the reasons given by --roles ouput make sense, and maybe even glancing at some messages by them from the relevant mailing list to make sure the script has not gone haywire.)
Hope that helps, Jonathan
Joe PerchesRe: problem when using --cc-cmd
On Tue, 2011-04-19 at 16:52 -0500, Jonathan Nieder wrote:
> Thiago Farina wrote: > > when I run: > > $ git send-email --to linux-kernel@vger.kernel.org --cc-cmd > > scripts/get_maintainer.pl foo > > I'm getting some lines like: > > Use of uninitialized value $cc in string eq at > > /home/tfarina/libexec/git-core/git-send-email line 964. > Yes, sounds like a bug. Cc-ing some send-email people for tips.
I haven't seen this.
What versions of ./scripts/get_maintainer.pl and git are you using?
> On the other hand, using --cc-cmd=scripts/get_maintainer.pl does not > sound like a great idea to me. On one hand the output of > get_maintainer.pl is not an unadorned address per line like --cc-cmd > expects. On the other hand, at least some versions of > get_maintainer.pl returned more addresses than are likely to be > interested people (by using --git by default). > > I think get_maintainer.pl is meant to be a starting point for tracking > down who might be interested in a patch and should be followed by > careful investigation. (That means making sure that there is a > reasonable number of people and the reasons given by --roles ouput > make sense, and maybe even glancing at some messages by them from the > relevant mailing list to make sure the script has not gone haywire.)
Jonathan is basically correct in the what he writes above.
I also think git history isn't a very good mechanism to rely on for determining MAINTAINERS, it should only be a fallback to determine who should receive a copy of a patch.
That said, I use scripts/get_maintainer.pl to generate to's and cc's. I do not use --git or --git-fallback and rely only on the MAINTAINERS file pattern matching.
Here are the settings I use:
$ cat ~/.gitconfig [sendemail] chainreplyto = false thread = false suppresscc = self tocmd = ~/bin/to.sh cccmd = ~/bin/cc.sh
$ cat ~/bin/to.sh #!/bin/bash
opts="--nogit --nogit-fallback --norolestats --pattern-depth=1"
if [[ $(basename $1) =~ ^0000- ]] ; then
./scripts/get_maintainer.pl --nom $opts $(dirname $1)/*
else
maint=$(./scripts/get_maintainer.pl --nol $opts $1) if [ "$maint" == "" ] ; then
echo "linux-kernel@vger.kernel.org"
else
echo "$maint"
fi
fi$ cat ~/bin/cc.sh #!/bin/bash
opts="--nogit --nogit-fallback --norolestats"
if [[ $(basename $1) =~ ^0000- ]] ; then
./scripts/get_maintainer.pl --nom $opts $(dirname $1)/*
else
./scripts/get_maintainer.pl $opts $1
fiThiago FarinaRe: problem when using --cc-cmd
On Wed, Apr 20, 2011 at 12:03 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2011-04-19 at 16:52 -0500, Jonathan Nieder wrote: >> Thiago Farina wrote: >> > when I run: >> > $ git send-email --to linux-kernel@vger.kernel.org --cc-cmd >> > scripts/get_maintainer.pl foo >> > I'm getting some lines like: >> > Use of uninitialized value $cc in string eq at >> > /home/tfarina/libexec/git-core/git-send-email line 964. >> Yes, sounds like a bug. Cc-ing some send-email people for tips. > > I haven't seen this. > > What versions of ./scripts/get_maintainer.pl and git are > you using? >
$ scripts/get_maintainer.pl --version scripts/get_maintainer.pl 0.26
$ git version git version 1.7.5.rc2.5.g60e19
>> On the other hand, using --cc-cmd=scripts/get_maintainer.pl does not >> sound like a great idea to me. On one hand the output of >> get_maintainer.pl is not an unadorned address per line like --cc-cmd >> expects. On the other hand, at least some versions of >> get_maintainer.pl returned more addresses than are likely to be >> interested people (by using --git by default). >> >> I think get_maintainer.pl is meant to be a starting point for tracking >> down who might be interested in a patch and should be followed by >> careful investigation. (That means making sure that there is a >> reasonable number of people and the reasons given by --roles ouput >> make sense, and maybe even glancing at some messages by them from the >> relevant mailing list to make sure the script has not gone haywire.) > > Jonathan is basically correct in the what he writes above. > > I also think git history isn't a very good mechanism to > rely on for determining MAINTAINERS, it should only be a > fallback to determine who should receive a copy of a patch. > > That said, I use scripts/get_maintainer.pl to generate > to's and cc's. I do not use --git or --git-fallback > and rely only on the MAINTAINERS file pattern matching. > > Here are the settings I use: >
Cool, thanks for sharing it. I'll add that to my config file.
> $ cat ~/.gitconfig > [sendemail] > chainreplyto = false > thread = false > suppresscc = self > tocmd = ~/bin/to.sh > cccmd = ~/bin/cc.sh > > $ cat ~/bin/to.sh > #!/bin/bash > > opts="--nogit --nogit-fallback --norolestats --pattern-depth=1" > > if [[ $(basename $1) =~ ^0000- ]] ; then > ./scripts/get_maintainer.pl --nom $opts $(dirname $1)/* > else > maint=$(./scripts/get_maintainer.pl --nol $opts $1) > > if [ "$maint" == "" ] ; then > echo "linux-kernel@vger.kernel.org" > else > echo "$maint" > fi > fi > > $ cat ~/bin/cc.sh > #!/bin/bash > > opts="--nogit --nogit-fallback --norolestats" > > if [[ $(basename $1) =~ ^0000- ]] ; then > ./scripts/get_maintainer.pl --nom $opts $(dirname $1)/* > else > ./scripts/get_maintainer.pl $opts $1 > fi > > > >
Joe PerchesRe: problem when using --cc-cmd
On Wed, 2011-04-20 at 12:45 -0300, Thiago Farina wrote:
> On Wed, Apr 20, 2011 at 12:03 AM, Joe Perches <joe@perches.com> wrote: > > On Tue, 2011-04-19 at 16:52 -0500, Jonathan Nieder wrote: > >> Thiago Farina wrote: > >> > when I run: > >> > $ git send-email --to linux-kernel@vger.kernel.org --cc-cmd > >> > scripts/get_maintainer.pl foo > >> > I'm getting some lines like: > >> > Use of uninitialized value $cc in string eq at > >> > /home/tfarina/libexec/git-core/git-send-email line 964. > >> Yes, sounds like a bug. Cc-ing some send-email people for tips. > > I haven't seen this. > > What versions of ./scripts/get_maintainer.pl and git are > > you using? > $ scripts/get_maintainer.pl --version > scripts/get_maintainer.pl 0.26 > $ git version > git version 1.7.5.rc2.5.g60e19
To get this to work properly, the output of cc-cmd (scripts/get_maintainer.pl) must be valid email addresses.
The git send-email --help for cc-cmd says:
--cc-cmd=<command>
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.You'll need to add "--norolestats" to the cc-cmd if you use scripts/get_maintainer.pl.
$ git send-email --to linux-kernel@vger.kernel.org \ --cc-cmd "scripts/get_maintainer.pl --norolestats" foo
I suppose you could call it a defect that the output of cc-cmd isn't screened for invalid email addresses but I think it's not really a problem.
Joe Perches[RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses
On Wed, 2011-04-20 at 12:45 -0300, Thiago Farina wrote:
> On Wed, Apr 20, 2011 at 12:03 AM, Joe Perches <joe@perches.com> wrote: > > On Tue, 2011-04-19 at 16:52 -0500, Jonathan Nieder wrote: > >> Thiago Farina wrote: > >> > when I run: > >> > $ git send-email --to linux-kernel@vger.kernel.org --cc-cmd > >> > scripts/get_maintainer.pl foo > >> > I'm getting some lines like: > >> > Use of uninitialized value $cc in string eq at > >> > /home/tfarina/libexec/git-core/git-send-email line 964. > >> Yes, sounds like a bug. Cc-ing some send-email people for tips.
Perhaps some patch like this.
Validate the address(es) returned from recipient_cmd. Die if the output contains an invalid address.
Signed-off-by: Joe Perches <joe@perches.com> --- git-send-email.perl | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl index 76565de..9273cf2 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -870,10 +870,14 @@ sub is_rfc2047_quoted { # use the simplest quoting being able to handle the recipient sub sanitize_address { my ($recipient) = @_; - my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/); + my ($recipient_name, $recipient_addr) = ($recipient =~ /^\s*(.*?)\s*(<[^>]+>)/); if (not $recipient_name) { - return $recipient; + return $recipient_addr if ($recipient_addr); + if ($recipient =~ /^\s*(.+\@\S*).*$/) { + return $1; + } + return ""; } # if recipient_name is already quoted, do nothing @@ -1343,11 +1347,13 @@ sub recipients_cmd { while (my $address = <$fh>) { $address =~ s/^\s*//g; $address =~ s/\s*$//g; - $address = sanitize_address($address); - next if ($address eq $sanitized_sender and $suppress_from); - push @addresses, $address; + my $sanitized_address = sanitize_address($address); + next if ($sanitized_address eq $sanitized_sender and $suppress_from); + die "($prefix) '$cmd' returned invalid address: '$address'\n" + if ($address =~ /.*${sanitized_address}.+/); + push @addresses, $sanitized_address; printf("($prefix) Adding %s: %s from: '%s'\n", - $what, $address, $cmd) unless $quiet; + $what, $sanitized_address, $cmd) unless $quiet; } close $fh or die "($prefix) failed to close pipe to '$cmd'";
Ævar Arnfjörð BjarmasonRe: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses
On Wed, Apr 20, 2011 at 23:50, Joe Perches <joe@perches.com> wrote:
> + my ($recipient_name, $recipient_addr) = ($recipient =~ /^\s*(.*?)\s*(<[^>]+>)/);
In Perl you can write (<.*?>) instead of (<[^>]+>)
> + if ($recipient =~ /^\s*(.+\@\S*).*$/) {If this program doesn't have some extract_emails_from_string() function already it probably should.
Joe PerchesRe: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses
On Thu, 2011-04-21 at 00:29 +0200, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Apr 20, 2011 at 23:50, Joe Perches <joe@perches.com> wrote: > > + my ($recipient_name, $recipient_addr) = ($recipient =~ /^\s*(.*?)\s*(<[^>]+>)/); > In Perl you can write (<.*?>) instead of (<[^>]+>)
Hey Ævar. That matches <>. Not a good email address.
This is what linux/scripts/get_maintainers.pl uses:
sub parse_email {
my ($formatted_email) = @_; my $name = "";
my $address = ""; if ($formatted_email =~ /^([^<]+)<(.+\@.*)>.*$/) {
$name = $1;
$address = $2;
} elsif ($formatted_email =~ /^\s*<(.+\@\S*)>.*$/) {
$address = $1;
} elsif ($formatted_email =~ /^(.+\@\S*).*$/) {
$address = $1;
} $name =~ s/^\s+|\s+$//g;
$name =~ s/^\"|\"$//g;
$address =~ s/^\s+|\s+$//g; if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
$name = "\"$name\"";
}return ($name, $address); }
There's probably some weakness in that.
> If this program doesn't have some extract_emails_from_string() > function already it probably should.
Maybe it does. It currently uses "sanitize_address".
Ævar Arnfjörð BjarmasonRe: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses
On Thu, Apr 21, 2011 at 00:45, Joe Perches <joe@perches.com> wrote:
> On Thu, 2011-04-21 at 00:29 +0200, Ævar Arnfjörð Bjarmason wrote: >> On Wed, Apr 20, 2011 at 23:50, Joe Perches <joe@perches.com> wrote: >> > + my ($recipient_name, $recipient_addr) = ($recipient =~ /^\s*(.*?)\s*(<[^>]+>)/); >> In Perl you can write (<.*?>) instead of (<[^>]+>) > > Hey Ævar. That matches <>. Not a good email address.
True, but you can use <.+?> instead.
I meant that you don't need to work around the lack of non-greedy regex features in Perl.
Joe PerchesRe: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses
On Thu, 2011-04-21 at 00:50 +0200, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Apr 21, 2011 at 00:45, Joe Perches <joe@perches.com> wrote: > > On Thu, 2011-04-21 at 00:29 +0200, Ævar Arnfjörð Bjarmason wrote: > >> On Wed, Apr 20, 2011 at 23:50, Joe Perches <joe@perches.com> wrote: > >> > + my ($recipient_name, $recipient_addr) = ($recipient =~ /^\s*(.*?)\s*(<[^>]+>)/); > >> In Perl you can write (<.*?>) instead of (<[^>]+>) > > Hey Ævar. That matches <>. Not a good email address. > True, but you can use <.+?> instead.
Nope, that matches all of "<>>" I want to terminate the match on the first >.