# problem when using --cc-cmd 10 messages from 2011-04-17 to 2011-04-20. Participants: Thiago Farina, Jonathan Nieder, Joe Perches, Ævar Arnfjörð Bjarmason. Thread: https://gitlist.dev/t/27124 ## Thiago Farina, 2011-04-17 22:32 Subject: problem when using --cc-cmd Message-ID: URL: https://gitlist.dev/e/BANLkTikdaSG_jbzaJ7UCpG5JnwneARfx3Q%40mail.gmail.com ``` Hi, 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 Nieder, 2011-04-19 21:52 Subject: Re: problem when using --cc-cmd Message-ID: <20110419215239.GA22632@elie> URL: https://gitlist.dev/e/20110419215239.GA22632%40elie In-Reply-To: ``` 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 Perches, 2011-04-20 03:03 Subject: Re: problem when using --cc-cmd Message-ID: <1303268630.24766.9.camel@Joe-Laptop> URL: https://gitlist.dev/e/1303268630.24766.9.camel%40Joe-Laptop In-Reply-To: <20110419215239.GA22632@elie> ``` 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 fi ``` ## Thiago Farina, 2011-04-20 15:45 Subject: Re: problem when using --cc-cmd Message-ID: URL: https://gitlist.dev/e/BANLkTinfbF3xyfrdgfmgHQF7RHCHk8ardw%40mail.gmail.com In-Reply-To: <1303268630.24766.9.camel@Joe-Laptop> ``` On Wed, Apr 20, 2011 at 12:03 AM, Joe Perches 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 Perches, 2011-04-20 19:48 Subject: Re: problem when using --cc-cmd Message-ID: <1303328900.24766.29.camel@Joe-Laptop> URL: https://gitlist.dev/e/1303328900.24766.29.camel%40Joe-Laptop In-Reply-To: ``` On Wed, 2011-04-20 at 12:45 -0300, Thiago Farina wrote: > On Wed, Apr 20, 2011 at 12:03 AM, Joe Perches 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= 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, 2011-04-20 21:50 Subject: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Message-ID: <1303336253.24766.34.camel@Joe-Laptop> URL: https://gitlist.dev/e/1303336253.24766.34.camel%40Joe-Laptop In-Reply-To: ``` On Wed, 2011-04-20 at 12:45 -0300, Thiago Farina wrote: > On Wed, Apr 20, 2011 at 12:03 AM, Joe Perches 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 --- 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ð Bjarmason, 2011-04-20 22:29 Subject: Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Message-ID: URL: https://gitlist.dev/e/BANLkTikuC1V1U-c0wyThOZAn6gVXr7S_cQ%40mail.gmail.com In-Reply-To: <1303336253.24766.34.camel@Joe-Laptop> ``` On Wed, Apr 20, 2011 at 23:50, Joe Perches 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 Perches, 2011-04-20 22:45 Subject: Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Message-ID: <1303339511.24766.41.camel@Joe-Laptop> URL: https://gitlist.dev/e/1303339511.24766.41.camel%40Joe-Laptop In-Reply-To: ``` On Thu, 2011-04-21 at 00:29 +0200, Ævar Arnfjörð Bjarmason wrote: > On Wed, Apr 20, 2011 at 23:50, Joe Perches 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/(? 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ð Bjarmason, 2011-04-20 22:50 Subject: Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Message-ID: URL: https://gitlist.dev/e/BANLkTintK1KgPWX7ycMQtVBnKi%2BtVj7k2Q%40mail.gmail.com In-Reply-To: <1303339511.24766.41.camel@Joe-Laptop> ``` On Thu, Apr 21, 2011 at 00:45, Joe Perches 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 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 Perches, 2011-04-20 23:01 Subject: Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Message-ID: <1303340477.24766.43.camel@Joe-Laptop> URL: https://gitlist.dev/e/1303340477.24766.43.camel%40Joe-Laptop In-Reply-To: ``` On Thu, 2011-04-21 at 00:50 +0200, Ævar Arnfjörð Bjarmason wrote: > On Thu, Apr 21, 2011 at 00:45, Joe Perches 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 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 >. ```