* problem when using --cc-cmd @ 2011-04-17 22:32 Thiago Farina 2011-04-19 21:52 ` Jonathan Nieder 0 siblings, 1 reply; 10+ messages in thread From: Thiago Farina @ 2011-04-17 22:32 UTC (permalink / raw) To: Git Mailing List; +Cc: Jonathan Nieder, Nguyen Thai Ngoc Duy 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: problem when using --cc-cmd 2011-04-17 22:32 problem when using --cc-cmd Thiago Farina @ 2011-04-19 21:52 ` Jonathan Nieder 2011-04-20 3:03 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Nieder @ 2011-04-19 21:52 UTC (permalink / raw) To: Thiago Farina Cc: Git Mailing List, Nguyen Thai Ngoc Duy, Joe Perches, Stephen Boyd 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: problem when using --cc-cmd 2011-04-19 21:52 ` Jonathan Nieder @ 2011-04-20 3:03 ` Joe Perches 2011-04-20 15:45 ` Thiago Farina 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2011-04-20 3:03 UTC (permalink / raw) To: Jonathan Nieder Cc: Thiago Farina, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: problem when using --cc-cmd 2011-04-20 3:03 ` Joe Perches @ 2011-04-20 15:45 ` Thiago Farina 2011-04-20 19:48 ` Joe Perches 2011-04-20 21:50 ` [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Joe Perches 0 siblings, 2 replies; 10+ messages in thread From: Thiago Farina @ 2011-04-20 15:45 UTC (permalink / raw) To: Joe Perches Cc: Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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 > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: problem when using --cc-cmd 2011-04-20 15:45 ` Thiago Farina @ 2011-04-20 19:48 ` Joe Perches 2011-04-20 21:50 ` [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Joe Perches 1 sibling, 0 replies; 10+ messages in thread From: Joe Perches @ 2011-04-20 19:48 UTC (permalink / raw) To: Thiago Farina Cc: Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses 2011-04-20 15:45 ` Thiago Farina 2011-04-20 19:48 ` Joe Perches @ 2011-04-20 21:50 ` Joe Perches 2011-04-20 22:29 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 10+ messages in thread From: Joe Perches @ 2011-04-20 21:50 UTC (permalink / raw) To: Thiago Farina Cc: Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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'"; ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses 2011-04-20 21:50 ` [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Joe Perches @ 2011-04-20 22:29 ` Ævar Arnfjörð Bjarmason 2011-04-20 22:45 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-04-20 22:29 UTC (permalink / raw) To: Joe Perches Cc: Thiago Farina, Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses 2011-04-20 22:29 ` Ævar Arnfjörð Bjarmason @ 2011-04-20 22:45 ` Joe Perches 2011-04-20 22:50 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2011-04-20 22:45 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Thiago Farina, Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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". ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses 2011-04-20 22:45 ` Joe Perches @ 2011-04-20 22:50 ` Ævar Arnfjörð Bjarmason 2011-04-20 23:01 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-04-20 22:50 UTC (permalink / raw) To: Joe Perches Cc: Thiago Farina, Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses 2011-04-20 22:50 ` Ævar Arnfjörð Bjarmason @ 2011-04-20 23:01 ` Joe Perches 0 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2011-04-20 23:01 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Thiago Farina, Jonathan Nieder, Git Mailing List, Nguyen Thai Ngoc Duy, Stephen Boyd 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 >. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-04-20 23:01 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-17 22:32 problem when using --cc-cmd Thiago Farina 2011-04-19 21:52 ` Jonathan Nieder 2011-04-20 3:03 ` Joe Perches 2011-04-20 15:45 ` Thiago Farina 2011-04-20 19:48 ` Joe Perches 2011-04-20 21:50 ` [RFC PATCH] git-send-email: Validate recipient_cmd (to-cmd, cc-cmd) addresses Joe Perches 2011-04-20 22:29 ` Ævar Arnfjörð Bjarmason 2011-04-20 22:45 ` Joe Perches 2011-04-20 22:50 ` Ævar Arnfjörð Bjarmason 2011-04-20 23:01 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).