From: Roger Leigh <hidden> Date: 2016-06-15 22:47:00
[I'm not currently subscribed to the list; I'd appreciate a CC
on any replies, thanks!]
Hi,
In most of the projects I work on, the git commit message has
the affected subsystem or component in square brackets, such as
[foo] change bar to baz
For example, with a single patch from a series produced by
git format-patch:
% head -n4 /tmp/patches/0005-sbuild-chroot_mountable-Don-t-derive-from-chroot.patch
From f01579584f1e7d77cf1e9c3306601a4cccff8c55 Mon Sep 17 00:00:00 2001
From: Roger Leigh <redacted>
Date: Fri, 10 Apr 2009 19:43:15 +0100
Subject: [PATCH 05/15] [sbuild] chroot_mountable: Don't derive from chroot
% git mailinfo </tmp/patches/0005-sbuild-chroot_mountable-Don-t-derive-from-chroot.patch /dev/null /dev/null
Author: Roger Leigh
Email: rleigh@debian.org
Subject: chroot_mountable: Don't derive from chroot
Date: Fri, 10 Apr 2009 19:43:15 +0100
The [sbuild] prefix has been dropped from the Subject, so an
important bit of context about the patch has been lost.
It's a bit of a bug that you can't round trip from a git-format-patch
to import with git-am and then not be able to produce the exact same
patch set with git-format-patch again (assuming preparing and applying
to the same point, of course).
Would it be possible to change the git-mailinfo logic to use a less
greedy pattern match so it leaves everything after
([PATCH( [0-9/])+])+ in the subject? AFAICT this is cleanup_subject in
builtin-mailinfo.c? Could this rather complex function not just do a
simple regex match which can also take care of stripping ([Rr]e:) ?
Thanks,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
From: Jeff King <hidden> Date: 2016-06-15 22:47:00
On Sun, Jun 28, 2009 at 08:38:58PM +0100, Roger Leigh wrote:
In most of the projects I work on, the git commit message has
the affected subsystem or component in square brackets, such as
[foo] change bar to baz
[...]
The [sbuild] prefix has been dropped from the Subject, so an
important bit of context about the patch has been lost.
It's a bit of a bug that you can't round trip from a git-format-patch
to import with git-am and then not be able to produce the exact same
patch set with git-format-patch again (assuming preparing and applying
to the same point, of course).
As an immediate solution, you probably want to use "-k" when generating
the patch (not to add the [PATCH] munging) and "-k" when reading the
patch via "git am" (which will avoid trying to strip any munging).
However:
Would it be possible to change the git-mailinfo logic to use a less
greedy pattern match so it leaves everything after
([PATCH( [0-9/])+])+ in the subject? AFAICT this is cleanup_subject in
builtin-mailinfo.c? Could this rather complex function not just do a
simple regex match which can also take care of stripping ([Rr]e:) ?
Yes, I think in the long run it makes sense to strip just the _first_
set of brackets. I don't think we want to be more specific than that in
the match, because we allow arbitrary cruft inside the brackets (like
"[RFC/PATCH]", etc). But if format-patch always puts exactly one set of
brackets, and am strips exactly one set, then that should retain your
subject in practice, even if it starts with [foo].
-Peff
From: Paolo Bonzini <hidden> Date: 2016-06-15 22:47:00
Would it be possible to change the git-mailinfo logic to use a less
greedy pattern match?
Like this? (I also simplified the first part of the if condition since I
was at it). Anyone, feel free to resubmit it as a proper patch.
Almost-Signed-off-by: Paolo Bonzini [off-list ref]
---
builtin-mailinfo.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
From: Andreas Ericsson <hidden> Date: 2016-06-15 22:47:00
Paolo Bonzini wrote:
quoted hunk
quoted
Would it be possible to change the git-mailinfo logic to use a less
greedy pattern match?
Like this? (I also simplified the first part of the if condition since I
was at it). Anyone, feel free to resubmit it as a proper patch.
Almost-Signed-off-by: Paolo Bonzini [off-list ref]
---
builtin-mailinfo.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
Pardon my ignorance, but wouldn't this still remove not only
"[PATCH 4/5]", but all of [PATCH 4/5] [sbuild]" anyway? The
parameters to strbuf_remove() seem unchanged.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
Pardon my ignorance, but wouldn't this still remove not only
"[PATCH 4/5]", but all of [PATCH 4/5] [sbuild]" anyway? The
parameters to strbuf_remove() seem unchanged.
I don't exclude I've screwed up, but note that pos is computed with
strchr, not strrchr. Since the second memmem does not find [PATCH], it
does not remove anything.
(BTW, cairo uses the [...] convention).
Paolo
Pardon my ignorance, but wouldn't this still remove not only
"[PATCH 4/5]", but all of [PATCH 4/5] [sbuild]" anyway? The
parameters to strbuf_remove() seem unchanged.
I don't exclude I've screwed up, but note that pos is computed with
strchr, not strrchr. Since the second memmem does not find [PATCH], it
does not remove anything.
It removes one character, which means the subject still gets mangled. If
it *doesn't* remove one character and also doesn't break out of the loop,
it'll loop indefinitely, since *subject->buf will never change.
There's something else wrong with your patch though, as mailinfo dumps
core with it for a patch starting with "[PATCH] [git]". It happens in
memmem(). Here's the backtrace:
(gdb) bt
#0 0x00c67c76 in memmem (haystack_start=0x8a4bae0, haystack_len=6,
needle_start=0x41544348, needle_len=5) at memmem.c:66
#1 0x0807a1e5 in cleanup_subject () at builtin-mailinfo.c:240
#2 handle_info () at builtin-mailinfo.c:878
#3 mailinfo () at builtin-mailinfo.c:929
#4 cmd_mailinfo (argc=4, argv=<value optimized out>, prefix=0x0)
at builtin-mailinfo.c:966
#5 0x0804b0f7 in run_builtin () at git.c:247
#6 handle_internal_command (argc=4, argv=0xbfb00f58) at git.c:393
#7 0x0804b2e2 in run_argv () at git.c:439
#8 main (argc=4, argv=0xbfb00f58) at git.c:510
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.