{"thread":{"id":"44368","subject":"[PATCH] reset: --unmerge","startedAt":"2016-10-24T23:10:21Z","lastAt":"2016-10-27T16:23:13Z","messageCount":9,"participants":["Junio C Hamano","Duy Nguyen","Sergey Organov","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"304813","messageId":"xmqqa8dttkbw.fsf@gitster.mtv.corp.google.com","threadId":"44368","inReplyTo":null,"subject":"[PATCH] reset: --unmerge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-24T23:10:11Z","receivedAt":"2016-10-24T23:10:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The procedure to resolve a merge conflict typically goes like this:\n\n - first open the file in the editor, and with the help of conflict\n markers come up with a resolution.\n\n - save the file.\n\n - look at the output from \"git diff\" to see the combined diff to\n double check if the resolution makes sense.\n\n - perform other tests, like trying to build the result with \"make\".\n\n - finally \"git add file\" to mark that you are done.\n\nand repeating the above until you are done with all the conflicted\npaths. If you, for whatever reason, accidentally \"git add file\" by\nmistake until you are convinced that you resolved it correctly (e.g.\ndoing \"git add file\" immediately after saving, without a chance to\nperuse the output from \"git diff\"), there is no good way to recover.\nThere is \"git checkout -m file\" but that overwrites the working tree\nfile to reproduce the conflicted state, which is not exactly what\nyou want. You only want to reproduce the conflicted state in the\nindex, so that you can inspect the (proposed) merge resolution you\nalready have in your working tree.\n\nAdd \"git reset --unmerge \" command that does just that.\n\nSigned-off-by: Junio C Hamano \n---\n\n * Not thought through thoroughly yet about details, such as \"should\n it always require at least one pathspec?\".\n\n builtin/reset.c | 8 +++++++-\n t/t7107-reset-unmerged.sh | 36 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9020ec66c8..3aa9e0b34a 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,6 +21,7 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"resolve-undo.h\"\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n@@ -272,6 +273,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n \tint intent_to_add = 0;\n+\tint unmerge = 0;\n \tconst struct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n@@ -286,6 +288,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL('N', \"intent-to-add\", &intent_to_add,\n \t\t\t\tN_(\"record only the fact that removed paths will be added later\")),\n+\t\tOPT_BOOL(0, \"unmerge\", &unmerge,\n+\t\t\t N_(\"recover conflicted stages from an earlier 'git add'\")),\n \t\tOPT_END()\n \t};\n \n@@ -357,7 +361,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\thold_locked_index(lock, 1);\n \t\tif (reset_type == MIXED) {\n \t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n-\t\t\tif (read_from_tree(&pathspec, oid.hash, intent_to_add))\n+\t\t\tif (unmerge)\n+\t\t\t\tunmerge_cache(&pathspec);\n+\t\t\telse if (read_from_tree(&pathspec, oid.hash, intent_to_add))\n \t\t\t\treturn 1;\n \t\t\tif (get_git_work_tree())\n \t\t\t\trefresh_index(&the_index, flags, NULL, NULL,\ndiff --git a/t/t7107-reset-unmerged.sh b/t/t7107-reset-unmerged.sh\nnew file mode 100755\nindex 0000000000..57b2e27150\n--- /dev/null\n+++ b/t/t7107-reset-unmerged.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+test_description='git reset with paths'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo one >file &&\n+\tgit add file &&\n+\tgit commit -m \"one\" &&\n+\tgit tag initial &&\n+\n+\techo two >file &&\n+\tgit commit -a -m \"two\" &&\n+\n+\tgit checkout -b side initial &&\n+\techo three >file &&\n+\tgit commit -a -m \"three\"\n+'\n+\n+test_expect_success \"cause conflict, resolve, and unresolve\" '\n+\tgit reset --hard &&\n+\tgit checkout master &&\n+\ttest_must_fail git merge side &&\n+\n+\tgit ls-files -u >expect &&\n+\n+\techo four >file &&\n+\tgit add file &&\n+\n+\tgit reset --unmerge -- file &&\n+\tgit ls-files -u >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n"},{"id":"304833","messageId":"CACsJy8B-GcMNv7pYYLpaUXc2kKnvyYEYm6w=fiaHy7rt4aug1Q@mail.gmail.com","threadId":"44368","inReplyTo":"xmqqa8dttkbw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-25T11:06:47Z","receivedAt":"2016-10-25T11:07:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 25, 2016 at 6:10 AM, Junio C Hamano wrote:\n> The procedure to resolve a merge conflict typically goes like this:\n>\n> - first open the file in the editor, and with the help of conflict\n> markers come up with a resolution.\n>\n> - save the file.\n>\n> - look at the output from \"git diff\" to see the combined diff to\n> double check if the resolution makes sense.\n>\n> - perform other tests, like trying to build the result with \"make\".\n>\n> - finally \"git add file\" to mark that you are done.\n>\n> and repeating the above until you are done with all the conflicted\n> paths. If you, for whatever reason, accidentally \"git add file\" by\n> mistake until you are convinced that you resolved it correctly (e.g.\n> doing \"git add file\" immediately after saving, without a chance to\n> peruse the output from \"git diff\"), there is no good way to recover.\n\nI made this exact mistake on a giant, half-resolved merge conflict the\nother day and panicked. While I prepared myself to script update-index\nto restore stages 2 and 3, I found \"update-index --unresolve\". It\nsounds like this \"reset --unmerge\" is the same functionality, right?\nI'm not objecting this patch though, update-index is not something a\ncasual user should use.\n\n> There is \"git checkout -m file\" but that overwrites the working tree\n> file to reproduce the conflicted state, which is not exactly what\n> you want. You only want to reproduce the conflicted state in the\n> index, so that you can inspect the (proposed) merge resolution you\n> already have in your working tree.\n-- \nDuy\n"},{"id":"304834","messageId":"CACsJy8D4cT2EV_t6=+XTHxLmjV+NtAE+KgiN3ZiP7JQzvu29oQ@mail.gmail.com","threadId":"44368","inReplyTo":"CACsJy8B-GcMNv7pYYLpaUXc2kKnvyYEYm6w=fiaHy7rt4aug1Q@mail.gmail.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-25T11:11:41Z","receivedAt":"2016-10-25T11:12:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 25, 2016 at 6:06 PM, Duy Nguyen wrote:\n> On Tue, Oct 25, 2016 at 6:10 AM, Junio C Hamano wrote:\n>> The procedure to resolve a merge conflict typically goes like this:\n>>\n>> - first open the file in the editor, and with the help of conflict\n>> markers come up with a resolution.\n>>\n>> - save the file.\n>>\n>> - look at the output from \"git diff\" to see the combined diff to\n>> double check if the resolution makes sense.\n>>\n>> - perform other tests, like trying to build the result with \"make\".\n>>\n>> - finally \"git add file\" to mark that you are done.\n\nBTW making git-add (and \"git commit -a\") refuse files with conflict\nmarkers present could prevent this mistake earlier and is probably a\nbetter option because the user won't have to discover 'reset\n--unmerge'. If the user likes to add the file with conflict markers\nanyway (because those look like conflict markers but are in fact not)\nthen they can go with \"git add -f\" or similar.\n-- \nDuy\n"},{"id":"304854","messageId":"87bmy81gi0.fsf@javad.com","threadId":"44368","inReplyTo":"xmqqa8dttkbw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2016-10-25T17:32:39Z","receivedAt":"2016-10-25T17:32:47Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano writes:\n\n> The procedure to resolve a merge conflict typically goes like this:\n>\n> - first open the file in the editor, and with the help of conflict\n> markers come up with a resolution.\n>\n> - save the file.\n>\n> - look at the output from \"git diff\" to see the combined diff to\n> double check if the resolution makes sense.\n>\n> - perform other tests, like trying to build the result with \"make\".\n>\n> - finally \"git add file\" to mark that you are done.\n>\n> and repeating the above until you are done with all the conflicted\n> paths. If you, for whatever reason, accidentally \"git add file\" by\n> mistake until you are convinced that you resolved it correctly (e.g.\n> doing \"git add file\" immediately after saving, without a chance to\n> peruse the output from \"git diff\"), there is no good way to recover.\n\n\"git reset --unmerge file\"\n\nto undo accidental\n\n\"git add file\"\n\nduring conflict resolution?\n\nI'm afraid \"unmerge\" sounds like revert of \"merge\", rather than revert\nof \"resolve\". I'd rather prefer to see something like:\n\ngit add --undo file\n\ngit merge --unresolve file\n\ngit reset --unresolve file\n\nin that order, to deal with the issue.\n\n-- Sergey\n"},{"id":"304875","messageId":"xmqqmvhsc8kn.fsf@gitster.mtv.corp.google.com","threadId":"44368","inReplyTo":"CACsJy8D4cT2EV_t6=+XTHxLmjV+NtAE+KgiN3ZiP7JQzvu29oQ@mail.gmail.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-25T23:28:24Z","receivedAt":"2016-10-25T23:28:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> BTW making git-add (and \"git commit -a\") refuse files with conflict\n> markers present could prevent this mistake earlier and is probably a\n> better option because the user won't have to discover 'reset\n> --unmerge'.\n\nThat may help some users, but you are solving a different problem.\n\nI do not say \"save\" unless I know the editor buffer contents is not\nready. \"This is ready to be saved\" however is different from \"This\nresolution is correct\", and I need the unmerged states in the index\nto verify, namely by looking at \"git diff\" (no other parameters)\noutput that shows only the paths with unmerged stages and in the\ncompact combined diff format.\n\nSomebody with a bright idea decided that vc-git-resolve-conflicts\nvariable should be on by default in Emacs 25.1 X-<, which causes\n\"save\" after resolving conflicts to automatically run \"git add\", to\ndestroy this valuable tool. My knee-jerk reaction, of course, to\nsuch a default is \"that's brain dead\", but perhaps they did so for\nsome good reason that I fail to fathom.\n\n"},{"id":"304883","messageId":"CACsJy8Dn7m2axEFkkQtnZMs2yzFwivAJyZCWxODg-HQ=qLnVMA@mail.gmail.com","threadId":"44368","inReplyTo":"xmqqmvhsc8kn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-10-26T09:32:14Z","receivedAt":"2016-10-26T09:32:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Oct 26, 2016 at 6:28 AM, Junio C Hamano wrote:\n> Somebody with a bright idea decided that vc-git-resolve-conflicts\n> variable should be on by default in Emacs 25.1 X-<,\n\nOh good, I have an excuse to stick to 24.5.1 for a while longer then.\n\n> which causes\n> \"save\" after resolving conflicts to automatically run \"git add\", to\n> destroy this valuable tool. My knee-jerk reaction, of course, to\n> such a default is \"that's brain dead\", but perhaps they did so for\n> some good reason that I fail to fathom.\n\nI was curious. The default is t since the variable's introduction [1].\nInterestingly the thread/bug that resulted in that commit started with\n\"report this bug to git\" [2]. Something about git-stash. I quote the\noriginal mail here in case anyone wants to look into it (not sure if\nit's actually reported here before, I don't pay much attention to\ngit-stash mails)\n\n-- 8< --\nBad news, everyone!\n\nWhen a stash contains changes for several files, and \"stash pop\"\nencounters conflicts only in some of them, the rest of the files are\nstages automatically.\n\nAt least, that happens with Git 2.1.0 on my machine, and some\ncommenters here: http://stackoverflow.com/a/1237337/615245\n\nSo then when we unstage the files which had conflicts after resolving\nthose, the result is mixed. Which doesn't look right.\n\nWhat shall we do? Unstage the automatically-staged files? Revert the\nchanges from this bug? It seems Git really wants the changes staged\nafter the conflict resolution.\n-- 8< --\n\n[1] http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=45651154473c7d2f16230da765d034ecfde7968a\n[2] https://lists.gnu.org/archive/html/bug-gnu-emacs/2015-05/msg00433.html\n-- \nDuy\n"},{"id":"304901","messageId":"xmqq1sz3ca4u.fsf@gitster.mtv.corp.google.com","threadId":"44368","inReplyTo":"CACsJy8Dn7m2axEFkkQtnZMs2yzFwivAJyZCWxODg-HQ=qLnVMA@mail.gmail.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-26T17:06:57Z","receivedAt":"2016-10-26T17:07:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> Interestingly the thread/bug that resulted in that commit started with\n> \"report this bug to git\" [2]. Something about git-stash. I quote the\n> original mail here in case anyone wants to look into it (not sure if\n> it's actually reported here before, I don't pay much attention to\n> git-stash mails)\n>\n> -- 8< --\n> Bad news, everyone!\n>\n> When a stash contains changes for several files, and \"stash pop\"\n> encounters conflicts only in some of them, the rest of the files are\n> stages automatically.\n\nIt indeed is curious.\n\nThat is the designed behaviour for _ANY_ mergy operation, and not\nlimited to \"stash pop\".\n\nA clean application is added to the index so that you can find out\nabout them from \"diff --cached\", while conflicted ones keep their\nunmerged stages so that the conflict can be resolved in the working\ntree files. There is no bad news here.\n\nOnce you resolve the conflict, you would add the final contents to\nthe working tree, but as anybody who knows how \"git diff\" after\nresolving conflicts in the working tree files is useful would know,\n\"saving the editor buffer after removing conflict markers\" is not a\nvalid signal that the user is confident that the contents is final.\n\n> At least, that happens with Git 2.1.0 on my machine, and some\n> commenters here: http://stackoverflow.com/a/1237337/615245\n>\n> So then when we unstage the files which had conflicts after resolving\n> those, the result is mixed. Which doesn't look right.\n\nWhoever wrote this does not understand how mergy operations in Git\nworks, I guess.\n\n> What shall we do? Unstage the automatically-staged files? Revert the\n> changes from this bug? It seems Git really wants the changes staged\n> after the conflict resolution.\n\nThe first order of business is to learn how mergy operations in Git\nis designed to work, and if they are in the business of building a\ntool around Git to make the life of users better, avoid going against\nthe designed workflow.\n\nIf this \"Bad news, everyone!\" is why vc-git-resolve-conflicts was\nadded and defaults to true, I can feel safe in toggling it off\nforever in my ~/.emacs, knowing that it is a totally broken option\nthat came from a desire to fix a problem that does not exist.\n\n"},{"id":"304966","messageId":"87d1il3hhs.fsf@linux-m68k.org","threadId":"44368","inReplyTo":"xmqqmvhsc8kn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] reset: --unmerge","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2016-10-27T16:05:03Z","receivedAt":"2016-10-27T16:05:13Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Okt 25 2016, Junio C Hamano wrote:\n\n> Somebody with a bright idea decided that vc-git-resolve-conflicts\n> variable should be on by default in Emacs 25.1 X-<\n\nThis is consistent with the behaviour of the other VC backends, where it\nisn't even customizable.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"304968","messageId":"xmqqzilp7od3.fsf@gitster.mtv.corp.google.com","threadId":"44368","inReplyTo":"87d1il3hhs.fsf@linux-m68k.org","subject":"Re: [PATCH] reset: --unmerge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-27T16:23:04Z","receivedAt":"2016-10-27T16:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab writes:\n\n> On Okt 25 2016, Junio C Hamano wrote:\n>\n>> Somebody with a bright idea decided that vc-git-resolve-conflicts\n>> variable should be on by default in Emacs 25.1 X-<\n>\n> This is consistent with the behaviour of the other VC backends, where it\n> isn't even customizable.\n\nThe problem I had was \"M-x save-buffer\" (after resolving the\nconflicts in it manually) running \"git add\" on the resulting file,\nrobbing from \"git diff\" an opportunity to help the user to see how\nthe result looks relative to both branches.\n\nDo you mean that VC mode broke the same feature equally other SCMs,\ntoo? Do other SCM supported by VC backends take advantage of\nunmerged stages in the index (until you say \"$scm add\") by allowing\nyou to verify the combined diff with \"$scm diff\"?\n\n\n\n"}]}