{"thread":{"id":"9455","subject":"[PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","startedAt":"2007-08-09T12:08:31Z","lastAt":"2007-08-16T06:19:55Z","messageCount":7,"participants":["Gerrit Pape","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"50314","messageId":"20070809120831.19319.qmail@a61af064a2a242.315fe32.mid.smarden.org","threadId":"9455","inReplyTo":null,"subject":"[PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-08-09T12:08:31Z","receivedAt":"2007-08-09T12:08:31Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"If called with one argument, check also for no_trivial_merge_strategies\nwhether all the merge can already be reached by HEAD.  Otherwise git-merge\ncreates an unnecessary commit, e.g.:\n\n $ (git init && touch foo && git add foo && git commit -m foo) >/dev/null\n $ git merge -s ours master\n Merge made by ours.\n $ git log --pretty=oneline\n d941346f022b7cb70c51d8122de4ae82657ae943 Merge branch 'master'\n 68a1d32229917d5d1f4c8f64096d1abde84e9f6b foo\n $\n\nSigned-off-by: Gerrit Pape <pape@smarden.org>\n---\n git-merge.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-merge.sh b/git-merge.sh\nindex 5ccf282..e90f62a 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -287,9 +287,6 @@ esac\n echo \"$head\" >\"$GIT_DIR/ORIG_HEAD\"\n \n case \"$index_merge,$#,$common,$no_commit\" in\n-f,*)\n-\t# We've been told not to try anything clever.  Skip to real merge.\n-\t;;\n ?,*,'',*)\n \t# No common ancestors found. We need a real merge.\n \t;;\n@@ -299,6 +296,9 @@ f,*)\n \tfinish_up_to_date \"Already up-to-date.\"\n \texit 0\n \t;;\n+f,*)\n+\t# We've been told not to try anything clever.  Skip to real merge.\n+\t;;\n ?,1,\"$head\",*)\n \t# Again the most common case of merging one remote.\n \techo \"Updating $(git rev-parse --short $head)..$(git rev-parse --short $1)\"\n-- \ndebian.1.5.3_rc4.1-dirty\n"},{"id":"50345","messageId":"7v3aysxvk3.fsf@assigned-by-dhcp.cox.net","threadId":"9455","inReplyTo":"20070809120831.19319.qmail@a61af064a2a242.315fe32.mid.smarden.org","subject":"Re: [PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-09T21:11:24Z","receivedAt":"2007-08-09T21:11:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Right now I do not have time to dig mailing list archive around\nmid March 2006, and I do not recall the requestor's original\nrationale, but I have a vague recollection that we added this\n\"no fast-forward check\" specifically in response to a user\nrequest.\n"},{"id":"50357","messageId":"20070810000126.GB10250@sigill.intra.peff.net","threadId":"9455","inReplyTo":"7v3aysxvk3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-08-10T00:01:26Z","receivedAt":"2007-08-10T00:01:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 09, 2007 at 02:11:24PM -0700, Junio C Hamano wrote:\n\n> Right now I do not have time to dig mailing list archive around\n> mid March 2006, and I do not recall the requestor's original\n> rationale, but I have a vague recollection that we added this\n> \"no fast-forward check\" specifically in response to a user\n> request.\n\nMaybe it was the \"I'm using a custom merge strategy that might refuse\nthe merge, but fast-forwards don't even invoke my merge strategy\"\nrequest?\n\n-Peff\n"},{"id":"50393","messageId":"7vejibu69w.fsf@assigned-by-dhcp.cox.net","threadId":"9455","inReplyTo":"7v3aysxvk3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-10T08:45:47Z","receivedAt":"2007-08-10T08:45:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Right now I do not have time to dig mailing list archive around\n> mid March 2006, and I do not recall the requestor's original\n> rationale, but I have a vague recollection that we added this\n> \"no fast-forward check\" specifically in response to a user\n> request.\n\nI do not think of a valid reason not to apply your patch.  We\nwanted to avoid the trivial-merge codepath, but that is not a\nvalid reason to record a fast forward situation as a merge of\nany kind.\n"},{"id":"50394","messageId":"7vabszu5zp.fsf@assigned-by-dhcp.cox.net","threadId":"9455","inReplyTo":"7vejibu69w.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-10T08:51:54Z","receivedAt":"2007-08-10T08:51:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Right now I do not have time to dig mailing list archive around\n>> mid March 2006, and I do not recall the requestor's original\n>> rationale, but I have a vague recollection that we added this\n>> \"no fast-forward check\" specifically in response to a user\n>> request.\n>\n> I do not think of a valid reason not to apply your patch.  We\n> wanted to avoid the trivial-merge codepath, but that is not a\n> valid reason to record a fast forward situation as a merge of\n> any kind.\n\nGaah.  I take it back.  At least, subtree _MUST_ not honor\nfast-forward blindly.  git-gui.git project is merged into\ngit.git with that strategy, but if I pull from Shawn once, and\nthen he has more updates while I do not have anything on my\ntree, it will be a fast-forward.  I should not be setting the\ngit.git tree to that of git-gui.git in such a case.\n"},{"id":"50411","messageId":"20070810135815.15682.qmail@16a33814590a07.315fe32.mid.smarden.org","threadId":"9455","inReplyTo":"7v3aysxvk3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-merge: do up-to-date check also for strategies ours, subtree.","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2007-08-10T13:58:15Z","receivedAt":"2007-08-10T13:58:15Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Thu, Aug 09, 2007 at 02:11:24PM -0700, Junio C Hamano wrote:\n> Right now I do not have time to dig mailing list archive around\n> mid March 2006, and I do not recall the requestor's original\n> rationale, but I have a vague recollection that we added this\n> \"no fast-forward check\" specifically in response to a user\n> request.\n\nHmm, I don't think my patch affects fast-forwards at all, it checks for\nno_trivial_merge_strategies whether the remote, that should be merged,\nhas any commits that are not in the current head.  If not, no merge\nstrategy should have to do anything.\n\nThe patch moves the first case switch at the third position.  If, with\nthe patch, the first switch matches, it doesn't change behavior as it\ndoes the same as the third; if the second switch matches, git-merge will\nreport 'Already up-to-date.' and exit.\n\nThis is with the patch:\n\n $ (git init && touch foo && git add foo && git commit -m foo) >/dev/null\n $ git checkout -b b\n Switched to a new branch \"b\"\n $ (touch bar && git add bar && git commit -m bar) >/dev/null\n $ git checkout master\n Switched to branch \"master\"\n $ git merge b\n Updating 769e777..8a03d99\n Fast forward\n  0 files changed, 0 insertions(+), 0 deletions(-)\n  create mode 100644 bar\n $ git reset --hard HEAD^\n HEAD is now at 769e777... foo\n $ git merge -s ours b\n Merge made by ours.\n $ git show\n commit f0dc487c15e502bc7ee823b6907468eb78668dbb\n Author: Gerrit Pape <pape@smarden.org>\n Date:   Fri Aug 10 13:44:10 2007 +0000\n \n     Merge branch 'b'\n \n diff --git a/bar b/bar\n deleted file mode 100644\n index e69de29..0000000\n $ git merge -s ours b\n Already up-to-date.\n $ \n\nWithout the patch, the last command results in\n\n $ git merge -s ours b\n Merge made by ours.\n $ git show\n commit 21154261ff23f64fc38e8a2ae79c7a1cccaeacd4\n Author: Gerrit Pape <pape@smarden.org>\n Date:   Fri Aug 10 13:48:54 2007 +0000\n \n     Merge branch 'b'\n $ \n\nRegards, Gerrit.\n"},{"id":"50852","messageId":"7vwsvwj910.fsf@gitster.siamese.dyndns.org","threadId":"9455","inReplyTo":"20070809120831.19319.qmail@a61af064a2a242.315fe32.mid.smarden.org","subject":"[PATCH] git-merge: do up-to-date check also for all strategies","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-16T06:19:55Z","receivedAt":"2007-08-16T06:19:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"How about doing this instead?  When \"subtree\" is used for\ncriss-cross (which is not recommended), you should not allow\nfast-forward.  Also \"ours\" should not allow fast-forward, as the\nresult will be their tree and by definition that is not what\n\"ours\" want.  All strategies including \"ours\" and \"subtree\"\nshould allow up-to-date (i.e. we are ahead of them).\n\nLooking at git-merge again after a long while makes me wonder \nif it makes sense to support only a single strategy.  Back when\nwe did not know which one of recursive or multi-head resolve was\nthe right way to go, being able to try multiple strategies and\npick the \"best\" result sounded like a fun plan, but I do not\nthink anybody uses this capability.\n\n-- >8 --\n[PATCH] git-merge: do up-to-date check also for all strategies\n\nThis clarifies the logic to omit fast-forward check and omit\ntrivial merge before running the specified strategy.\n\nThe \"index_merge\" variable started out as a flag to say \"do not\ndo anything clever\", but when recursive was changed to skip the\ntrivial merge, the semantics were changed and the variable alone\ndoes not make sense anymore.\n\nThis splits the variable into two, allow_fast_forward (which is\nalmost always true, and avoids making a merge commit when the\nother commit is a descendant of our branch, but is set to false\nfor ours and subtree) and allow_trivial_merge (which is false\nfor ours, recursive and subtree).\n\nUnlike the earlier implementation, the \"ours\" strategy allows an\nup-to-date condition.  When we are up-to-date, the result will\nbe our commit, and by definition, we will have our tree as the\nresult.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n git-merge.sh                |   35 +++++++++++--------\n t/t6028-merge-up-to-date.sh |   77 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 97 insertions(+), 15 deletions(-)\n create mode 100755 t/t6028-merge-up-to-date.sh\n\ndiff --git a/git-merge.sh b/git-merge.sh\nindex 5ccf282..3a01db0 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -19,10 +19,12 @@ LF='\n all_strategies='recur recursive octopus resolve stupid ours subtree'\n default_twohead_strategies='recursive'\n default_octopus_strategies='octopus'\n-no_trivial_merge_strategies='ours subtree'\n+no_fast_forward_strategies='subtree ours'\n+no_trivial_strategies='recursive recur subtree ours'\n use_strategies=\n \n-index_merge=t\n+allow_fast_forward=t\n+allow_trivial_merge=t\n \n dropsave() {\n \trm -f -- \"$GIT_DIR/MERGE_HEAD\" \"$GIT_DIR/MERGE_MSG\" \\\n@@ -265,11 +267,20 @@ esac\n \n for s in $use_strategies\n do\n-\tfor nt in $no_trivial_merge_strategies\n+\tfor ss in $no_fast_forward_strategies\n \tdo\n \t\tcase \" $s \" in\n-\t\t*\" $nt \"*)\n-\t\t\tindex_merge=f\n+\t\t*\" $ss \"*)\n+\t\t\tallow_fast_forward=f\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\tdone\n+\tfor ss in $no_trivial_strategies\n+\tdo\n+\t\tcase \" $s \" in\n+\t\t*\" $ss \"*)\n+\t\t\tallow_trivial_merge=f\n \t\t\tbreak\n \t\t\t;;\n \t\tesac\n@@ -286,10 +297,7 @@ case \"$#\" in\n esac\n echo \"$head\" >\"$GIT_DIR/ORIG_HEAD\"\n \n-case \"$index_merge,$#,$common,$no_commit\" in\n-f,*)\n-\t# We've been told not to try anything clever.  Skip to real merge.\n-\t;;\n+case \"$allow_fast_forward,$#,$common,$no_commit\" in\n ?,*,'',*)\n \t# No common ancestors found. We need a real merge.\n \t;;\n@@ -299,7 +307,7 @@ f,*)\n \tfinish_up_to_date \"Already up-to-date.\"\n \texit 0\n \t;;\n-?,1,\"$head\",*)\n+t,1,\"$head\",*)\n \t# Again the most common case of merging one remote.\n \techo \"Updating $(git rev-parse --short $head)..$(git rev-parse --short $1)\"\n \tgit update-index --refresh 2>/dev/null\n@@ -322,11 +330,8 @@ f,*)\n \t# We are not doing octopus, not fast forward, and have only\n \t# one common.\n \tgit update-index --refresh 2>/dev/null\n-\tcase \" $use_strategies \" in\n-\t*' recursive '*|*' recur '*)\n-\t\t: run merge later\n-\t\t;;\n-\t*)\n+\tcase \"$allow_trivial_merge\" in\n+\tt)\n \t\t# See if it is really trivial.\n \t\tgit var GIT_COMMITTER_IDENT >/dev/null || exit\n \t\techo \"Trying really trivial in-index merge...\"\ndiff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh\nnew file mode 100755\nindex 0000000..f8f3e3f\n--- /dev/null\n+++ b/t/t6028-merge-up-to-date.sh\n@@ -0,0 +1,77 @@\n+#!/bin/sh\n+\n+test_description='merge fast forward and up to date'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t>file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tgit tag c0 &&\n+\n+\techo second >file &&\n+\tgit add file &&\n+\ttest_tick &&\n+\tgit commit -m second &&\n+\tgit tag c1 &&\n+\tgit branch test\n+'\n+\n+test_expect_success 'merge -s recursive up-to-date' '\n+\n+\tgit reset --hard c1 &&\n+\ttest_tick &&\n+\tgit merge -s recursive c0 &&\n+\texpect=$(git rev-parse c1) &&\n+\tcurrent=$(git rev-parse HEAD) &&\n+\ttest \"$expect\" = \"$current\"\n+\n+'\n+\n+test_expect_success 'merge -s recursive fast-forward' '\n+\n+\tgit reset --hard c0 &&\n+\ttest_tick &&\n+\tgit merge -s recursive c1 &&\n+\texpect=$(git rev-parse c1) &&\n+\tcurrent=$(git rev-parse HEAD) &&\n+\ttest \"$expect\" = \"$current\"\n+\n+'\n+\n+test_expect_success 'merge -s ours up-to-date' '\n+\n+\tgit reset --hard c1 &&\n+\ttest_tick &&\n+\tgit merge -s ours c0 &&\n+\texpect=$(git rev-parse c1) &&\n+\tcurrent=$(git rev-parse HEAD) &&\n+\ttest \"$expect\" = \"$current\"\n+\n+'\n+\n+test_expect_success 'merge -s ours fast-forward' '\n+\n+\tgit reset --hard c0 &&\n+\ttest_tick &&\n+\tgit merge -s ours c1 &&\n+\texpect=$(git rev-parse c0^{tree}) &&\n+\tcurrent=$(git rev-parse HEAD^{tree}) &&\n+\ttest \"$expect\" = \"$current\"\n+\n+'\n+\n+test_expect_success 'merge -s subtree up-to-date' '\n+\n+\tgit reset --hard c1 &&\n+\ttest_tick &&\n+\tgit merge -s subtree c0 &&\n+\texpect=$(git rev-parse c1) &&\n+\tcurrent=$(git rev-parse HEAD) &&\n+\ttest \"$expect\" = \"$current\"\n+\n+'\n+\n+test_done\n"}]}