{"thread":{"id":"24752","subject":"[PATCH 0/2] set NO_TRIVIAL for custom merge strategies","startedAt":"2010-08-16T01:06:52Z","lastAt":"2010-08-16T03:39:27Z","messageCount":4,"participants":["Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"148165","messageId":"20100816010652.GA22546@burratino","threadId":"24752","inReplyTo":null,"subject":"[PATCH 0/2] set NO_TRIVIAL for custom merge strategies","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-16T01:06:52Z","receivedAt":"2010-08-16T01:06:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As noticed at <http://bugs.debian.org/581691>, the example custom\n“theirs” strategy does not take effect for trivial merges.\nA user-defined “ours”-like strategy would also not be possible without\nthis change.\n\nLonger term, it would be nice to be able to override the\nNO_FAST_FORWARD and NO_TRIVIAL flags for custom strategies in\n~/.gitconfig, but let’s wait until we need it.\n\nThoughts?\n\nJonathan Nieder (2):\n  t7606 (merge-theirs): modernize style\n  merge: let custom strategies intervene in trivial merges\n\n builtin/merge.c         |    1 +\n t/t7606-merge-custom.sh |   96 ++++++++++++++++++++++++++++++++++-------------\n 2 files changed, 71 insertions(+), 26 deletions(-)\n"},{"id":"148167","messageId":"20100816010846.GA22573@burratino","threadId":"24752","inReplyTo":"20100816010652.GA22546@burratino","subject":"[PATCH 1/2] t7606 (merge-theirs): modernize style","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-16T01:08:46Z","receivedAt":"2010-08-16T01:08:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Guard setup commands with test_expect_success, so they are easier\nto visually skip over and get to the good part.  While at it:\n\n - use test_commit for brevity and reproducible object names;\n\n - use test_cmp instead of using the test builtin to compare the\n   result of command substitution, for better output with -v on\n   failure.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t7606-merge-custom.sh |   66 ++++++++++++++++++++++++++++------------------\n 1 files changed, 40 insertions(+), 26 deletions(-)\n\ndiff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh\nindex 52a451d..82045cd 100755\n--- a/t/t7606-merge-custom.sh\n+++ b/t/t7606-merge-custom.sh\n@@ -1,46 +1,60 @@\n #!/bin/sh\n \n-test_description='git merge\n+test_description=\"git merge\n \n-Testing a custom strategy.'\n+Testing a custom strategy.\n+\n+*   (HEAD, master) Merge commit 'c2'\n+|\\\n+| * (tag: c2) c2\n+* | (tag: c1) c1\n+|/\n+* (tag: c0) c0\n+\"\n \n . ./test-lib.sh\n \n-cat >git-merge-theirs <<EOF\n-#!$SHELL_PATH\n-eval git read-tree --reset -u \\\\\\$\\$#\n-EOF\n-chmod +x git-merge-theirs\n-PATH=.:$PATH\n-export PATH\n+test_expect_success 'set up custom strategy' '\n+\tcat >git-merge-theirs <<-EOF &&\n+\t#!$SHELL_PATH\n+\teval git read-tree --reset -u \\\\\\$\\$#\n+\tEOF\n+\n+\tchmod +x git-merge-theirs &&\n+\tPATH=.:$PATH &&\n+\texport PATH\n+'\n \n test_expect_success 'setup' '\n-\techo c0 >c0.c &&\n-\tgit add c0.c &&\n-\tgit commit -m c0 &&\n-\tgit tag c0 &&\n-\techo c1 >c1.c &&\n-\tgit add c1.c &&\n-\tgit commit -m c1 &&\n-\tgit tag c1 &&\n-\tgit reset --hard c0 &&\n+\ttest_commit c0 c0.c &&\n+\ttest_commit c1 c1.c &&\n+\tgit reset --keep c0 &&\n \techo c1c1 >c1.c &&\n-\techo c2 >c2.c &&\n-\tgit add c1.c c2.c &&\n-\tgit commit -m c2 &&\n-\tgit tag c2\n+\tgit add c1.c &&\n+\ttest_commit c2 c2.c\n '\n \n test_expect_success 'merge c2 with a custom strategy' '\n \tgit reset --hard c1 &&\n+\n+\tgit rev-parse c1 >head.old &&\n+\tgit rev-parse c2 >second-parent.expected &&\n+\tgit rev-parse c2^{tree} >tree.expected &&\n \tgit merge -s theirs c2 &&\n-\ttest \"$(git rev-parse c1)\" != \"$(git rev-parse HEAD)\" &&\n-\ttest \"$(git rev-parse c1)\" = \"$(git rev-parse HEAD^1)\" &&\n-\ttest \"$(git rev-parse c2)\" = \"$(git rev-parse HEAD^2)\" &&\n-\ttest \"$(git rev-parse c2^{tree})\" = \"$(git rev-parse HEAD^{tree})\" &&\n+\n+\tgit rev-parse HEAD >head &&\n+\tgit rev-parse HEAD^1 >first-parent &&\n+\tgit rev-parse HEAD^2 >second-parent &&\n+\tgit rev-parse HEAD^{tree} >tree &&\n+\tgit update-index --refresh &&\n \tgit diff --exit-code &&\n \tgit diff --exit-code c2 HEAD &&\n \tgit diff --exit-code c2 &&\n+\n+\t! test_cmp head.old head &&\n+\ttest_cmp head.old first-parent &&\n+\ttest_cmp second-parent.expected second-parent &&\n+\ttest_cmp tree.expected tree &&\n \ttest -f c0.c &&\n \tgrep c1c1 c1.c &&\n \ttest -f c2.c\n-- \n1.7.2.1.544.ga752d.dirty\n"},{"id":"148168","messageId":"20100816011106.GB22573@burratino","threadId":"24752","inReplyTo":"20100816010652.GA22546@burratino","subject":"[PATCH 2/2] merge: let custom strategies intervene in trivial merges","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-16T01:11:06Z","receivedAt":"2010-08-16T01:11:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As v1.6.1-rc1~294^2 (2008-08-23) explains, custom merge strategies\ndo not even kick in when the merge is truly trivial.  But they\nshould, since otherwise a custom “--strategy=theirs” is not useful.\n\nPerhaps custom strategies should not allow fast-forward either.  This\npatch does not make that change, since it is less important (because\nit is always possible to explicitly use --no-ff).\n\nReported-by: Yaroslav Halchenko <debian@onerussian.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/merge.c         |    1 +\n t/t7606-merge-custom.sh |   36 +++++++++++++++++++++++++++++++++---\n 2 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 37ce4f5..e48e90b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -131,6 +131,7 @@ static struct strategy *get_strategy(const char *name)\n \n \tret = xcalloc(1, sizeof(struct strategy));\n \tret->name = xstrdup(name);\n+\tret->attr = NO_TRIVIAL;\n \treturn ret;\n }\n \ndiff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh\nindex 82045cd..13c2193 100755\n--- a/t/t7606-merge-custom.sh\n+++ b/t/t7606-merge-custom.sh\n@@ -4,11 +4,13 @@ test_description=\"git merge\n \n Testing a custom strategy.\n \n-*   (HEAD, master) Merge commit 'c2'\n+*   (HEAD, master) Merge commit 'c3'\n |\\\n-| * (tag: c2) c2\n+| * (tag: c3) c3\n * | (tag: c1) c1\n |/\n+| * tag: c2) c2\n+|/\n * (tag: c0) c0\n \"\n \n@@ -31,7 +33,9 @@ test_expect_success 'setup' '\n \tgit reset --keep c0 &&\n \techo c1c1 >c1.c &&\n \tgit add c1.c &&\n-\ttest_commit c2 c2.c\n+\ttest_commit c2 c2.c &&\n+\tgit reset --keep c0 &&\n+\ttest_commit c3 c3.c\n '\n \n test_expect_success 'merge c2 with a custom strategy' '\n@@ -60,4 +64,30 @@ test_expect_success 'merge c2 with a custom strategy' '\n \ttest -f c2.c\n '\n \n+test_expect_success 'trivial merge with custom strategy' '\n+\tgit reset --hard c1 &&\n+\n+\tgit rev-parse c1 >head.old &&\n+\tgit rev-parse c3 >second-parent.expected &&\n+\tgit rev-parse c3^{tree} >tree.expected &&\n+\tgit merge -s theirs c3 &&\n+\n+\tgit rev-parse HEAD >head &&\n+\tgit rev-parse HEAD^1 >first-parent &&\n+\tgit rev-parse HEAD^2 >second-parent &&\n+\tgit rev-parse HEAD^{tree} >tree &&\n+\tgit update-index --refresh &&\n+\tgit diff --exit-code &&\n+\tgit diff --exit-code c3 HEAD &&\n+\tgit diff --exit-code c3 &&\n+\n+\t! test_cmp head.old head &&\n+\ttest_cmp head.old first-parent &&\n+\ttest_cmp second-parent.expected second-parent &&\n+\ttest_cmp tree.expected tree &&\n+\ttest -f c0.c &&\n+\t! test -e c1.c &&\n+\ttest -f c3.c\n+'\n+\n test_done\n-- \n1.7.2.1.544.ga752d.dirty\n"},{"id":"148174","messageId":"7vbp93dy4w.fsf@alter.siamese.dyndns.org","threadId":"24752","inReplyTo":"20100816011106.GB22573@burratino","subject":"Re: [PATCH 2/2] merge: let custom strategies intervene in trivial merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-16T03:39:27Z","receivedAt":"2010-08-16T03:39:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> As v1.6.1-rc1~294^2 (2008-08-23) explains, custom merge strategies\n> do not even kick in when the merge is truly trivial.  But they\n> should, since otherwise a custom “--strategy=theirs” is not useful.\n\nI think it is a double-edged sword that trivial merge kicks in for custom\nstrategies.  If your mind is focused narrowly on \"ours\" or \"theirs\",\nindeed it may be inconvenient, but on the other hand other custom\nstrategies may not want to run heavy-weight operations if a trivial merge\nis sufficient.\n\nSo in the longer term, I suspect that each custom strategy needs to be\nable to tell the merge machinery if it wants ff and trivial.\n"}]}