Re: [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s
Brandon Casey draf...@gmail.com writes: The cherry-pick -s functionality is currently broken in two ways. 1. handling of rfc2822 continuation lines has a bug, and the continuation lines are not handled correctly. This is not limited to you, but people should think twice when writing has a bug and are not handled correctly in their log message. Did you write what the expected and actual behaviours? +rfc2822_mesg=$non_rfc2822_mesg + +Signed-off-by: A.U. Thor + aut...@example.com +Signed-off-by: B.U. Thor but...@example.com The S-o-b: lines are meant to record people's contact info in human readable forms, and folding the lines like the above makes it a lot harder to read. They typically do not have to be folded. Besides, the footer lines are *not* RFC2822 headers (and are not used as such when send-email comes up with Cc: list) in the first place; have we ever said anything about supporting the RFC2822 line folding in the commit footer? If not (and I am reasonably sure we never have), I personally think we should actively *discourage* line folding there. i.e. we should produce this: Signed-off-by: A.U. Thor aut...@example.com (cherry picked from ) Signed-off-by: C O Mmitter commit...@example.com not Signed-off-by: A.U. Thor aut...@example.com (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) Signed-off-by: C O Mmitter commit...@example.com I can buy that, but then this makes it very clear that these footer lines are not shaped like RFC2822 headers, no? 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
Re: [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s
On Thu, Nov 15, 2012 at 5:58 PM, Junio C Hamano gits...@pobox.com wrote: Brandon Casey draf...@gmail.com writes: The cherry-pick -s functionality is currently broken in two ways. 1. handling of rfc2822 continuation lines has a bug, and the continuation lines are not handled correctly. This is not limited to you, but people should think twice when writing has a bug and are not handled correctly in their log message. Did you write what the expected and actual behaviours? Yeah, I wasn't clear here. The bug was that the incorrect index variable was being used, which caused the wrong line to be examined to see if it was an rfc2822 continuation line. I could have mentioned that. +rfc2822_mesg=$non_rfc2822_mesg + +Signed-off-by: A.U. Thor + aut...@example.com +Signed-off-by: B.U. Thor but...@example.com The S-o-b: lines are meant to record people's contact info in human readable forms, and folding the lines like the above makes it a lot harder to read. They typically do not have to be folded. Well, I wasn't adding functionality here, I was only fixing what I noticed was broken when I started touching this code. Besides, the footer lines are *not* RFC2822 headers (and are not used as such when send-email comes up with Cc: list) in the first place; have we ever said anything about supporting the RFC2822 line folding in the commit footer? If not (and I am reasonably sure we never have), I personally think we should actively *discourage* line folding there. That's fine with me. I can't think of a reason that would make it necessary to support line continuation. i.e. we should produce this: Signed-off-by: A.U. Thor aut...@example.com (cherry picked from ) Signed-off-by: C O Mmitter commit...@example.com not Signed-off-by: A.U. Thor aut...@example.com (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709) Signed-off-by: C O Mmitter commit...@example.com I can buy that, but then this makes it very clear that these footer lines are not shaped like RFC2822 headers, no? The lines that are _not_ (cherry picked from ...) lines do follow the format defined by rfc2822 for header lines (mostly). That's probably why the author of the function in sequencer.c that checks for a s-o-b footer named it ends_rfc2822_footer. I'll remove the *broken* existing code that was intended to support continuation lines and submit a new patch. -Brandon -- 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 2/5] t/t3511: demonstrate breakage in cherry-pick -s
The cherry-pick -s functionality is currently broken in two ways.
1. handling of rfc2822 continuation lines has a bug, and the
continuation lines are not handled correctly.
2. the (cherry picked from ...) lines are commonly appended to the
end of the s-o-b footer and should be detected as part of the footer.
They should not cause a new line to be inserted before the new s-o-b
is added.
i.e. we should produce this:
Signed-off-by: A.U. Thor aut...@example.com
(cherry picked from )
Signed-off-by: C O Mmitter commit...@example.com
not
Signed-off-by: A.U. Thor aut...@example.com
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mmitter commit...@example.com
Signed-off-by: Brandon Casey bca...@nvidia.com
---
t/t3511-cherry-pick-x.sh | 77
1 file changed, 77 insertions(+)
create mode 100755 t/t3511-cherry-pick-x.sh
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
new file mode 100755
index 000..b4e5c65
--- /dev/null
+++ b/t/t3511-cherry-pick-x.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -x and -s'
+
+. ./test-lib.sh
+
+pristine_detach () {
+ git cherry-pick --quit
+ git checkout -f $1^0
+ git read-tree -u --reset HEAD
+ git clean -d -f -f -q -x
+}
+
+non_rfc2822_mesg='base with footer
+
+Commit message body is here.'
+
+rfc2822_mesg=$non_rfc2822_mesg
+
+Signed-off-by: A.U. Thor
+ aut...@example.com
+Signed-off-by: B.U. Thor but...@example.com
+
+rfc2822_cherry_mesg=$rfc2822_mesg
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: C.U. Thor cut...@example.com
+
+
+test_expect_success setup '
+ git config advice.detachedhead false
+ echo unrelated unrelated
+ git add unrelated
+ test_commit initial foo a
+ test_commit $non_rfc2822_mesg foo b non-rfc2822-base
+ git reset --hard initial
+ test_commit $rfc2822_mesg foo b rfc2822-base
+ git reset --hard initial
+ test_commit $rfc2822_cherry_mesg foo b rfc2822-cherry-base
+ pristine_detach initial
+ test_commit conflicting unrelated
+'
+
+test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822
footer' '
+ pristine_detach initial
+ git cherry-pick -s non-rfc2822-base
+ cat -EOF expect
+ $non_rfc2822_mesg
+
+ Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+ EOF
+ git log -1 --pretty=format:%B actual
+ test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick -s not confused by rfc2822 continuation line'
'
+ pristine_detach initial
+ git cherry-pick -s rfc2822-base
+ cat -EOF expect
+ $rfc2822_mesg
+ Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+ EOF
+ git log -1 --pretty=format:%B actual
+ test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick treats -s (cherry picked from... line as
part of footer' '
+ pristine_detach initial
+ git cherry-pick -s rfc2822-cherry-base
+ cat -EOF expect
+ $rfc2822_cherry_mesg
+ Signed-off-by: $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
+ EOF
+ git log -1 --pretty=format:%B actual
+ test_cmp expect actual
+'
+
+test_done
--
1.8.0
--
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
