{"thread":{"id":"3670","subject":"On merging strategies, fast forward and index merge","startedAt":"2006-03-18T10:17:26Z","lastAt":"2006-03-18T22:53:52Z","messageCount":4,"participants":["Mark Wooding","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"17646","messageId":"slrne1nnhm.fr9.mdw@metalzone.distorted.org.uk","threadId":"3670","inReplyTo":null,"subject":"On merging strategies, fast forward and index merge","fromName":"Mark Wooding","fromEmail":"mdw@distorted.org.uk","sentAt":"2006-03-18T10:17:26Z","receivedAt":"2006-03-18T10:17:26Z","isPatch":false,"sender":{"key":"mdw@distorted.org.uk","avatar":null},"body":"I recently read Junio's description of how to dig oneself out of a hole\nusing `git merge -s ours' (I'm learning to use the space...), and I've\nrealised there's a problem here.\n\nThe `ours' merge strategy is meant to create a merge commit whose tree\nis in every way identical to that of the starting commit.  But `git\nmerge' won't always do this, because it doesn't always invoke the\nstrategy program.\n\nConsider the command `git merge -s ours MESSAGE MASTER FAILED'.\n\n  * If we've not actually messed with our MASTER since the FAILED branch\n    departed, then MASTER is actually an ancestor of FAILED, and `git\n    merge' will unhelpfully fast-forward us to the tip of the FAILED\n    branch.  Instead of leaving the merge result like MASTER, it's made\n    it entirely the wrong thing!\n\n  * If both MASTER and FAILED have made changes, but to different files,\n    then `git merge' will try an index-level merge, find that it\n    succeeds, and leave us with a mixture of MASTER and FAILED files.\n    Which is (in this case) entirely what we didn't want.\n\nAdditionally, it occurs to me that the fast-forwarding behaviour isn't\nalways what I want anyway.  Consider a merge of a topic branch:\n\n  `git merge MESSAGE MASTER TOPIC'\n\nIf I allow fast-forward, I lose information about where the topic\nstarted and ended.  This is a shame, particularly if I find other places\nI want to apply those changes (either as a string of similar commits, or\nsquidged into a single one) onto other branches.\n\nBecause code speaks louder, I'll follow-up this article with a suggested\npatch.\n\n-- [mdw]\n"},{"id":"17647","messageId":"20060318101941.8941.52615.stgit@metalzone.distorted.org.uk","threadId":"3670","inReplyTo":"slrne1nnhm.fr9.mdw@metalzone.distorted.org.uk","subject":"[PATCH] git-merge: New options `--no-fast-forward' and `--direct'.","fromName":"Mark Wooding","fromEmail":"mdw@distorted.org.uk","sentAt":"2006-03-18T10:19:42Z","receivedAt":"2006-03-18T10:19:42Z","isPatch":true,"sender":{"key":"mdw@distorted.org.uk","avatar":null},"body":"From: Mark Wooding <mdw@distorted.org.uk>\n\nThese options disable some of git-merge's optimizations.  \n\n--no-fast-forward\n\tDoes what it says on the tin: git-merge will always make a\n\tcommit as a result of this merge (or leave one in the pipeline,\n\tif --no-commit was given).\n\n--direct\n\tDon't do anything clever: go directly to the merge strategy\n\tprograms.  In particular, this forbids an attempt at in-index\n\tmerging.\n\nWe also force direct merging with the `ours' strategy, since this is\nobviously what was wanted.\n\nSigned-off-by: Mark Wooding <mdw@distorted.org.uk>\n---\n\n Documentation/merge-options.txt |    9 ++++++++-\n git-merge.sh                    |   28 ++++++++++++++++++++++------\n git-pull.sh                     |   13 +++++++++++--\n 3 files changed, 41 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 53cc355..5b145a1 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -6,7 +6,6 @@\n \tnot autocommit, to give the user a chance to inspect and\n \tfurther tweak the merge result before committing.\n \n-\n -s <strategy>, \\--strategy=<strategy>::\n \tUse the given merge strategy; can be supplied more than\n \tonce to specify them in the order they should be tried.\n@@ -14,3 +13,11 @@\n \tis used instead (`git-merge-recursive` when merging a single\n \thead, `git-merge-octopus` otherwise).\n \n+--no-ff, \\--no-fast-forward::\n+\tDon't fast-forward, even when it looks possible.  There will\n+\talways be a commit to do at the end of the merge.\n+\n+--direct::\n+\tDon't do anything clever: go directly to the merge strategy\n+\tprograms.  In particular, this forbids an attempt at in-index\n+\tmerging.\ndiff --git a/git-merge.sh b/git-merge.sh\nindex cc0952a..d6a579f 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -13,6 +13,8 @@ LF='\n all_strategies='recursive octopus resolve stupid ours'\n default_strategies='recursive'\n use_strategies=\n+ff=t\n+index_merge=t\n if test \"@@NO_PYTHON@@\"; then\n \tall_strategies='resolve octopus stupid ours'\n \tdefault_strategies='resolve'\n@@ -65,6 +67,12 @@ do\n \t\tno_summary=t ;;\n \t--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)\n \t\tno_commit=t ;;\n+\t--no-f|--no-ff|--no-fa|--no-fas|--no-fast|--no-fast-|--no-fast-f|\\\n+\t\t--no-fast-fo|--no-fast-for|--no-fast-forw|--no-fast-forwa|\\\n+\t\t--no-fast-forwar|--no-fast-forward)\n+\t\tff=f ;;\n+\t--d|--di|--dir|--dire|--direc|--direct)\n+\t\tff=f index_merge=f ;;\n \t-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\\\n \t\t--strateg=*|--strategy=*|\\\n \t-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)\n@@ -90,6 +98,10 @@ do\n \tshift\n done\n \n+# `ours' is a funny strategy and clever merging optimizations here make\n+# it not work.\n+case \" $use_strategies \" in *\" ours \"*) ff=f index_merge=f ;; esac\n+\n test \"$#\" -le 2 && usage ;# we need at least two heads.\n \n merge_msg=\"$1\"\n@@ -118,18 +130,18 @@ case \"$#\" in\n esac\n echo \"$head\" >\"$GIT_DIR/ORIG_HEAD\"\n \n-case \"$#,$common,$no_commit\" in\n-*,'',*)\n+case \"$#,$ff,$index_merge,$common,$no_commit\" in\n+*,*,*,'',*)\n \t# No common ancestors found. We need a real merge.\n \t;;\n-1,\"$1\",*)\n+1,*,*,\"$1\",*)\n \t# If head can reach all the merge then we are up to date.\n \t# but first the most common case of merging one remote\n \techo \"Already up-to-date.\"\n \tdropsave\n \texit 0\n \t;;\n-1,\"$head\",*)\n+1,t,*,\"$head\",*)\n \t# Again the most common case of merging one remote.\n \techo \"Updating from $head to $1\"\n \tgit-update-index --refresh 2>/dev/null\n@@ -139,11 +151,11 @@ case \"$#,$common,$no_commit\" in\n \tdropsave\n \texit 0\n \t;;\n-1,?*\"$LF\"?*,*)\n+1,*,*,?*\"$LF\"?*,*)\n \t# We are not doing octopus and not fast forward.  Need a\n \t# real merge.\n \t;;\n-1,*,)\n+1,*,t,*,)\n \t# We are not doing octopus, not fast forward, and have only\n \t# one common.  See if it is really trivial.\n \tgit var GIT_COMMITTER_IDENT >/dev/null || exit\n@@ -164,6 +176,10 @@ case \"$#,$common,$no_commit\" in\n \tfi\n \techo \"Nope.\"\n \t;;\n+1,*,*,*,)\n+\t# Only a single remote, but we've been told not to try anything\n+\t# clever.  Skip to real merge.\n+\t;;\n *)\n \t# An octopus.  If we can reach all the remote we are up to date.\n \tup_to_date=t\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 17fda26..229cec7 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -8,7 +8,7 @@ USAGE='[-n | --no-summary] [--no-commit]\n LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.'\n . git-sh-setup\n \n-strategy_args= no_summary= no_commit=\n+strategy_args= no_summary= no_commit= noff= direct=\n while case \"$#,$1\" in 0) break ;; *,-*) ;; *) break ;; esac\n do\n \tcase \"$1\" in\n@@ -17,6 +17,12 @@ do\n \t\tno_summary=-n ;;\n \t--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)\n \t\tno_commit=--no-commit ;;\n+\t--no-f|--no-ff|--no-fa|--no-fas|--no-fast|--no-fast-|--no-fast-f|\\\n+\t\t--no-fast-fo|--no-fast-for|--no-fast-forw|--no-fast-forwa|\\\n+\t\t--no-fast-forwar|--no-fast-forward)\n+\t\tnoff=--no-fast-forward ;;\n+\t--d|--di|--dir|--dire|--direc|--direct)\n+\t\tdirect=--direct ;;\n \t-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\\\n \t\t--strateg=*|--strategy=*|\\\n \t-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)\n@@ -92,4 +98,7 @@ case \"$strategy_args\" in\n esac\n \n merge_name=$(git-fmt-merge-msg <\"$GIT_DIR/FETCH_HEAD\")\n-git-merge $no_summary $no_commit $strategy_args \"$merge_name\" HEAD $merge_head\n+git-merge \\\n+\t$no_summary $no_commit $noff $direct \\\n+\t$strategy_args \\\n+\t\"$merge_name\" HEAD $merge_head\n"},{"id":"17656","messageId":"7vmzfns9c6.fsf@assigned-by-dhcp.cox.net","threadId":"3670","inReplyTo":"20060318101941.8941.52615.stgit@metalzone.distorted.org.uk","subject":"Re: [PATCH] git-merge: New options `--no-fast-forward' and `--direct'.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-18T21:59:21Z","receivedAt":"2006-03-18T21:59:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Wooding <mdw@distorted.org.uk> writes:\n\n> These options disable some of git-merge's optimizations.  \n\nWhile there is no question about the part of the proposed change\nto bypass \"trivial merge\", I do not necessarily agree with\n\"skipping fast forward\" part.\n\nIt is a problem that \"ours\" strategy cannot be used as a way to\nrecover from the \"accidentally rewound head\" situation, because\nit is prevented from running under certain conditions as you\ndescribed.  But that does not necessarily mean we should make\n\"ours\" to work in these situations.\n\nYou accidentally discarded B while somebody picked it up:\n\n                 o---o\n                /     \\\n    ---o---o---A---o---o---B\n               ^your head\n\nNow you would want to recover.  With your patch, it would create\nthis:\n\n                 o---o\n                /     \\\n    ---o---o---A---o---o---B\n                \\           \\\n                 ------------M\n                             ^your updated head,\n                              having the same tree as A\n\nBut recording A as a parent of M is not necessary.  I think what\nwe want to have as the result is this instead:\n\n                 o---o\n                /     \\\n    ---o---o---A---o---o---B---M\n                               ^your updated head,\n                                having the same tree as A\n\nThis is something you cannot do within the current git-merge\nframework; it is set up to either just fast forward or make a\nmulti-parent commit.  You would want have a \"revert to this\nstate\" [*1*], something like this (assuming you have rewound to\nA and currently your index matches A):\n\n        $ git reset --soft B\n        $ git commit -m 'Discard A..B and revert to A'\n\nI did \"ours\" primarily as a demonstration of a funky thing\npeople could do with the consolidated driver \"git-merge\".  I did\nnot have a useful use-case in mind back then, but it turned out\nto be the ideal way to recover from \"accidentally rewound head\"\nsituation, except that making a merge commit between A and B is\nnot always the way to recover from it.  If we wanted to, we\ncould have a special purpose command that does \"git merge -s\nours\" if there will be a new commit, otherwise the above two\ncommands sequence if it will be a fast forward, but the\n\"recovering from accidentally rewound head\" _is_ really a\nspecial purpose, so I do not know if it is worth it.\n\nIn your cover letter, you talked about using --no-fast-forward\nto collapse your sole topic branch into your master branch.  I\ndo not think smudging the development history with extra merge\ncommits for that is justfied either.  There is no reason for you\nto discard your topic branch heads after you merged them into\nmaster.  If they get in the way of your normal workflow, you can\nstash them away as tags that you do not usually see in \"git\nbranch\" output [*2*].\n\nAlso I am already unhappy that git-merge knows about the\nspecifics of strategies [*3*], e.g. it knows octopus is\ncurrently the only strategy that can do more than two heads.\nYour patch gives more strategy specific knowledge to it, but I\ndo not know how to avoid it.\n\n\n[Footnotes]\n\n*1* As opposed to \"git revert X\" which means \"revert the effect\nof commit X\", you would want \"revert to the state X\".\n\n*2* I keep some of my old topic branch heads under\n.git/tags/attic/.\n\n*3* Another thing I am unhappy about is the list of available\nstrategies.  I initially wanted to allow users to write their\nown merge strategies and have them on their PATH (not even\nnecessarily in GIT_EXEC_PATH directory), so that you can do a\ngit-merge-mdw secretly, keep it in ~mdw/bin and cook it for a\nwhile using yourself as a guinea pig, and then share that with\nthe community later, _without_ touching git-merge.\n"},{"id":"17657","messageId":"7vhd5vs6tb.fsf@assigned-by-dhcp.cox.net","threadId":"3670","inReplyTo":"7vmzfns9c6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-merge: New options `--no-fast-forward' and `--direct'.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-03-18T22:53:52Z","receivedAt":"2006-03-18T22:53:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">From nobody Mon Sep 17 00:00:00 2001\nFrom: Junio C Hamano <junkio@cox.net>\nDate: Sat Mar 18 14:50:53 2006 -0800\nSubject: [PATCH] git-merge knows some strategies want to skip trivial merges\n\nMost notably \"ours\".  Also this makes sure we do not record\nduplicated parents on the parent list of the resulting commit.\n\nThis is based on Mark Wooding's work, but does not change the UI\nnor introduce new flags.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n * How about this instead?  It looks larger than it really is\n   because strategy-defaulting code needed to get moved around.\n\n git-merge.sh |   67 +++++++++++++++++++++++++++++++++++-----------------------\n 1 files changed, 40 insertions(+), 27 deletions(-)\n\n313093ea6d29bbce5977556645eb5946dbfb211e\ndiff --git a/git-merge.sh b/git-merge.sh\nindex cc0952a..78ab422 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -11,11 +11,15 @@ LF='\n '\n \n all_strategies='recursive octopus resolve stupid ours'\n-default_strategies='recursive'\n+default_twohead_strategies='recursive'\n+default_octopus_strategies='octopus'\n+no_trivial_merge_strategies='ours'\n use_strategies=\n+\n+index_merge=t\n if test \"@@NO_PYTHON@@\"; then\n \tall_strategies='resolve octopus stupid ours'\n-\tdefault_strategies='resolve'\n+\tdefault_twohead_strategies='resolve'\n fi\n \n dropsave() {\n@@ -90,8 +94,6 @@ do\n \tshift\n done\n \n-test \"$#\" -le 2 && usage ;# we need at least two heads.\n-\n merge_msg=\"$1\"\n shift\n head_arg=\"$1\"\n@@ -99,6 +101,8 @@ head=$(git-rev-parse --verify \"$1\"^0) ||\n shift\n \n # All the rest are remote heads\n+test \"$#\" = 0 && usage ;# we need at least one remote head.\n+\n remoteheads=\n for remote\n do\n@@ -108,6 +112,27 @@ do\n done\n set x $remoteheads ; shift\n \n+case \"$use_strategies\" in\n+'')\n+\tcase \"$#\" in\n+\t1)\n+\t\tuse_strategies=\"$default_twohead_strategies\" ;;\n+\t*)\n+\t\tuse_strategies=\"$default_octopus_strategies\" ;;\n+\tesac\n+\t;;\n+esac\n+\n+for s in $use_strategies\n+do\n+\tcase \" $s \" in\n+\t*\" $no_trivial_merge_strategies \"*)\n+\t\tindex_merge=f\n+\t\tbreak\n+\t\t;;\n+\tesac\n+done\n+\n case \"$#\" in\n 1)\n \tcommon=$(git-merge-base --all $head \"$@\")\n@@ -118,18 +143,21 @@ case \"$#\" in\n esac\n echo \"$head\" >\"$GIT_DIR/ORIG_HEAD\"\n \n-case \"$#,$common,$no_commit\" in\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-1,\"$1\",*)\n+?,1,\"$1\",*)\n \t# If head can reach all the merge then we are up to date.\n-\t# but first the most common case of merging one remote\n+\t# but first the most common case of merging one remote.\n \techo \"Already up-to-date.\"\n \tdropsave\n \texit 0\n \t;;\n-1,\"$head\",*)\n+?,1,\"$head\",*)\n \t# Again the most common case of merging one remote.\n \techo \"Updating from $head to $1\"\n \tgit-update-index --refresh 2>/dev/null\n@@ -139,11 +167,11 @@ case \"$#,$common,$no_commit\" in\n \tdropsave\n \texit 0\n \t;;\n-1,?*\"$LF\"?*,*)\n+?,1,?*\"$LF\"?*,*)\n \t# We are not doing octopus and not fast forward.  Need a\n \t# real merge.\n \t;;\n-1,*,)\n+?,1,*,)\n \t# We are not doing octopus, not fast forward, and have only\n \t# one common.  See if it is really trivial.\n \tgit var GIT_COMMITTER_IDENT >/dev/null || exit\n@@ -188,17 +216,6 @@ esac\n # We are going to make a new commit.\n git var GIT_COMMITTER_IDENT >/dev/null || exit\n \n-case \"$use_strategies\" in\n-'')\n-\tcase \"$#\" in\n-\t1)\n-\t\tuse_strategies=\"$default_strategies\" ;;\n-\t*)\n-\t\tuse_strategies=octopus ;;\n-\tesac\t\t\n-\t;;\n-esac\n-\n # At this point, we need a real merge.  No matter what strategy\n # we use, it would operate on the index, possibly affecting the\n # working tree, and when resolved cleanly, have the desired tree\n@@ -270,11 +287,7 @@ done\n # auto resolved the merge cleanly.\n if test '' != \"$result_tree\"\n then\n-    parents=\"-p $head\"\n-    for remote\n-    do\n-        parents=\"$parents -p $remote\"\n-    done\n+    parents=$(git-show-branch --independent \"$head\" \"$@\" | sed -e 's/^/-p /')\n     result_commit=$(echo \"$merge_msg\" | git-commit-tree $result_tree $parents) || exit\n     finish \"$result_commit\" \"Merge $result_commit, made by $wt_strategy.\"\n     dropsave\n-- \n1.2.4.g2fc2\n"}]}