Re: [PATCH] send-email: don't cc *-by lines with '-' prefix
On Mon, 2019-03-18 at 15:28 +0900, Junio C Hamano wrote: > Joe Perches writes: > > > My preference would be for correctness. > > I presume something like this isn't too onerous. > > I am guessing that /^---/ is to stop at the three-dash line *OR* > after the initial handful of lines of the first diff header (as the > last resort) and that is why it is not looking for /^---$/. Right. > If that is the case, I think it makes a lot of sense. It is a > general improvement not tied to the case that triggered this thread. > > Independently, I think it makes sense to do something like > > /^([a-z][a-z-]*-by|Cc): (.*)/i > > to tighten the match to exclude a non-trailer; that would have been > sufficient for the original case that triggered this thread.
Re: [PATCH] send-email: don't cc *-by lines with '-' prefix
On Sun, 2019-03-17 at 20:27 +0100, Rasmus Villemoes wrote: > On 16/03/2019 20.26, Baruch Siach wrote: > > Since commit ef0cc1df90f6b ("send-email: also pick up cc addresses from > > -by trailers") in git version 2.20, git send-email adds to cc list > > addresses from all *-by lines. As a side effect a line with > > '-Signed-off-by' is now also added to cc. This makes send-email pick > > lines from patches that remove patch files from the git repo. This is > > common in the Buildroot project that often removes (and adds) patch > > files that have 'Signed-off-by' in their patch description part. > > Yocto/OpenEmbedded and other projects do the same > > > Consider only *-by lines that start with [a-z] (case insensitive) to > > avoid unrelated addresses in cc. > > While I agree with Joe in principle that we really should not look > inside the diff part, all lines there start with [ +-], so we wouldn't > normally pick up anything from that due to the anchoring. Except for the > misc-by regexp that added hyphens to grab Reported-and-tested-by and > similar. So this is by far the simplest fix that doesn't hurt the common > use cases the misc-by handling was added to support, so > > Acked-by: Rasmus Villemoes My preference would be for correctness. I presume something like this isn't too onerous. --- git-send-email.perl | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index 8200d58cdc..83b0429576 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1697,9 +1697,10 @@ sub process_file { } } # Now parse the message body + my $in_patch = 0; while(<$fh>) { $message .= $_; - if (/^([a-z-]*-by|Cc): (.*)/i) { + if (!$in_patch && /^([a-z-]*-by|Cc): (.*)/i) { chomp; my ($what, $c) = ($1, $2); # strip garbage for the address we'll use: @@ -1725,6 +1726,8 @@ sub process_file { push @cc, $c; printf(__("(body) Adding cc: %s from line '%s'\n"), $c, $_) unless $quiet; + } elsif (/^---/) { + $in_patch = 1; } } close $fh;
Re: [PATCH] send-email: don't cc *-by lines with '-' prefix
On Sat, 2019-03-16 at 22:14 +0200, Baruch Siach wrote: > Hi Joe, Hello Baruch. > On Sat, Mar 16 2019, Joe Perches wrote: > > So buildroot uses '+Signed-off-by:' and '-Signed-off-by:' lines > > for some internal purpose? > > > > Why? > > > > https://buildroot.org/downloads/manual/manual.html > > > > doesn't mention it. > > No. Patches to the Buildroot project often add or remove patch > files. See this one for example: > > http://lists.busybox.net/pipermail/buildroot/2019-March/244762.html > > In this case 'git send-email' added Peter Korsgaard to cc because a > patch file with his sign-off is removed. > > (mbox) Adding cc: Baruch Siach from line 'From: Baruch > Siach ' > (body) Adding cc: Petr Vorel from line 'Cc: Petr Vorel > ' > (body) Adding cc: Baruch Siach from line 'Signed-off-by: > Baruch Siach ' > (body) Adding cc: Peter Korsgaard from line > '-Signed-off-by: Peter Korsgaard ' I see. IMO git send-email should not really be adding -by: lines from actual patch content but only from lines before any '^---'.
Re: [PATCH] send-email: don't cc *-by lines with '-' prefix
On Sat, 2019-03-16 at 21:49 +0200, Baruch Siach wrote: > Hi Joe, > > On Sat, Mar 16 2019, Joe Perches wrote: > > On Sat, 2019-03-16 at 21:26 +0200, Baruch Siach wrote: > > > Since commit ef0cc1df90f6b ("send-email: also pick up cc addresses from > > > -by trailers") in git version 2.20, git send-email adds to cc list > > > addresses from all *-by lines. As a side effect a line with > > > '-Signed-off-by' is now also added to cc. This makes send-email pick > > > lines from patches that remove patch files from the git repo. This is > > > common in the Buildroot project that often removes (and adds) patch > > > files that have 'Signed-off-by' in their patch description part. > > > > Why is such a line used and why shouldn't an author > > of a to-be-removed patch be cc'd? > > These lines are currently used because the '^([a-z-]*-by)' regexp > matches. That part I already understood. I am not a buildroot user. > Buildroot is a tool that build various software packages. The patches > being removed are usually for packages that Buildroot patches to fix the > build. These patches are often pulled from upstream git repo of > respective package. When the package version updates, the patch is > dropped. > > We don't cc patch authors when we add the patch in the first place, > because the regexp does not match '+Signed-off-by'. I see not reason to > cc them when we remove the patch. So buildroot uses '+Signed-off-by:' and '-Signed-off-by:' lines for some internal purpose? Why? https://buildroot.org/downloads/manual/manual.html doesn't mention it.
Re: [PATCH] send-email: don't cc *-by lines with '-' prefix
On Sat, 2019-03-16 at 21:26 +0200, Baruch Siach wrote: > Since commit ef0cc1df90f6b ("send-email: also pick up cc addresses from > -by trailers") in git version 2.20, git send-email adds to cc list > addresses from all *-by lines. As a side effect a line with > '-Signed-off-by' is now also added to cc. This makes send-email pick > lines from patches that remove patch files from the git repo. This is > common in the Buildroot project that often removes (and adds) patch > files that have 'Signed-off-by' in their patch description part. Why is such a line used and why shouldn't an author of a to-be-removed patch be cc'd? > > Consider only *-by lines that start with [a-z] (case insensitive) to > avoid unrelated addresses in cc. > > Cc: Joe Perches > Cc: Rasmus Villemoes > Signed-off-by: Baruch Siach > --- > git-send-email.perl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index 8eb63b5a2f8d..5656ba83d9b1 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -1693,7 +1693,7 @@ sub process_file { > # Now parse the message body > while(<$fh>) { > $message .= $_; > - if (/^([a-z-]*-by|Cc): (.*)/i) { > + if (/^([a-z][a-z-]*-by|Cc): (.*)/i) { > chomp; > my ($what, $c) = ($1, $2); > # strip garbage for the address we'll use:
Re: git send-email and sending the cover-letter to all cc addresses found in a patch series
On Fri, 2018-03-23 at 08:38 +0100, Dominik Brodowski wrote: > On Thu, Mar 22, 2018 at 10:44:54AM -0700, Linus Torvalds wrote: > > On Thu, Mar 22, 2018 at 10:29 AM, Peter Zijlstra > > wrote: > > > > > > But why !? Either Cc me on more of the series such that the whole makes > > > sense, or better yet, write a proper Changelog. > > > > This is a common issue. We should encourage people to always send at > > least the cover-page to everybody who gets cc'd, even if they don't > > get the whole series. > > Will try to do that in future. Does git send-email have such an option? Or > do I have to specify all cc addresses in the cover letter manually? I found > some reference to an unresolved discussion on git@ of that topic in 2016, so > I might not be the only one who could make use of that feature... The main problem might be the quantity of recipients. Many spam filters look at how many recipients on an email to eliminate likely spam. kernel.org mailing lists have a maximum email header size. Too many recipients aren't delivered to the lists. Using BCC could work, but replies then don't go to all of the recipients. Generally, I send only to listed the 0/n patch only to lists and individual patches to maintainers.
Re: grep vs git grep performance?
On Sat, 2017-10-28 at 00:11 +0200, Ævar Arnfjörð Bjarmason wrote: > On Fri, Oct 27 2017, Joe Perches jotted: [] > > git grep performance has already been > > quite successfully improved. > > ...and I have WIP patches to use the PCRE engine for patterns without -P > which I intend to start sending soon after the next release. One addition that would be quite nice would be an option to have regex matches span input lines. grep v2.54 was the last grep version that allowed this and I keep it around just for that. ie: $ cat hello.txt Hello World $ grep -P "Hello\s*World" hello.txt $ grep-2.5.4 -P "Hello\s*World" hello.txt Hello World
Re: grep vs git grep performance?
On Thu, 2017-10-26 at 10:45 -0700, Stefan Beller wrote: > On Thu, Oct 26, 2017 at 10:41 AM, Joe Perches wrote: > > On Thu, 2017-10-26 at 09:58 -0700, Stefan Beller wrote: > > > + Avar who knows a thing about pcre (I assume the regex compilation > > > has impact on grep speed) > > > > > > On Thu, Oct 26, 2017 at 8:02 AM, Joe Perches wrote: > > > > Comparing a cache warm git grep vs command line grep > > > > shows significant differences in cpu & wall clock. > > > > > > > > Any ideas how to improve this? > > > > > > > > $ time git grep "\bseq_.*%p\W" | wc -l > > > > 112 > > > > > > > > real0m4.271s > > > > user0m15.520s > > > > sys 0m0.395s > > > > > > > > $ time grep -r --include=*.[ch] "\bseq_.*%p\W" * | wc -l > > > > 112 > > > > > > > > real0m1.164s > > > > user0m0.847s > > > > sys 0m0.314s > > > > > > > > > > I wonder how much is algorithmic advantage vs coding/micro > > > optimization that we can do. > > > > As do I. I presume this is libpcre related. > > > > For instance, git grep performance is better than grep for: > > > > $ time git grep -w "seq_printf" -- "*.[ch]" | wc -l > > 8609 > > > > real0m0.301s > > user0m0.548s > > sys 0m0.372s > > > > $ time grep -w -r --include=*.[ch] "seq_printf" * | wc -l > > 8609 > > > > real0m0.706s > > user0m0.396s > > sys 0m0.309s > > > > One important piece of information is what version of Git you are running, > > > $ git tag --contains origin/ab/pcre-v2 > v2.14.0 v2.10 > ... > > (and the version of pcre, see the numbers) > https://git.kernel.org/pub/scm/git/git.git/commit/?id=94da9193a6eb8f1085d611c04ff8bbb4f5ae1e0a I definitely didn't have that one. I recompiled git latest (with USE_LIBPCRE2) and reran. Here are the results $ git --version git version 2.15.0.rc2.48.g4e40fb3 $ time git grep -P "\bseq_.*%p\W" -- "*.[ch]" | wc -l 112 real0m0.437s user0m1.008s sys 0m0.381s So, git grep performance has already been quite successfully improved. Thanks.
Re: grep vs git grep performance?
On Thu, 2017-10-26 at 09:58 -0700, Stefan Beller wrote: > + Avar who knows a thing about pcre (I assume the regex compilation > has impact on grep speed) > > On Thu, Oct 26, 2017 at 8:02 AM, Joe Perches wrote: > > Comparing a cache warm git grep vs command line grep > > shows significant differences in cpu & wall clock. > > > > Any ideas how to improve this? > > > > $ time git grep "\bseq_.*%p\W" | wc -l > > 112 > > > > real0m4.271s > > user0m15.520s > > sys 0m0.395s > > > > $ time grep -r --include=*.[ch] "\bseq_.*%p\W" * | wc -l > > 112 > > > > real0m1.164s > > user0m0.847s > > sys 0m0.314s > > > > I wonder how much is algorithmic advantage vs coding/micro > optimization that we can do. As do I. I presume this is libpcre related. For instance, git grep performance is better than grep for: $ time git grep -w "seq_printf" -- "*.[ch]" | wc -l 8609 real0m0.301s user0m0.548s sys 0m0.372s $ time grep -w -r --include=*.[ch] "seq_printf" * | wc -l 8609 real0m0.706s user0m0.396s sys 0m0.309s
Re: grep vs git grep performance?
On Thu, 2017-10-26 at 18:13 +0200, SZEDER Gábor wrote: > > Comparing a cache warm git grep vs command line grep > > shows significant differences in cpu & wall clock. > > > > Any ideas how to improve this? > > > > $ time git grep "\bseq_.*%p\W" | wc -l > > 112 > > > > real0m4.271s > > user0m15.520s > > sys 0m0.395s > > > > $ time grep -r --include=*.[ch] "\bseq_.*%p\W" * | wc -l > > 112 > > > > real0m1.164s > > user0m0.847s > > sys 0m0.314s > > Note that this "regular" grep is limited to *.c and *.h files, while > the above git grep invocation isn't and has to look at all tracked > files. How does > > git grep "\bseq_.*%p\W" "*.[ch]" > > fare? Same-ish $ time git grep "\bseq_.*%p\W" -- "*.[ch]" | wc -l 112 real0m4.225s user0m14.485s sys 0m0.413s
Re: grep vs git grep performance?
On Thu, 2017-10-26 at 17:11 +0200, Han-Wen Nienhuys wrote: > On Thu, Oct 26, 2017 at 5:02 PM, Joe Perches wrote: > > Comparing a cache warm git grep vs command line grep > > shows significant differences in cpu & wall clock. > > > > Any ideas how to improve this? > > Is git-grep multithreaded? Yes, at least according to the documentation $ git grep --help [] grep.threads Number of grep worker threads to use. If unset (or set to 0), 8 threads are used by default (for now). > IIRC, grep -r uses multiple threads. (Do > you have a 4-core machine?) I have a 2 core machine with hyperthreading $ cat /proc/cpuinfo [] model name : Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz stepping: 3 microcode : 0xba cpu MHz : 2400.000 cache size : 3072 KB physical id : 0 siblings: 4 core id : 0 cpu cores : 2
grep vs git grep performance?
Comparing a cache warm git grep vs command line grep shows significant differences in cpu & wall clock. Any ideas how to improve this? $ time git grep "\bseq_.*%p\W" | wc -l 112 real0m4.271s user0m15.520s sys 0m0.395s $ time grep -r --include=*.[ch] "\bseq_.*%p\W" * | wc -l 112 real0m1.164s user0m0.847s sys 0m0.314s
Re: [RFC for GIT] pull-request: add praise to people doing QA
On Thu, 2017-01-19 at 15:42 -0800, Jacob Keller wrote: > On Thu, Jan 19, 2017 at 1:20 PM, Jeff King wrote: > > On Thu, Jan 19, 2017 at 09:43:45PM +0100, Wolfram Sang wrote: > > > > > > As to the implementation, I am wondering if we can make this somehow > > > > work well with the "trailers" code we already have, instead of > > > > inventing yet another parser of trailers. > > > > > > > > In its current shape, "interpret-trailers" focuses on "editing" an > > > > existing commit log message to tweak the trailer lines. That mode > > > > of operation would help amending and rebasing, and to do that it > > > > needs to parse the commit log message, identify trailer blocks, > > > > parse out each trailer lines, etc. > > > > > > > > There is no fundamental reason why its output must be an edited > > > > original commit log message---it should be usable as a filter that > > > > picks trailer lines of the selected trailer type, like "Tested-By", > > > > etc. > > > > > > I didn't know about trailers before. As I undestand it, I could use > > > "Tested-by" as the key, and the commit subject as the value. This list > > > then could be parsed and brought into proper output shape. It would > > > simplify the subject parsing, but most things my AWK script currently > > > does would still need to stay or to be reimplemented (extracting names > > > from tags, creating arrays of tags given by $name). Am I correct? > > > > > > All under the assumption that trailers work on a range of commits. I > > > have to admit that adding this to git is beyond my scope. > > > > This sounds a lot like the shortlog-trailers work I did about a year > > ago: > > > > http://public-inbox.org/git/20151229073832.gn8...@sigill.intra.peff.net/ > > > > http://public-inbox.org/git/20151229075013.ga9...@sigill.intra.peff.net/ > > > > Nobody seemed to really find it useful, so I didn't pursue it. > > > > Some of the preparatory patches in that series bit-rotted in the > > meantime, but you can play with a version based on v2.7.0 by fetching > > the "shortlog-trailers-historical" branch from > > https://github.com/peff/git.git. > > > > And then things like: > > > > git shortlog --ident=tested-by --format='...tested a patch by %an' > > > > work (and you can put whatever commit items you want into the --format, > > including just dumping the hash if you want to do more analysis). > > > > -Peff > > This sounds interesting to me! When I have some more time to take a > look at this i might see if I can revive it. Can the terminology please be standardized to what was once called bylines? https://patchwork.kernel.org/patch/9307703/
Re: [PATCH] printk: Remove no longer used second struct cont
On Thu, 2016-12-15 at 21:00 -0800, Junio C Hamano wrote: > Joe Perches writes: > > > grep 2.5.4 was the last version that supported the -P option to > > grep through for multiple lines. > > Does anybody know why it was dropped? perl compatible regexes in grep have always been "experimental" and never officially supported. >From the grep manual https://www.gnu.org/software/grep/manual/grep.html --perl-regexp Interpret the pattern as a Perl-compatible regular expression (PCRE). This is highly experimental, particularly when combined with the -z (--null-data) option, and ‘grep -P’ may warn of unimplemented features. See Other Options. It wasn't dropped so much as "enhanced" away. Oh well.
Re: [PATCH] printk: Remove no longer used second struct cont
On Thu, 2016-12-15 at 18:10 -0800, Linus Torvalds wrote: > On Thu, Dec 15, 2016 at 5:57 PM, Joe Perches wrote: > > > > > > In fact, I thought we already upped the check-patch limit to 100? > > > > Nope, CodingStyle neither. > > > > Last time I tried was awhile ago. > > Ok, it must have been just talked about, and with the exceptions for > strings etc I may not have seen as many of the really annoying line > breaks lately. > > I don't mind a 80-column "soft limit" per se: if some code > consistently goes over 80 columns, there really is something seriously > wrong there. So 80 columns may well be the right limit for that kind > of check (or even less). Newspaper column widths were relatively small for a good reason. I think most of the uses of simple statements should be on a single line. I'd rather see just a few arguments on a single line than a dozen though. Especially those with long identifiers, functions with many arguments are just difficult to visually scan. > But if we have just a couple of lines that are longer (in a file that > is 3k+ lines), I'd rather not break those. > > I tend use "git grep" a lot, and it's much easier to see function > argument use if it's all on one line. > > Of course, some function calls really are *so* long that they have to > be broken up, but that's where the "if it's a couple of lines that go > a bit over the 80 column limit..." exception basically comes in. > > Put another way: long lines definitely aren't good. But breaking long > lines has some downsides too, so there should be a balance between the > two, rather than some black-and-white limit. > > In fact, we've seldom had cases where black-and-white limits work well. One thing that _would_ be useful is some enhancement to git grep that would look for multi-line statements more easily. The git grep -P option doesn't span lines. grep 2.5.4 was the last version that supported the -P option to grep through for multiple lines. It'd be nice to have something like git grep --code_style=c90 --function that'd show all multiple line uses/definitions/declarations of a particular function. I played with extending git grep a bit once, mostly to get the \s mechanism to span lines. It kinda worked. Still, it seems like real work to implement well.
Re: [PATCH V2] git-send-email: Add ability to cc: any "bylines" in the commit message
On Wed, 2016-08-31 at 12:34 -0700, Junio C Hamano wrote: > Joe Perches writes: > > Many commits have various forms of bylines similar to > A missing blank line (I can tweak while queuing). [] > > + next if $suppress_cc{'bylines'} and $what !~ > > /Signed-off-by/i and $what =~ /by$/i; > Having to keep this /by$/i in sync with whatever definition of > bylines is will be error prone. How about doing it in this way? > > # Now parse the message body > + my $bypat = r/[\w-]+-by/; > while (<$fh>) { > ... > if (/^(Signed-off-by|Cc|$bypat): (.*)$/i) { > ... > next if $suppress_cc{'bodycc'} and $what =~ > /Cc/i; > + next if $suppress_cc{'bylines'} and > + $what !~ /^Signed-off-by/i and > + $what =~ /^$bypat/i; > > Other than that, looking good. Sure, whatever you want, do you want a v3 from me or can you fix it up however you want?
[PATCH V2] git-send-email: Add ability to cc: any "bylines" in the commit message
Many commits have various forms of bylines similar to "Acked-by: Name " and "Reported-by: Name " Add the ability to cc: bylines (e.g. Acked-by:) when using git send-email. This can be suppressed with --suppress-cc=bylines. Signed-off-by: Joe Perches --- Documentation/git-send-email.txt | 11 +++ git-send-email.perl | 16 +++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 642d0ef..0b0d945 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -278,9 +278,10 @@ Automating the value of `sendemail.identity`. --[no-]signed-off-by-cc:: - If this is set, add emails found in Signed-off-by: or Cc: lines to the - cc list. Default is the value of `sendemail.signedoffbycc` configuration - value; if that is unspecified, default to --signed-off-by-cc. + If this is set, add emails found in Signed-off-by: or Cc: or any other + byline (e.g. Acked-by:) lines to the cc list. Default is the value of + `sendemail.signedoffbycc` configuration value; if that is unspecified, + default to --signed-off-by-cc. --[no-]cc-cover:: If this is set, emails found in Cc: headers in the first patch of @@ -307,8 +308,10 @@ Automating patch body (commit message) except for self (use 'self' for that). - 'sob' will avoid including anyone mentioned in Signed-off-by lines except for self (use 'self' for that). +- 'bylines' will avoid including anyone mentioned in any "-by:" lines + in the patch header except for Signed-off-by. - 'cccmd' will avoid running the --cc-cmd. -- 'body' is equivalent to 'sob' + 'bodycc' +- 'body' is equivalent to 'sob' + 'bodycc' + 'bylines' - 'all' will suppress all auto cc values. -- + diff --git a/git-send-email.perl b/git-send-email.perl index da81be4..1f53328 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -84,7 +84,7 @@ git send-email --dump-aliases --identity* Use the sendemail. options. --to-cmd * Email To: via ` \$patch_path` --cc-cmd * Email Cc: via ` \$patch_path` ---suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all. +--suppress-cc * author, self, sob, cc, cccmd, body, bodycc, bylines, all. --[no-]cc-cover* Email Cc: addresses in the cover letter. --[no-]to-cover* Email To: addresses in the cover letter. --[no-]signed-off-by-cc* Send to Signed-off-by: addresses. Default on. @@ -431,13 +431,13 @@ my(%suppress_cc); if (@suppress_cc) { foreach my $entry (@suppress_cc) { die "Unknown --suppress-cc field: '$entry'\n" - unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/; + unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|bylines)$/; $suppress_cc{$entry} = 1; } } if ($suppress_cc{'all'}) { - foreach my $entry (qw (cccmd cc author self sob body bodycc)) { + foreach my $entry (qw (cccmd cc author self sob body bodycc bylines)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'all'}; @@ -448,7 +448,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from; $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc; if ($suppress_cc{'body'}) { - foreach my $entry (qw (sob bodycc)) { + foreach my $entry (qw (sob bodycc bylines)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'body'}; @@ -1545,7 +1545,7 @@ foreach my $t (@files) { # Now parse the message body while(<$fh>) { $message .= $_; - if (/^(Signed-off-by|Cc): (.*)$/i) { + if (/^(Signed-off-by|Cc|[^\s]+[\w-]by): (.*)$/i) { chomp; my ($what, $c) = ($1, $2); chomp $c; @@ -1555,6 +1555,12 @@ foreach my $t (@files) { } else { next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i; next if $suppress_cc{'bodycc'} and $what =~ /Cc/i; + next if $suppress_cc{'bylines'} and $what !~ /Signed-off-by/i and $what =~ /by$/i; + } + if ($c !~ /.+@.+/) { + printf("(body) Ignoring %s from line '%s'\n", + $what, $_) unless $quiet; + next; } push @cc, $c; printf("(body) Adding cc: %s from line '%s'\n", -- 2.10.0.rc2.1.gaa4c9e0
Re: [PATCH] git-send-email: Add ability to cc: any "trailers" from commit message
On Wed, 2016-08-31 at 10:54 -0700, Junio C Hamano wrote: > Joe Perches writes: > > > > Many commits have various forms of trailers similar to > > "Acked-by: Name " and "Reported-by: Name " > > > > Add the ability to cc these trailers when using git send-email. > I thought you were asking what we call these " followed by > " at the end of the log message, and "footers or trailers" > was the answer. > > I do not have a strong objection against limiting to "-by:" lines; > for one thing, it would automatically avoid having to worry about > "Bug-ID:" and other trailers that won't have e-mail address at all. > > But if you are _only_ picking up "-by:" lines, then calling this > option "trailers" is way too wide and confusing. I do not think > there is any specific name for "-by:" lines, though. Perhaps you > would need to invent some name that has "-by" as a substring. > > "any-by"? or just "by"? I dunno. Thinking about this a little, "bylines" seems much better. > >@@ -1545,7 +1545,7 @@ foreach my $t (@files) { > > # Now parse the message body > > while(<$fh>) { > > $message .= $_; > > - if (/^(Signed-off-by|Cc): (.*)$/i) { > > + if (/^(Signed-off-by|Cc|[^\s]+[_-]by): (.*)$/i) { > Micronits: > > (1) do you really want to grab a run of any non-blanks? Don't > you want to exclude at least a colon? It could use [\w_-]+ > (2) allowing an underscore looks a bit unusual. It's for typos. A relatively high percentage of these things in at least the kernel were malformed when I started this 5 years ago. I don't have an objection to requiring the proper form using only dashes though. Maybe that'd help reduce the typo frequency anyway. > I am aware of the fact that people sometimes write only a name with > no e-mail address when giving credit to a third-party and we want to > avoid upsetting the underlying MTA by feeding it a non-address. > > Looking at existing helper subs like extract_valid_address and > sanitize_address that all addresses we pass to the MTA go through, > it appears to me that we try to support an addr-spec with only > local-part without @domain, so this new check might turn out to be > too strict from that point of view, but on the other hand I suspect > it won't be a huge issue because the addresses in the footers are > for public consumption and it may not make much sense to have a > local-only address there. I dunno. > > > > > push @cc, $c; > > printf("(body) Adding cc: %s from line '%s'\n", me either but I think it doesn't hurt because as you suggest, these are supposed to be public. Thanks for the review. cheers, Joe
Re: [PATCH] git-send-email: Add ability to cc: any "trailers" from commit message
On Wed, 2016-08-31 at 10:54 -0700, Junio C Hamano wrote: > Joe Perches writes: > > > > > Many commits have various forms of trailers similar to > > "Acked-by: Name " and "Reported-by: Name " > > > > Add the ability to cc these trailers when using git send-email. > I thought you were asking what we call these " followed by > " at the end of the log message, and "footers or trailers" > was the answer. > > I do not have a strong objection against limiting to "-by:" lines; > for one thing, it would automatically avoid having to worry about > "Bug-ID:" and other trailers that won't have e-mail address at all. > > But if you are _only_ picking up "-by:" lines, then calling this > option "trailers" is way too wide and confusing. I do not think > there is any specific name for "-by:" lines, though. Perhaps you > would need to invent some name that has "-by" as a substring. > > "any-by"? or just "by"? I dunno. Signatures?
[PATCH] git-send-email: Add ability to cc: any "trailers" from commit message
Many commits have various forms of trailers similar to "Acked-by: Name " and "Reported-by: Name " Add the ability to cc these trailers when using git send-email. This can be suppressed with --suppress-cc=trailers. Signed-off-by: Joe Perches --- Documentation/git-send-email.txt | 10 ++ git-send-email.perl | 16 +++- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 642d0ef..999c842 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -278,9 +278,10 @@ Automating the value of `sendemail.identity`. --[no-]signed-off-by-cc:: - If this is set, add emails found in Signed-off-by: or Cc: lines to the - cc list. Default is the value of `sendemail.signedoffbycc` configuration - value; if that is unspecified, default to --signed-off-by-cc. + If this is set, add emails found in Signed-off-by: or Cc: or any other + trailer -by: lines to the cc list. Default is the value of + `sendemail.signedoffbycc` configuration value; if that is unspecified, + default to --signed-off-by-cc. --[no-]cc-cover:: If this is set, emails found in Cc: headers in the first patch of @@ -307,8 +308,9 @@ Automating patch body (commit message) except for self (use 'self' for that). - 'sob' will avoid including anyone mentioned in Signed-off-by lines except for self (use 'self' for that). +- 'trailers' will avoid including anyone mentioned in any "-by:" lines. - 'cccmd' will avoid running the --cc-cmd. -- 'body' is equivalent to 'sob' + 'bodycc' +- 'body' is equivalent to 'sob' + 'bodycc' + 'trailers' - 'all' will suppress all auto cc values. -- + diff --git a/git-send-email.perl b/git-send-email.perl index da81be4..255465a 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -84,7 +84,7 @@ git send-email --dump-aliases --identity* Use the sendemail. options. --to-cmd * Email To: via ` \$patch_path` --cc-cmd * Email Cc: via ` \$patch_path` ---suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all. +--suppress-cc * author, self, sob, cc, cccmd, body, bodycc, trailers, all. --[no-]cc-cover* Email Cc: addresses in the cover letter. --[no-]to-cover* Email To: addresses in the cover letter. --[no-]signed-off-by-cc* Send to Signed-off-by: addresses. Default on. @@ -431,13 +431,13 @@ my(%suppress_cc); if (@suppress_cc) { foreach my $entry (@suppress_cc) { die "Unknown --suppress-cc field: '$entry'\n" - unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/; + unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|trailers)$/; $suppress_cc{$entry} = 1; } } if ($suppress_cc{'all'}) { - foreach my $entry (qw (cccmd cc author self sob body bodycc)) { + foreach my $entry (qw (cccmd cc author self sob body bodycc trailers)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'all'}; @@ -448,7 +448,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from; $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc; if ($suppress_cc{'body'}) { - foreach my $entry (qw (sob bodycc)) { + foreach my $entry (qw (sob bodycc trailers)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'body'}; @@ -1545,7 +1545,7 @@ foreach my $t (@files) { # Now parse the message body while(<$fh>) { $message .= $_; - if (/^(Signed-off-by|Cc): (.*)$/i) { + if (/^(Signed-off-by|Cc|[^\s]+[_-]by): (.*)$/i) { chomp; my ($what, $c) = ($1, $2); chomp $c; @@ -1555,6 +1555,12 @@ foreach my $t (@files) { } else { next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i; next if $suppress_cc{'bodycc'} and $what =~ /Cc/i; + next if $suppress_cc{'trailers'} and $what !~ /Signed-off-by/i && $what =~ /by$/i; + } + if ($c !~ /.+@.+/) { + printf("(body) Ignoring %s from line '%s'\n", + $what, $_) unless $quiet; + next; } push @cc, $c; printf("(body) Adding cc: %s from line '%s'\n", -- 2.10.0.rc2.1.gb2aa91d
Re: git am and duplicate signatures
On Tue, 2016-08-30 at 11:17 -0700, Junio C Hamano wrote: > Joe Perches writes: > > > > > On Tue, 2016-08-30 at 10:41 -0700, Joe Perches wrote: > > > > > > Maybe something like traces or chains. > > Or "taggers" or "tagged-bys" > I am afraid that you are way too late; the ship has already sailed a > few years ago, if not earlier, I think. What's the ship's name? Is it footers or trailers?
Re: git am and duplicate signatures
On Tue, 2016-08-30 at 10:41 -0700, Joe Perches wrote: > Maybe something like traces or chains. Or "taggers" or "tagged-bys"
Re: git am and duplicate signatures
On Tue, 2016-08-30 at 10:34 -0700, Junio C Hamano wrote: > Joe Perches writes: > > On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote: > > > > > > Support for more generic footers was supposed to come when the > > > "interpret-trailers" topic started, but the author of the topic > > > seems to have lost interest before the mechanism has become ready to > > > be integrated in the workflow commands like "am", "commit", "rebase" > > > etc., which is unfortunate. > > I think adding at least an option to git send-email > > allowing auto-cc's for all > > "-by: [name] " > > lines in the commit log would be useful. > > > > Today, only "Signed-off-by" and "CC" lines are > > added to cc's. > > > > I've always called these lines "-by:" lines > > "signatures", but perhaps there's a better name. > I think we casually call them footers (as they are counter-part to > "headers"), or trailers. I think they are neither footers, which would relate more to the email headers, nor trailers. Maybe something like traces or chains. btw: I submitted this awhile ago http://www.spinics.net/lists/git/msg162269.html
Re: git am and duplicate signatures
On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote: > Support for more generic footers was supposed to come when the > "interpret-trailers" topic started, but the author of the topic > seems to have lost interest before the mechanism has become ready to > be integrated in the workflow commands like "am", "commit", "rebase" > etc., which is unfortunate. I think adding at least an option to git send-email allowing auto-cc's for all "-by: [name] " lines in the commit log would be useful. Today, only "Signed-off-by" and "CC" lines are added to cc's. I've always called these lines "-by:" lines "signatures", but perhaps there's a better name. Any preference? from git send-email --help --suppress-cc= Specify an additional category of recipients to suppress the auto-cc of: · author will avoid including the patch author · self will avoid including the sender · cc will avoid including anyone mentioned in Cc lines in the patch header except for self (use self for that). · bodycc will avoid including anyone mentioned in Cc lines in the patch body (commit message) except for self (use self for that). · sob will avoid including anyone mentioned in Signed-off-by lines except for self (use self for that). · cccmd will avoid running the --cc-cmd. · body is equivalent to sob + bodycc · all will suppress all auto cc values. > > > > sequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP > Yes, I think this is one of the warts we talked about getting rid of > but haven't got around to it. It is there because "format-patch -s" > was incorrectly written to dedup Signed-off-by: from anywhere in its > early implementation and to keep the same behaviour. We should drop > that flag from append_signoff() function.
Re: git am and duplicate signatures
(adding lkml) On Tue, 2016-08-30 at 09:54 -0700, Junio C Hamano wrote: > Joe Perches writes: > > git-am -s will avoid duplicating the last signature > > in a patch. > > > > But given a developer creates a patch, send it around for > > acks/other signoffs, collects signatures and then does > > a git am -s on a different branch, this sort of sign-off > > chain is possible: > > > > Signed-off-by: Original Developer > > Acked-by: Random Developer > > Signed-off-by: Original Developer > Both correct and allowing the earlier one duplicated as long as > there is somebody/something else in between is deliberate. linux-kernel has a script (scripts/checkpatch.pl) that looks for duplicate signatures (-by: [name] ) Should the last Signed-off-by: in the commit log be excluded from this check? > > Should there be an option to avoid duplicate signatures > > in a sequence where an author can git-am the same patch? > I dunno. The way "Signed-off-by" is handled is designed > specifically to support the meaning of that footer, namely to record > where it originated and whose hands it passed, used in the kernel > and Git land. Other projects certainly may have need for footers > that denote different things that want different semantics (e.g. Who > authored it and who cheered on it), but that is outside the scope of > the "Signed-off-by" supported by "am -s" and "commit -s". > > Support for more generic footers was supposed to come when the > "interpret-trailers" topic started, but the author of the topic > seems to have lost interest before the mechanism has become ready to > be integrated in the workflow commands like "am", "commit", "rebase" > etc., which is unfortunate. > > > > > sequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP > Yes, I think this is one of the warts we talked about getting rid of > but haven't got around to it. It is there because "format-patch -s" > was incorrectly written to dedup Signed-off-by: from anywhere in its > early implementation and to keep the same behaviour. We should drop > that flag from append_signoff() function.
git am and duplicate signatures
git-am -s will avoid duplicating the last signature in a patch. But given a developer creates a patch, send it around for acks/other signoffs, collects signatures and then does a git am -s on a different branch, this sort of sign-off chain is possible: Signed-off-by: Original Developer Acked-by: Random Developer Signed-off-by: Original Developer Should there be an option to avoid duplicate signatures in a sequence where an author can git-am the same patch? sequencer.c:append_signoff() has a flag for APPEND_SIGNOFF_DEDUP sequencer.c:void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag) but builtin/commit.c: append_signoff(&sb, ignore_non_trailer(&sb), 0); doesn't have an optional use mechanism available.
Re: [Ksummit-discuss] checkkpatch (in)sanity ?
On Sun, 2016-08-28 at 23:24 +0200, Dennis Kaarsemaker wrote: > > There are some that want an ncurses only version of git blame > > that could use arrow-key style navigation for historical commit > > line-ranges. > > > > git gui blame kind of works, but it's not ncurses/text based. > > git-cola kind of works too, but it's not text based either. > > > > Are there other existing tools for blame history viewing? > tig has a neat way of doing blame history digging: do a `tig blame > filename`, select a line and hit the comma key to re-blame from the > parent of the commit that was blamed for that line. Thanks, of course I neglected to mention tig. What I think some here want is the ability to view back and forth from old to new rather than just split the window horizontally with the commit content. Basically, apply the patch hunks for a specific range to see color coded changes rather just show the entire patch in a separate view. Sure, seeing the commit log and patch is useful. Seeing the block of code pre and post patch for the specific section can be more useful. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ksummit-discuss] checkkpatch (in)sanity ?
On Sun, 2016-08-28 at 11:59 +0200, Julia Lawall wrote: > On Sun, 28 Aug 2016, Alexey Dobriyan wrote: [] > > The problem is that c-h.pl generates noise in the commit history and > > makes git-blame less useful than it can be. > > Could it be that this is a problem with git blame, rather than with > checkpatch? Last year there was a discussion on this list about how there > is an option to git blame that will cause it to step through the history, > and not show only the most recent patch that has modified a given line. It is more or less an ease-of-use limitation of git blame. There are some that want an ncurses only version of git blame that could use arrow-key style navigation for historical commit line-ranges. git gui blame kind of works, but it's not ncurses/text based. git-cola kind of works too, but it's not text based either. Are there other existing tools for blame history viewing? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: get_maintainer.pl and .mailmap entries with more than 2 addresses
On Wed, 2016-08-03 at 00:17 +0200, Florian Mickler wrote: > cc'd mche...@s-opensource.com (Mauro, is your kernel.org address up?) > > Am Tue, 02 Aug 2016 09:36:21 -0700 > schrieb Joe Perches : > > > > > Hello Florian. > > There is at least an oddity with get_maintainer handling of a > > .mailmap entry form. > > > > For instance: > > > > Mauro's .mailmap entry is: > > Mauro Carvalho Chehab > > > > > > > > Is this a valid form? > > > > get_maintainer output for Mauro is: > > > > $ ./scripts/get_maintainer.pl drivers/media/ -f > > Mauro Carvalho Chehab > > > > (maintainer:MEDIA INPUT INFRASTRUCTURE > > (V4L/DVB)) > > > > I believe the Mauro's and Shuah's .mailmap entries are improper and > > should be changed, but I'm not completely aware of git .mailmap > > handling and the documentation seems weakly specified. > > > Hmm.. looking at Mauros last .mailmap commit it seems like your patch is > ok for Mauro. > > Although and are probably > missing? (@Mauro) > > > $ git shortlog | grep "^Mauro C" > Mauro Carvalho Chehab (4404): > $ git log | grep "^Author:.*Mauro Carvalho Chehab" | sort | uniq -c > 2 Author: Mauro Carvalho Chehab > 146 Author: Mauro Carvalho Chehab > 645 Author: Mauro Carvalho Chehab > 794 Author: Mauro Carvalho Chehab > 2015 Author: Mauro Carvalho Chehab > 448 Author: Mauro Carvalho Chehab > 353 Author: Mauro Carvalho Chehab > 1 Author: Mauro Carvalho Chehab > > > > Anyway, from a technical viewpoint your patches seem to fix > the .mailmap entry as the author intended. (See Junio's Email for the > documantation part) > But I would wait for the ack from Mauro and Shuah. As far as I understand, a single entry with just their name and preferred email address would work too because the name parts are all spelled identically. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
get_maintainer.pl and .mailmap entries with more than 2 addresses
Hello Florian. There is at least an oddity with get_maintainer handling of a .mailmap entry form. For instance: Mauro's .mailmap entry is: Mauro Carvalho Chehab Is this a valid form? get_maintainer output for Mauro is: $ ./scripts/get_maintainer.pl drivers/media/ -f Mauro Carvalho Chehab (maintainer:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)) I believe the Mauro's and Shuah's .mailmap entries are improper and should be changed, but I'm not completely aware of git .mailmap handling and the documentation seems weakly specified. https://git-scm.com/docs/git-check-mailmap Maybe get_maintainer.pl needs a change or perhaps this patch is appropriate. --- .mailmap | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.mailmap b/.mailmap index c0d5704..c7f92ca 100644 --- a/.mailmap +++ b/.mailmap @@ -96,7 +96,12 @@ Linus Lüssing Linus Lüssing Mark Brown Matthieu CASTET -Mauro Carvalho Chehab +Mauro Carvalho Chehab +Mauro Carvalho Chehab +Mauro Carvalho Chehab +Mauro Carvalho Chehab +Mauro Carvalho Chehab +Mauro Carvalho Chehab Matt Ranostay Matthew Ranostay Matt Ranostay Mayuresh Janorkar @@ -132,7 +137,10 @@ Santosh Shilimkar Sascha Hauer S.Çağlar Onur Shiraz Hashim -Shuah Khan +Shuah Khan +Shuah Khan +Shuah Khan +Shuah Khan Simon Kelley Stéphane Witzmann Stephen Hemminger -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3] git-send-email: Add auto-cc to all body signatures
On Wed, 2015-12-02 at 09:58 -0800, Junio C Hamano wrote: > "Trailers" are not limited to "*-by:" btw: what are "Trailers" limited by? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3] git-send-email: Add auto-cc to all body signatures
On Wed, 2015-12-02 at 09:58 -0800, Junio C Hamano wrote: > Joe Perches writes: > > > Many types of signatures are used by various projects. > > > > The most common type is formatted: > > "[some_signature_type]-by: First Last domain.tld>" > > e.g: > > "Reported-by: First Last domain.tld>" (no quotes are used) > > > > Make git-send-email use these signatures as "CC:" entries. > > > > Add command line option --suppress-cc=signatures to avoid > > adding these entries to the cc. > > > > Signed-off-by: Joe Perches perches.com> > > Acked-by: Jeff Kirsher intel.com> > > I wonder what send-email with this patch does to the above two lines > with "" not "@" ;-) How was this patch sent? gnome evolution v3.18.2 email client. And it seems all newer versions of evolution beyond 3.12 are really, really poor at sending inline patches. I'll update and resend using git-send-email eventually > In any case, did you mean "Helped-by:" not "Acked-by:"? "git > shortlog git-send-email.perl" does not show that name as one of the > major stakeholders who would be capable of giving an Ack on it. At least for linux-kernel, "Acked-by:" doesn't mean a maintainer or a contributor to a particular module/file, just someone that has looked at the patch, tried it, and approved of the concept. I don't know what process git uses for approval/signatures. > > --- > > > It's been four years, but I recently ran into this. I mistakenly thought > > > that git would actually pick up cc addresses also from Reported-by, so > > > the reporter ended up not being cc'ed. Is there any chance this could be > > > revisited, > > > > Here's a refresh if desired. I still think it's sensible. > > What the patch tries to achieve may make a lot of sense. I however > do not necessarily think this particular implementation does, > unfortunately. > > These "Random-by:", especially the ones that the author adds on his > own initiative like "Reported-by:", are often followed by just a > name but not an addresses. A "Signed-off-by:" and "Cc:" that is not > followed by a valid e-mail address may deserve to get an error (or > perhaps an end-user interaction "This is not a valid address. What > do you want to do about it?") so "/^(Signed-off-by|Cc): (.*)$/i" > does not need its own sanity check on $2, because a later call to > extract-valid-address or extract-valid-address-or-die will take care > of it. > It would however be wrong to cause the program to error out or even > bother the user upon seeing such random trailer lines that the > author did not mean to have an e-mail address on it in the first > place. If you have a trailer line > > Random-by: Joe Perches > > without an address, I suspect you will end up adding "Joe" and > "Perches" as two addresses on the Cc: line, which is most likely not > what the user intended [*1*]. At least with new versions of git-send-email.perl that's true so the patch will need to validate that there is an email address following. > As to the lingo, these are still not signatures, but during the past > years, it seems that we settled on using the term "trailers" for > these e-mail header-like things at the end of the log message. > "Trailers" are not limited to "*-by:" so this patch is not about > adding auto-cc to all trailers--a retitle would be > > send-email: add auto-cc to addresses that appear on *-by: trailers > > or something (and the option and variable names may need to be > updated to match). > > > [Footnote] > > *1* I further suspect that the existing code shares a similar issue. > Don't Cc: and Signed-off-by: expect a single address on each line in > the usual fashion? Perhaps a two-patch series whose first part does > > - if (/^(Signed-off-by|Cc): (.*)$/i) { > + if (/^(Signed-off-by|Cc): (.*<[^>]*>)\s*$/i) { > > to tighten it (so that "Cc: Joe Perches" would not result in two > pieces of mail sent to Joe and Perches), with your patch as a follow > up, may be a good way forward. > > I dunno. I believe the old git-send-email code required addresses and validated the form after Signed-off-by:'s. I haven't looked at the code for several years and just refreshed it without much thinking or testing. I'll do a bit more and resend. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3] git-send-email: Add auto-cc to all body signatures
Many types of signatures are used by various projects. The most common type is formatted: "[some_signature_type]-by: First Last domain.tld>" e.g: "Reported-by: First Last domain.tld>" (no quotes are used) Make git-send-email use these signatures as "CC:" entries. Add command line option --suppress-cc=signatures to avoid adding these entries to the cc. Signed-off-by: Joe Perches perches.com> Acked-by: Jeff Kirsher intel.com> --- > It's been four years, but I recently ran into this. I mistakenly thought > that git would actually pick up cc addresses also from Reported-by, so > the reporter ended up not being cc'ed. Is there any chance this could be > revisited, Here's a refresh if desired. I still think it's sensible. Documentation/git-send-email.txt | 3 ++- git-send-email.perl | 11 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index b9134d2..0866ae2 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -306,8 +306,9 @@ Automating patch body (commit message) except for self (use 'self' for that). - 'sob' will avoid including anyone mentioned in Signed-off-by lines except for self (use 'self' for that). +- 'signatures' will avoid including anyone mentioned in any "-by:" lines. - 'cccmd' will avoid running the --cc-cmd. -- 'body' is equivalent to 'sob' + 'bodycc' +- 'body' is equivalent to 'sob' + 'bodycc' + 'signatures' - 'all' will suppress all auto cc values. -- + diff --git a/git-send-email.perl b/git-send-email.perl index e907e0ea..536e264 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -83,7 +83,7 @@ git send-email [options] --identity* Use the sendemail. options. --to-cmd * Email To: via ` \$patch_path` --cc-cmd * Email Cc: via ` \$patch_path` ---suppress-cc * author, self, sob, cc, cccmd, body, bodycc, all. +--suppress-cc * author, self, sob, cc, cccmd, body, bodycc, signatures, all. --[no-]cc-cover* Email Cc: addresses in the cover letter. --[no-]to-cover* Email To: addresses in the cover letter. --[no-]signed-off-by-cc* Send to Signed-off-by: addresses. Default on. @@ -421,13 +421,13 @@ my(%suppress_cc); if (@suppress_cc) { foreach my $entry (@suppress_cc) { die "Unknown --suppress-cc field: '$entry'\n" - unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/; + unless $entry =~ /^(?:all|cccmd|cc|author|self|sob|body|bodycc|signatures)$/; $suppress_cc{$entry} = 1; } } if ($suppress_cc{'all'}) { - foreach my $entry (qw (cccmd cc author self sob body bodycc)) { + foreach my $entry (qw (cccmd cc author self sob body bodycc signatures)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'all'}; @@ -438,7 +438,7 @@ $suppress_cc{'self'} = $suppress_from if defined $suppress_from; $suppress_cc{'sob'} = !$signed_off_by_cc if defined $signed_off_by_cc; if ($suppress_cc{'body'}) { - foreach my $entry (qw (sob bodycc)) { + foreach my $entry (qw (sob bodycc signatures)) { $suppress_cc{$entry} = 1; } delete $suppress_cc{'body'}; @@ -1516,7 +1516,7 @@ foreach my $t (@files) { # Now parse the message body while(<$fh>) { $message .= $_; - if (/^(Signed-off-by|Cc): (.*)$/i) { + if (/^(Signed-off-by|Cc|[^\s]+[_-]by): (.*)$/i) { chomp; my ($what, $c) = ($1, $2); chomp $c; @@ -1526,6 +1526,7 @@ foreach my $t (@files) { } else { next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i; next if $suppress_cc{'bodycc'} and $what =~ /Cc/i; + next if $suppress_cc{'signatures'} and $what =~ /by$/i; } push @cc, $c; printf("(body) Adding cc: %s from line '%s'\n", -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Odd git am behavior rewriting subject, adding "ASoC: " prefix
On Wed, 2014-11-05 at 22:12 +1300, Chris Packham wrote: > On Wed, Nov 5, 2014 at 2:12 PM, Joe Perches wrote: > > I have a patch file created by git format-patch. [] > > ASoC:? where does that come from? [] > Looks like you have an apply-patch-msg hook installed. What does the > output of 'ls -l .git/hooks' look like. Correct. I didn't look there. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Odd git am behavior rewriting subject, adding "ASoC: " prefix
I have a patch file created by git format-patch. Applying it via git am changes the subject prefix. Anyone know why? $ git --version git version 2.1.2 $ git am -i 0002-staging-ft1000-Logging-message-neatening.patch Commit Body is: -- staging: ft1000: Logging message neatening Use a more common logging style. o Convert DEBUG macros to pr_debug o Add pr_fmt o Remove embedded function names from pr_debug o Convert printks to pr_ o Coalesce formats and align arguments o Add missing terminating newlines Signed-off-by: Joe Perches -- Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all choosing "Y" emits: Applying: ASoC: staging: ft1000: Logging message neatening ASoC:? where does that come from? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: project wide: git config entry for [diff] renames=true
On Thu, 2014-09-25 at 14:00 -0400, Jeff King wrote: > On Thu, Sep 25, 2014 at 08:48:31AM -0700, Joe Perches wrote: > > > On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote: > > > > > In the future, please generate a git "move" diff, which makes it easier > > > to review, and prove that nothing really changed. It also helps if the > > > file is a bit different from what you diffed against, which in my case, > > > was true. > > > > Maybe it'd be possible to add > > > > [diff] > > renames = true > > > > to the .git/config file. > > > > but I don't find a mechanism to add anything to the > > .git/config and have it be pulled. > > There is no such mechanism within git. We've resisted adding one because > of the danger of something like: > > [diff] > external = rm -rf / > > diff.renames is probably safe, but any config-sharing mechanism would > have to deal with either whitelisting, or providing some mechanism for > the puller to review changes before blindly following them. Another mechanism might be to add a repository top level .gitconfig and add whatever to that. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
project wide: git config entry for [diff] renames=true
On Thu, 2014-09-25 at 17:03 +0200, Greg Kroah-Hartman wrote: > In the future, please generate a git "move" diff, which makes it easier > to review, and prove that nothing really changed. It also helps if the > file is a bit different from what you diffed against, which in my case, > was true. Maybe it'd be possible to add [diff] renames = true to the .git/config file. but I don't find a mechanism to add anything to the .git/config and have it be pulled. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkpatch: Add test for commit id formatting style in commit log
On Sun, 2014-08-10 at 14:35 -0700, Andrew Morton wrote: > On Sun, 10 Aug 2014 14:28:01 -0700 Joe Perches wrote: > > > On Thu, Jul 3, 2014 at 12:00 AM, Joe Perches wrote: > > > > Commit logs have various forms of commit id references. > > > > > > > > Try to standardize on a 12 character long lower case > > > > commit id along with a description of parentheses and > > > > the quoted subject line > > > > > > > > ie: commit 0123456789ab ("commit description") > > > > > > Now this is in mainline, checkpatch starts complaining about my "too long" > > > (40 chars) commit IDs in commit messages :-( > > > > > > 40 chars may be too long (but it's quick to copy-and-paste, as "git show" > > > shows that by default), but 12 sounds a bit short, as that's only 48 bits. > > > > Right now, this test allows 12 to 16 byte length commit ids > > without emitting a warning. > > > > Andrew wanted this test, I don't care how long the commit id > > is in the commit log. > > Well, I mainly wanted to stop having to add "commit description" when > people forget it. The length check was perhaps a bit anal. How about > we make it "12 or more"? Fine by me, just change the 16 to 40 --- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 31a731e..b385bcb 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2133,7 +2133,7 @@ sub process { # Check for improperly formed commit descriptions if ($in_commit_log && $line =~ /\bcommit\s+[0-9a-f]{5,}/i && - $line !~ /\b[Cc]ommit [0-9a-f]{12,16} \("/) { + $line !~ /\b[Cc]ommit [0-9a-f]{12,40} \("/) { $line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i; my $init_char = $1; my $orig_commit = lc($2); @@ -2141,7 +2141,7 @@ sub process { my $desc = 'commit description'; ($id, $desc) = git_commit_info($orig_commit, $id, $desc); ERROR("GIT_COMMIT_ID", - "Please use 12 to 16 chars for the git commit ID like: '${init_char}ommit $id (\"$desc\")'\n" . $herecurr); + "Please use 12 or more chars for the git commit ID like: '${init_char}ommit $id (\"$desc\")'\n" . $herecurr); } # Check for added, moved or deleted files -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkpatch: Add test for commit id formatting style in commit log
On Sun, 2014-08-10 at 23:08 +0200, Geert Uytterhoeven wrote: > Hi Joe, Hi Geert. > On Thu, Jul 3, 2014 at 12:00 AM, Joe Perches wrote: > > Commit logs have various forms of commit id references. > > > > Try to standardize on a 12 character long lower case > > commit id along with a description of parentheses and > > the quoted subject line > > > > ie: commit 0123456789ab ("commit description") > > Now this is in mainline, checkpatch starts complaining about my "too long" > (40 chars) commit IDs in commit messages :-( > > 40 chars may be too long (but it's quick to copy-and-paste, as "git show" > shows that by default), but 12 sounds a bit short, as that's only 48 bits. Right now, this test allows 12 to 16 byte length commit ids without emitting a warning. Andrew wanted this test, I don't care how long the commit id is in the commit log. > According to the Birthday Paradox (en.wikiipedia.org/wiki/Birthday_problem), > there's a probability of 50% of a collision if you use 48 bits IDs in a > repository with ca. 16 milion (2^24) objects. A Linux kernel repository > counts ca. 4 million objects, so we're getting close... > > So soon we'll get "error: short SHA1 is ambiguous". > > BTW, is there actually an easy way to make "git show" show all options for > an ambiguous SHA1? Not so far as I know, but I'm nothing like a git expert. The script I used before adding this to checkpatch was: $ cat format_commit.sh #!/bin/bash regex1="^error: short SHA1 $1 is ambiguous\." regex2="fatal: ambiguous argument '$1': unknown revision or path not in the working tree\." tmp=$(mktemp --tmpdir format_commit.X) git log --format='%H ("%s")' -1 $1 > $tmp 2>&1 read line < $tmp rm -f $tmp if [[ $line =~ $regex1 ]] ; then echo "checking commits $1..." git rev-list --remotes | grep -i "^$1" | while read line ; do git log --format='%H ("%s")' -1 $line | echo "commit $(cut -c 1-12,41-)" done elif [[ $line =~ $regex2 ]] ; then echo "No matching commit" exit 1 else echo "commit $(echo $line | cut -c1-12,41-)" fi exit 0 $ so that using "$ format_commit.sh 1234" looks at _all_ the commit references by using git rev-list then greps that output for the matches, but it is darn slow... $ time ./format_commit.sh 1234 checking commits 1234... commit 1234351cba95 ("xfs: introduce xlog_copy_iovec") commit 1234471e2d11 ("perf header: Fix numa topology printing") commit 1234f4bada54 ("hwrng: Kconfig: remove dependency for atmel-rng driver") commit 12340313cf94 ("MAINTAINERS: add new cgroup list to CC notice") commit 12346037a718 ("UBIFS: dump more in the lprops debugging check") commit 12342c475f5d ("iwlwifi: proper monitor support") commit 1234010684bb ("Add notation that the Asus W5F laptop has a short cable instead of 80-wire.") commit 123411f2d0da ("[CPUFREQ] dprintf format fixes in cpufreq/speedstep-centrino.c") real0m24.535s user0m21.668s sys 0m5.332s -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git blame vs git log --follow performance
On Mon, 2014-01-27 at 08:33 +0700, Duy Nguyen wrote: > On Mon, Jan 27, 2014 at 4:10 AM, Joe Perches wrote: > > For instance (using the Linus' linux kernel git): > > > > $ time git log --follow -- drivers/firmware/google/Kconfig > /dev/null > > > > real0m42.329s > > user0m40.984s > > sys 0m0.792s > > > > $ time git blame -- drivers/firmware/google/Kconfig > /dev/null > > > > real0m0.963s > > user0m0.860s > > sys 0m0.096s > > > > It's not fair to compare blame and log. If you compare, compare it to > non follow version Perhaps not, but git blame does follow renames. $ git blame --help [] The origin of lines is automatically followed across whole-file renames (currently there is no option to turn the rename-following off). To follow lines moved from one file to another, or to follow lines that were copied and pasted from another file, etc., see the -C and -M options. > I tested a version with rename detection logic removed. It did not > change the timing significantly. To improve --follow I think we need > to do something about path filtering. Perhaps the log history could stop being read when a commit is found that creates the file without another file being deleted in the same commit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git blame vs git log --follow performance
Hi. Is there something that can be done about improving git log --follow -- performance to be nearly equivalent speed to git blame -- ? The overall cpu time taken for these 2 commands that track individual file history can be quite different. git log --follow -- and git blame -- It seems that there can be a couple orders of magnitude delta in the overall time taken. For instance (using the Linus' linux kernel git): $ time git log --follow -- drivers/firmware/google/Kconfig > /dev/null real0m42.329s user0m40.984s sys 0m0.792s $ time git blame -- drivers/firmware/google/Kconfig > /dev/null real0m0.963s user0m0.860s sys 0m0.096s This particular file has never been renamed. Looking at the output on screen, there does seem to be 25+ seconds of cpu time consumed after the initial (last shown) commit that introduces this file. Perhaps adding a whole-file rename option to the "git log" history simplification mechanism could help? Thoughts? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug? git format-patch -M -D then git am fails
On Tue, 2012-11-13 at 14:55 -0800, Junio C Hamano wrote: > Joe Perches writes: > > > (Sorry about the partial message. > > evolution and ctrl-enter sends, grumble...) > > > > If a file is deleted with git rm and a patch > > is then generated with git format-patch -M -D > > git am is unable to apply the resultant patch. > > > > Is this working as designed? > > I would say it is broken as designed and it is even documented. > > Please run "git format-patch --help | less" and then type > "/--irreversible-delete" to find: > > The resulting patch is not meant to be applied with patch nor > git apply; this is solely for people who want to just > concentrate on reviewing the text after the change. yeah, it's just that not using -D can result in some unfortunately large patches being sent to mailing lists. I don't believe that reversibility is a really useful aspect of deletion patches when there are known git repositories involved. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug? git format-patch -M -D then git am fails
(Sorry about the partial message. evolution and ctrl-enter sends, grumble...) If a file is deleted with git rm and a patch is then generated with git format-patch -M -D git am is unable to apply the resultant patch. Is this working as designed? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
bug? git format-patch -M -D then git am fails
If a file is deleted with git rm and a patch is then generated with git format-patch -M - -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 wrote: > > On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote: [] > >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] send-email: add series-cc-cmd option
On Tue, 2012-11-13 at 00:03 +0100, Felipe Contreras wrote: > On Mon, Nov 12, 2012 at 11:52 PM, Joe Perches 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] send-email: add series-cc-cmd option
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. 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html