Bug: Incorrect stripping of the [PATCH] prefix in git-am
Description: When applying a patch created via `git format-patch` with `git am`, any prefix of the commit message that's within square brackets is stripped from the commit message. Reproduction: $ git log --oneline --decorate --graph --all * b41514b (HEAD) [baz] baz | * 5e31740 (master) [bar] bar |/ * aaf5d34 [foo] foo $ git format-patch aaf5d34 $ git checkout master $ git am 0001-baz-baz.patch $ git log --oneline --decorate --graph --all * d5161b8 (HEAD, master) baz * 5e31740 [bar] bar * aaf5d34 [foo] foo I have omitted all output except for the `git log` output for brevity. As you can see, the commit resulting from `git am` has lost the "[bar]" prefix from its commit message. Looking at the patch, $ cat 0001-baz-baz.patch From b41514be8a70fd44052bff0d3ce720373ec9cfd8 Mon Sep 17 00:00:00 2001 From: Nathanael Huebbe Date: Wed, 25 Nov 2015 15:28:09 +0100 Subject: [PATCH] [baz] baz --- baz | 1 + 1 file changed, 1 insertion(+) create mode 100644 baz diff --git a/baz b/baz new file mode 100644 index 000..7601807 --- /dev/null +++ b/baz @@ -0,0 +1 @@ +baz -- 2.1.4 I see that the commit message contains the "[PATCH]"-prefix that `git am` is supposed to strip, yet it seems to incorrectly continue to also strip the "[baz]"-prefix. Affected versions: I have reproduced the bug with versions 1.9.1, 2.1.4, and 2.6.3 Severity: While I do not consider this a high priority bug, it becomes quite irksome in some workflows. In my case, an upstream `svn` repository has the policy of using "[branch-name]" prefixes to the commit messages, which are stripped whenever I transplant a commit using the `git format-patch`/`git am` combination. -- Please be aware that the enemies of your civil rights and your freedom are on CC of all unencrypted communication. Protect yourself. signature.asc Description: OpenPGP digital signature
Re: Bug: Incorrect stripping of the [PATCH] prefix in git-am
Yes, it looks like the `--keep-non-patch` option works around this. However, shouldn't that be the default behaviour? I mean, what is the point in stripping stuff that is not proven to be inserted by `git` itself? That's not what I expect a tool to do which I trust. Cheers, Nathanael On 11/25/2015 04:44 PM, stefan.na...@atlas-elektronik.com wrote: > Am 25.11.2015 um 16:29 schrieb huebbe: >> Description: >> >> When applying a patch created via `git format-patch` with `git am`, >> any prefix of the commit message that's within square brackets is stripped >> from the commit message. > > As advertised in the man page of 'git am' (or 'git mailinfo' that is). > > Is 'git am --keep-non-patch' what you're looking for ? > >> >> >> Reproduction: >> >> $ git log --oneline --decorate --graph --all >> * b41514b (HEAD) [baz] baz >> | * 5e31740 (master) [bar] bar >> |/ >> * aaf5d34 [foo] foo >> $ git format-patch aaf5d34 >> $ git checkout master >> $ git am 0001-baz-baz.patch >> $ git log --oneline --decorate --graph --all >> * d5161b8 (HEAD, master) baz >> * 5e31740 [bar] bar >> * aaf5d34 [foo] foo >> >> I have omitted all output except for the `git log` output for brevity. >> As you can see, the commit resulting from `git am` has lost the "[bar]" >> prefix from its commit message. >> >> Looking at the patch, >> >> $ cat 0001-baz-baz.patch >> From b41514be8a70fd44052bff0d3ce720373ec9cfd8 Mon Sep 17 00:00:00 2001 >> From: Nathanael Huebbe >> Date: Wed, 25 Nov 2015 15:28:09 +0100 >> Subject: [PATCH] [baz] baz >> >> --- >> baz | 1 + >> 1 file changed, 1 insertion(+) >> create mode 100644 baz >> >> diff --git a/baz b/baz >> new file mode 100644 >> index 000..7601807 >> --- /dev/null >> +++ b/baz >> @@ -0,0 +1 @@ >> +baz >> -- >> 2.1.4 >> >> I see that the commit message contains the "[PATCH]"-prefix that `git am` is >> supposed to strip, >> yet it seems to incorrectly continue to also strip the "[baz]"-prefix. >> >> >> Affected versions: >> I have reproduced the bug with versions 1.9.1, 2.1.4, and 2.6.3 >> >> >> Severity: >> While I do not consider this a high priority bug, it becomes quite irksome >> in some workflows. >> In my case, an upstream `svn` repository has the policy of using >> "[branch-name]" prefixes >> to the commit messages, which are stripped whenever I transplant a commit >> using the >> `git format-patch`/`git am` combination. >> >> >> > > HTH, > Stefan > -- Please be aware that the enemies of your civil rights and your freedom are on CC of all unencrypted communication. Protect yourself. signature.asc Description: OpenPGP digital signature
Re: Bug: Incorrect stripping of the [PATCH] prefix in git-am
On 12/02/2015 01:58 AM, Jeff King wrote: > On Wed, Nov 25, 2015 at 04:59:35PM +0100, huebbe wrote: > >> Yes, it looks like the `--keep-non-patch` option works around this. >> >> However, shouldn't that be the default behaviour? >> I mean, what is the point in stripping stuff that is not proven to be >> inserted by `git` itself? >> That's not what I expect a tool to do which I trust. > > The "[]" convention is a microformat used by Linux kernel folks. So it's > not "whoops, we are stripping stuff not added by git". It is respecting > a microformat used by the tool's authors. > > That being said, if we were choosing a default from scratch today, it > might go the other way. But we aren't, and we have to deal with the > burden of breaking existing scripts by flipping it. > > -Peff Ok, I think I understand the reason for this behavior now. However, I still believe that the `git format-patch`/`git am` combo should retain the original commit messages by default. As such, I would like to ask whether it would be possible/sensible to somehow escape square brackets, or mark the beginning of the original commit message in the `git format-patch` output? This would allow `git am` to reproduce the exact commit message by default without breaking the "[]" convention. For example, I could imagine to use this behavior: * `git format-patch` produces a message of the format "[PATCH ...]: original commit message". * `git am` strips the longest prefix that consists entirely of bracketed stuff and ends with the string "]: ". If that terminator is not found -> old behavior. * Users could insert any "[FOO]" strings they like before the ":", which would still be stripped from the commit message by `git am`. Of course, any such change would not be without pitfalls. With the rule above, we would get ": " prepended to commit messages if a patch is created with a `git` version that has the change, and applied with an older version that does not know about the terminator. What do you guys think? Is there anything I missed? Cheers, Nathanael -- Please be aware that the enemies of your civil rights and your freedom are on CC of all unencrypted communication. Protect yourself. signature.asc Description: OpenPGP digital signature
Re: Bug: Incorrect stripping of the [PATCH] prefix in git-am
Dear Peff, I have no problem working around this bug/feature. I just happen to think that the current *default* behaviour is not the default behaviour that users have a right to expect: I believe that users have every right to expect `git format-patch`/`git am` to preserve commit messages perfectly by default. Since I see that people are using the current behaviour as a feature, I tried to come up with an alternative behaviour that would do both: 1. Respect user assumptions to preserve commit messages by default. 2. Allow users to prepend stuff when mailing a patch, that will get stripped automatically. Just because the current behaviour is ... irritating. Cheers, Nathanael On 12/02/2015 04:49 PM, Jeff King wrote: > On Wed, Dec 02, 2015 at 01:38:18PM +0100, huebbe wrote: > >> As such, I would like to ask whether it would be possible/sensible >> to somehow escape square brackets, or mark the beginning >> of the original commit message in the `git format-patch` output? >> This would allow `git am` to reproduce the exact commit message by default >> without breaking the "[]" convention. > > I am not sure why "git format-patch -k | git am -k" does not do what you > want. That is what those options were added for (and what git-rebase > uses internally to make sure commit messages are left unmunged). > > -Peff > -- Please be aware that the enemies of your civil rights and your freedom are on CC of all unencrypted communication. Protect yourself. signature.asc Description: OpenPGP digital signature
