{"thread":{"id":"20960","subject":"[PATCH v2 0/4] \"git reset --merge\" related improvements","startedAt":"2009-09-16T04:14:38Z","lastAt":"2009-09-17T05:23:12Z","messageCount":8,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"123297","messageId":"20090916035131.3737.33020.chriscool@tuxfamily.org","threadId":"20960","inReplyTo":null,"subject":"[PATCH v2 0/4] \"git reset --merge\" related improvements","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-09-16T04:14:38Z","receivedAt":"2009-09-16T04:14:38Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In this new version, the test cases that were bogus are now fixed. And\nthe new \"git reset\" option is now called \"--merge-safe\" (instead of\n\"--merge-dirty\").\n\nTo make the test cases pass I had to change a little bit the way\n\"--merge-safe\" is implemented. It now does not set the \"reset\" flag in the\n\"struct unpack_trees_options\" passed to \"unpack_trees()\". \n\nAnd commit messages have been improved, thanks to input from Daniel, Junio\nand Linus. And there is no more \"exec </dev/null\" in the test script, thanks\nto Jakub.\n\nI prefer to keep Stephan as the author of patch 3/4 because he designed and\nimplemented the new feature in the first place.\n\nI am working on the documentation for \"--merge-safe\" and on improving the\nexisting \"git reset\" documentation using a table at the same time. So\nanother patch will be added to this series later.\n\nChristian Couder (2):\n  reset: add a few tests for \"git reset --merge\"\n  reset: add test cases for \"--merge-safe\" option\n\nStephan Beyer (2):\n  reset: use \"unpack_trees()\" directly instead of \"git read-tree\"\n  reset: add option \"--merge-safe\" to \"git reset\"\n\n builtin-reset.c        |   81 ++++++++++++++++++++++++++++-------\n t/t7110-reset-merge.sh |  112 ++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 177 insertions(+), 16 deletions(-)\n create mode 100755 t/t7110-reset-merge.sh\n"},{"id":"123299","messageId":"20090916041443.3737.86878.chriscool@tuxfamily.org","threadId":"20960","inReplyTo":"20090916035131.3737.33020.chriscool@tuxfamily.org","subject":"[PATCH v2 1/4] reset: add a few tests for \"git reset --merge\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-09-16T04:14:39Z","receivedAt":"2009-09-16T04:14:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Commit 9e8eceab (\"Add 'merge' mode to 'git reset'\", 2008-12-01),\nadded the --merge option to git reset, but there were no test cases\nfor it.\n\nThis was not a big problem because \"git reset\" was just forking and\nexecing \"git read-tree\", but this will change in a following patch.\n\nSo let's add a few test cases to make sure that there will be no\nregression.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t7110-reset-merge.sh |   68 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 68 insertions(+), 0 deletions(-)\n create mode 100755 t/t7110-reset-merge.sh\n\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nnew file mode 100755\nindex 0000000..1ec32f9\n--- /dev/null\n+++ b/t/t7110-reset-merge.sh\n@@ -0,0 +1,68 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2009 Christian Couder\n+#\n+\n+test_description='Tests for \"git reset --merge\"'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'creating initial files' '\n+     echo \"line 1\" >> file1 &&\n+     echo \"line 2\" >> file1 &&\n+     echo \"line 3\" >> file1 &&\n+     cp file1 file2 &&\n+     git add file1 file2 &&\n+     test_tick &&\n+     git commit -m \"Initial commit\"\n+'\n+\n+test_expect_success 'reset --merge is ok with changes in file it does not touch' '\n+     echo \"line 4\" >> file1 &&\n+     echo \"line 4\" >> file2 &&\n+     test_tick &&\n+     git commit -m \"add line 4\" file1 &&\n+     git reset --merge HEAD^ &&\n+     ! grep 4 file1 &&\n+     grep 4 file2 &&\n+     git reset --merge HEAD@{1} &&\n+     grep 4 file1 &&\n+     grep 4 file2\n+'\n+\n+test_expect_success 'reset --merge discards changes added to index (1)' '\n+     echo \"line 5\" >> file1 &&\n+     git add file1 &&\n+     git reset --merge HEAD^ &&\n+     ! grep 4 file1 &&\n+     ! grep 5 file1 &&\n+     grep 4 file2 &&\n+     echo \"line 5\" >> file2 &&\n+     git add file2 &&\n+     git reset --merge HEAD@{1} &&\n+     ! grep 4 file2 &&\n+     ! grep 5 file1 &&\n+     grep 4 file1\n+'\n+\n+test_expect_success 'reset --merge discards changes added to index (2)' '\n+     echo \"line 4\" >> file2 &&\n+     git add file2 &&\n+     git reset --merge HEAD^ &&\n+     ! grep 4 file2 &&\n+     git reset --merge HEAD@{1} &&\n+     ! grep 4 file2 &&\n+     grep 4 file1\n+'\n+\n+test_expect_success 'reset --merge fails with changes in file it touches' '\n+     echo \"line 6\" >> file1 &&\n+     test_tick &&\n+     git commit -m \"add line 6\" file1 &&\n+     sed -e \"s/line 1/changed line 1/\" <file1 >file3 &&\n+     mv file3 file1 &&\n+     test_must_fail git reset --merge HEAD^ 2>err.log &&\n+     grep file1 err.log | grep \"not uptodate\"\n+'\n+\n+test_done\n-- \n1.6.5.rc0.150.g38fe6\n"},{"id":"123300","messageId":"20090916041443.3737.91512.chriscool@tuxfamily.org","threadId":"20960","inReplyTo":"20090916035131.3737.33020.chriscool@tuxfamily.org","subject":"[PATCH v2 2/4] reset: use \"unpack_trees()\" directly instead of \"git read-tree\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-09-16T04:14:40Z","receivedAt":"2009-09-16T04:14:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Stephan Beyer <s-beyer@gmx.net>\n\nThis patch makes \"reset_index_file()\" call \"unpack_trees()\" directly\ninstead of forking and execing \"git read-tree\". So the code is more\nefficient.\n\nAnd it's also easier to see which unpack_tree() options will be used,\nas we don't need to follow \"git read-tree\"'s command line parsing\nwhich is quite complex.\n\nAs Daniel Barkalow found, there is a difference between this new\nversion and the old one. The old version gives an error for\n\"git reset --merge\" with unmerged entries and the new version does\nnot. But this can be seen as a bug fix, because \"--merge\" was the\nonly \"git reset\" option with this behavior and this behavior was\nnot documented.\n\nThe code comes from the sequencer GSoC project:\n\ngit://repo.or.cz/git/sbeyer.git\n\n(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)\n\nMentored-by: Daniel Barkalow <barkalow@iabervon.org>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-reset.c |   51 ++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 40 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 73e6022..ddb81f3 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -18,6 +18,8 @@\n #include \"tree.h\"\n #include \"branch.h\"\n #include \"parse-options.h\"\n+#include \"unpack-trees.h\"\n+#include \"cache-tree.h\"\n \n static const char * const git_reset_usage[] = {\n \t\"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]\",\n@@ -52,29 +54,56 @@ static inline int is_merge(void)\n \treturn !access(git_path(\"MERGE_HEAD\"), F_OK);\n }\n \n+static int parse_and_init_tree_desc(const unsigned char *sha1,\n+\t\t\t\t\t     struct tree_desc *desc)\n+{\n+\tstruct tree *tree = parse_tree_indirect(sha1);\n+\tif (!tree)\n+\t\treturn 1;\n+\tinit_tree_desc(desc, tree->buffer, tree->size);\n+\treturn 0;\n+}\n+\n static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)\n {\n-\tint i = 0;\n-\tconst char *args[6];\n+\tint nr = 1;\n+\tint newfd;\n+\tstruct tree_desc desc[2];\n+\tstruct unpack_trees_options opts;\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \n-\targs[i++] = \"read-tree\";\n+\tmemset(&opts, 0, sizeof(opts));\n+\topts.head_idx = 1;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n+\topts.fn = oneway_merge;\n+\topts.merge = 1;\n \tif (!quiet)\n-\t\targs[i++] = \"-v\";\n+\t\topts.verbose_update = 1;\n \tswitch (reset_type) {\n \tcase MERGE:\n-\t\targs[i++] = \"-u\";\n-\t\targs[i++] = \"-m\";\n+\t\topts.update = 1;\n \t\tbreak;\n \tcase HARD:\n-\t\targs[i++] = \"-u\";\n+\t\topts.update = 1;\n \t\t/* fallthrough */\n \tdefault:\n-\t\targs[i++] = \"--reset\";\n+\t\topts.reset = 1;\n \t}\n-\targs[i++] = sha1_to_hex(sha1);\n-\targs[i] = NULL;\n \n-\treturn run_command_v_opt(args, RUN_GIT_CMD);\n+\tnewfd = hold_locked_index(lock, 1);\n+\n+\tread_cache_unmerged();\n+\n+\tif (parse_and_init_tree_desc(sha1, desc + nr - 1))\n+\t\treturn error(\"Failed to find tree of %s.\", sha1_to_hex(sha1));\n+\tif (unpack_trees(nr, desc, &opts))\n+\t\treturn -1;\n+\tif (write_cache(newfd, active_cache, active_nr) ||\n+\t    commit_locked_index(lock))\n+\t\treturn error(\"Could not write new index file.\");\n+\n+\treturn 0;\n }\n \n static void print_new_head_line(struct commit *commit)\n-- \n1.6.5.rc0.150.g38fe6\n"},{"id":"123301","messageId":"20090916041443.3737.63217.chriscool@tuxfamily.org","threadId":"20960","inReplyTo":"20090916035131.3737.33020.chriscool@tuxfamily.org","subject":"[PATCH v2 3/4] reset: add option \"--merge-safe\" to \"git reset\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-09-16T04:14:41Z","receivedAt":"2009-09-16T04:14:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Stephan Beyer <s-beyer@gmx.net>\n\nThis option is nearly like \"--merge\" except that it is\nsafer. The table below show the differences between these\noptions.\n\nworking index HEAD target         working index HEAD\n  B      B     A     A   --m-s      B      A     A\n                         --merge    A      A     A\n  B      B     A     C   --m-s       (disallowed)\n                         --merge    C      C     C\n\nIn this table, A, B and C are some different states of\na file. For example the first 2 lines of the table mean\nthat if a file is in state B in the working tree and\nthe index, and in a different state A in HEAD and in\nthe target, then \"git reset --merge-safe target\" will\nput the file in state B in the working tree and in\nstate A in the index and HEAD.\n\nSo as can be seen in the table, \"--merge\" discards changes\nin the index, while \"--merge-safe\" does not.\n\nA following patch will add some test cases for\n\"--merge-safe\", where the differences between \"--merge\"\nand \"--merge-safe\" can also be seen.\n\nThe \"--merge-safe\" option is implemented by doing a 2 way\nmerge between HEAD and the reset target, and if this\nsucceeds by doing a mixed reset to the target.\n\nThe code comes from the sequencer GSoC project:\n\ngit://repo.or.cz/git/sbeyer.git\n\n(at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)\n\nBut in the sequencer project the \"reset\" flag was set\nin the \"struct unpack_trees_options\" passed to\n\"unpack_trees()\". With this flag the changes in the\nworking tree were discarded if the file was different\nbetween HEAD and the reset target.\n\nMentored-by: Daniel Barkalow <barkalow@iabervon.org>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-reset.c |   30 +++++++++++++++++++++++++-----\n 1 files changed, 25 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex ddb81f3..78d42e6 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -22,13 +22,15 @@\n #include \"cache-tree.h\"\n \n static const char * const git_reset_usage[] = {\n-\t\"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]\",\n+\t\"git reset [--mixed | --soft | --hard | --merge | --merge-safe] [-q] [<commit>]\",\n \t\"git reset [--mixed] <commit> [--] <paths>...\",\n \tNULL\n };\n \n-enum reset_type { MIXED, SOFT, HARD, MERGE, NONE };\n-static const char *reset_type_names[] = { \"mixed\", \"soft\", \"hard\", \"merge\", NULL };\n+enum reset_type { MIXED, SOFT, HARD, MERGE, MERGE_SAFE, NONE };\n+static const char *reset_type_names[] = {\n+\t\"mixed\", \"soft\", \"hard\", \"merge\", \"merge_safe\", NULL\n+};\n \n static char *args_to_str(const char **argv)\n {\n@@ -81,6 +83,7 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \tif (!quiet)\n \t\topts.verbose_update = 1;\n \tswitch (reset_type) {\n+\tcase MERGE_SAFE:\n \tcase MERGE:\n \t\topts.update = 1;\n \t\tbreak;\n@@ -95,6 +98,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \n \tread_cache_unmerged();\n \n+\tif (reset_type == MERGE_SAFE) {\n+\t\tunsigned char *head_sha1;\n+\t\tif (get_sha1(\"HEAD\", head_sha1))\n+\t\t\treturn error(\"You do not have a valid HEAD.\");\n+\t\tif (parse_and_init_tree_desc(head_sha1, desc))\n+\t\t\treturn error(\"Failed to find tree of HEAD.\");\n+\t\tnr++;\n+\t\topts.fn = twoway_merge;\n+\t}\n+\n \tif (parse_and_init_tree_desc(sha1, desc + nr - 1))\n \t\treturn error(\"Failed to find tree of %s.\", sha1_to_hex(sha1));\n \tif (unpack_trees(nr, desc, &opts))\n@@ -238,6 +251,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\"reset HEAD, index and working tree\", HARD),\n \t\tOPT_SET_INT(0, \"merge\", &reset_type,\n \t\t\t\t\"reset HEAD, index and working tree\", MERGE),\n+\t\tOPT_SET_INT(0, \"merge-safe\", &reset_type,\n+\t\t\t\t\"reset HEAD, index and working tree\",\n+\t\t\t\tMERGE_SAFE),\n \t\tOPT_BOOLEAN('q', NULL, &quiet,\n \t\t\t\t\"disable showing new HEAD in hard reset and progress message\"),\n \t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, \"select hunks interactively\"),\n@@ -324,9 +340,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type == SOFT) {\n \t\tif (is_merge() || read_cache() < 0 || unmerged_cache())\n \t\t\tdie(\"Cannot do a soft reset in the middle of a merge.\");\n+\t} else {\n+\t\tint err = reset_index_file(sha1, reset_type, quiet);\n+\t\tif (reset_type == MERGE_SAFE)\n+\t\t\terr = err || reset_index_file(sha1, MIXED, quiet);\n+\t\tif (err)\n+\t\t\tdie(\"Could not reset index file to revision '%s'.\", rev);\n \t}\n-\telse if (reset_index_file(sha1, reset_type, quiet))\n-\t\tdie(\"Could not reset index file to revision '%s'.\", rev);\n \n \t/* Any resets update HEAD to the head being switched to,\n \t * saving the previous head in ORIG_HEAD before. */\n-- \n1.6.5.rc0.150.g38fe6\n"},{"id":"123298","messageId":"20090916041443.3737.58488.chriscool@tuxfamily.org","threadId":"20960","inReplyTo":"20090916035131.3737.33020.chriscool@tuxfamily.org","subject":"[PATCH v2 4/4] reset: add test cases for \"--merge-safe\" option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-09-16T04:14:42Z","receivedAt":"2009-09-16T04:14:42Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This shows that with the \"--merge-safe\" option, changes that are\nboth in the work tree and the index are kept in the work tree after\nthe reset (but discarded in the index). As with the \"--merge\" option,\nchanges that are in both the work tree and the index are discarded\nafter the reset.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t7110-reset-merge.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++--\n 1 files changed, 47 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nindex 1ec32f9..5b6db41 100755\n--- a/t/t7110-reset-merge.sh\n+++ b/t/t7110-reset-merge.sh\n@@ -3,7 +3,7 @@\n # Copyright (c) 2009 Christian Couder\n #\n \n-test_description='Tests for \"git reset --merge\"'\n+test_description='Tests for \"git reset\" with --merge and --merge-safe'\n \n . ./test-lib.sh\n \n@@ -30,6 +30,20 @@ test_expect_success 'reset --merge is ok with changes in file it does not touch'\n      grep 4 file2\n '\n \n+test_expect_success 'reset --merge-safe is ok with changes in file it does not touch' '\n+     git reset --hard HEAD^ &&\n+     echo \"line 4\" >> file1 &&\n+     echo \"line 4\" >> file2 &&\n+     test_tick &&\n+     git commit -m \"add line 4\" file1 &&\n+     git reset --merge-safe HEAD^ &&\n+     ! grep 4 file1 &&\n+     grep 4 file2 &&\n+     git reset --merge-safe HEAD@{1} &&\n+     grep 4 file1 &&\n+     grep 4 file2\n+'\n+\n test_expect_success 'reset --merge discards changes added to index (1)' '\n      echo \"line 5\" >> file1 &&\n      git add file1 &&\n@@ -55,13 +69,43 @@ test_expect_success 'reset --merge discards changes added to index (2)' '\n      grep 4 file1\n '\n \n+test_expect_success 'reset --merge-safe fails with changes in index in files it touches' '\n+     echo \"line 4\" >> file2 &&\n+     echo \"line 5\" >> file1 &&\n+     git add file1 &&\n+     test_must_fail git reset --merge-safe HEAD^ &&\n+     git reset --hard HEAD\n+'\n+\n+test_expect_success 'reset --merge-safe keeps changes it does not touch' '\n+     echo \"line 4\" >> file2 &&\n+     git add file2 &&\n+     git reset --merge-safe HEAD^ &&\n+     grep 4 file2 &&\n+     git reset --merge-safe HEAD@{1} &&\n+     grep 4 file2 &&\n+     grep 4 file1 &&\n+     git reset --hard HEAD\n+'\n+\n test_expect_success 'reset --merge fails with changes in file it touches' '\n-     echo \"line 6\" >> file1 &&\n+     echo \"line 5\" >> file1 &&\n      test_tick &&\n-     git commit -m \"add line 6\" file1 &&\n+     git commit -m \"add line 5\" file1 &&\n      sed -e \"s/line 1/changed line 1/\" <file1 >file3 &&\n      mv file3 file1 &&\n      test_must_fail git reset --merge HEAD^ 2>err.log &&\n+     grep file1 err.log | grep \"not uptodate\" &&\n+     git reset --hard HEAD^\n+'\n+\n+test_expect_success 'reset --merge-safe fails with changes in file it touches' '\n+     echo \"line 5\" >> file1 &&\n+     test_tick &&\n+     git commit -m \"add line 5\" file1 &&\n+     sed -e \"s/line 1/changed line 1/\" <file1 >file3 &&\n+     mv file3 file1 &&\n+     test_must_fail git reset --merge-safe HEAD^ 2>err.log &&\n      grep file1 err.log | grep \"not uptodate\"\n '\n \n-- \n1.6.5.rc0.150.g38fe6\n"},{"id":"123366","messageId":"7vfxamzqga.fsf@alter.siamese.dyndns.org","threadId":"20960","inReplyTo":"20090916041443.3737.63217.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 3/4] reset: add option \"--merge-safe\" to \"git reset\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-16T18:37:09Z","receivedAt":"2009-09-16T18:37:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> From: Stephan Beyer <s-beyer@gmx.net>\n>\n> This option is nearly like \"--merge\" except that it is\n> safer. The table below show the differences between these\n> options.\n>\n> working index HEAD target         working index HEAD\n>   B      B     A     A   --m-s      B      A     A\n>                          --merge    A      A     A\n>   B      B     A     C   --m-s       (disallowed)\n>                          --merge    C      C     C\n>\n> In this table, A, B and C are some different states of\n> a file. For example the first 2 lines of the table mean\n> that if a file is in state B in the working tree and\n> the index, and in a different state A in HEAD and in\n> the target, then \"git reset --merge-safe target\" will\n> put the file in state B in the working tree and in\n> state A in the index and HEAD.\n\nAt first, I had to spend a few minutes guessing what you meant by \"target\"\nin the table.  All the other words are well known and do not need to be\nexplained, but you can make it even easier to understand by updating the\nsentence before the table, perhaps like:\n\n    When running \"git reset --option target\" to reset the HEAD to another\n    commit (as a special case \"target\" could be the same as HEAD), here\n    is what happens to paths in various state.\n\nAs you mentioned in the proposed commit log message of the other entry,\nyou have a different behaviour for unmerged case.  Can you add that case\nto the table as well?\n\nThe original use case of Linus's \"reset --merge\" was:\n\n    $ edit ... ;# you may have some local changes to the work tree\n    $ git merge/pull ...\n    ... (1) it merges cleanly;\n    ... (2) you see conflicts and do not commit, or\n    ... (3) you resolve conflicts and commit, while leaving the earlier\n    ...     modified paths alone.\n    ... In any of the three cases, you inspect the result, and say\n    ... \"ah, crap!\"\n    ... You want to go back to the state before the merge, i.e.\n    ... target = HEAD^ in (1) or (3) above and target = HEAD in (2).\n    $ git reset --merge $target\n\nRecall that \"git merge/pull ...\" step does not even touch anything if you\nhave a dirty index (i.e. \"diff --cached\" is non-empty), so any difference\nbetween the index and HEAD to reset the failed merge away must come from\nthe merge itself\n\nImmediately before you run \"reset --merge\" in the above sequence, you can\ncategorize the paths in various states this way:\n\n  work   index  HEAD  how that path got into this state...\n    A      A     A    not involved in this merge.\n    B      A     A    not involved in this merge, originally modified.\n    B      B     A    cleanly merged.\n    B      B     B    cleanly merged (and committed if (1) or (3)).\n    C      U     A    merge left conflicts\n\nwhere U denotes unmerged path in the index, and C is a file in the work\ntree with conflict markers.  The path had content A in HEAD before the\nstart of the merge that you are resetting away.\n\nNote that the target is A in all cases in the above table.\n\nWe would want to go back to HEAD == index == target for all of these\ncases, and discarding B in \"cleanly merged\" entries is absolutely the\nright thing to do.  Clearly, --merge-safe is _not_ designed to work in\nthis scenario.\n\nDon't get me wrong.  I am not saying --merge-safe is unsafe nor useless.\n\nI am trying to show a way with an intended use-case (and the mechanical\nnotation you and Daniel wrote up, which is very nice to illustrate what\nexactly happens) to explain how --merge works, and more importantly why\nit works that way.\n\nThat is because I would like to see an intended use case of this new\nfeature in a bigger picture.  With the table, I can see what it does (or\nat least what you wanted it to do), but it does not clearly explain what\nthis new mode of operation is good for, in what context it is designed\nto be used, and what problem it intends to solve.  I think you find it\nvery clear, by reading my explanation above, what Linus's --merge is\nintended to solve:\n\n\tAfter a (possibly conflicted) merge modified my index and my work\n        tree, \"reset --hard\" to the commit before I started the merge will\n        discard the local modifications I had in the work tree before the\n        merge.  \"reset --merge\" was invented to let me go back to the\n        state before the merge, without discarding such local changes I\n        had before the merge.\n\nI want to be able to explain to others what --merge-safe is for in a\nsimilar way myself before I have it in my tree, and I can't (yet).\n"},{"id":"123400","messageId":"200909170554.49416.chriscool@tuxfamily.org","threadId":"20960","inReplyTo":"7vfxamzqga.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/4] reset: add option \"--merge-safe\" to \"git reset\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-09-17T03:54:48Z","receivedAt":"2009-09-17T03:54:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wednesday 16 September 2009, Junio C Hamano wrote:\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > From: Stephan Beyer <s-beyer@gmx.net>\n> >\n> > This option is nearly like \"--merge\" except that it is\n> > safer. The table below show the differences between these\n> > options.\n> >\n> > working index HEAD target         working index HEAD\n> >   B      B     A     A   --m-s      B      A     A\n> >                          --merge    A      A     A\n> >   B      B     A     C   --m-s       (disallowed)\n> >                          --merge    C      C     C\n> >\n> > In this table, A, B and C are some different states of\n> > a file. For example the first 2 lines of the table mean\n> > that if a file is in state B in the working tree and\n> > the index, and in a different state A in HEAD and in\n> > the target, then \"git reset --merge-safe target\" will\n> > put the file in state B in the working tree and in\n> > state A in the index and HEAD.\n>\n> At first, I had to spend a few minutes guessing what you meant by\n> \"target\" in the table.  All the other words are well known and do not\n> need to be explained, but you can make it even easier to understand by\n> updating the sentence before the table, perhaps like:\n>\n>     When running \"git reset --option target\" to reset the HEAD to another\n>     commit (as a special case \"target\" could be the same as HEAD), here\n>     is what happens to paths in various state.\n\nOk, I will update the sentence like this.\n\n> As you mentioned in the proposed commit log message of the other entry,\n> you have a different behaviour for unmerged case.  Can you add that case\n> to the table as well?\n\nThe behavior is not different between --merge and --merge-safe, the behavior \nis different between --merge before patch 2/4 and --merge after patch 2/4.\nI will add a test case to show this.\n\n> The original use case of Linus's \"reset --merge\" was:\n>\n>     $ edit ... ;# you may have some local changes to the work tree\n>     $ git merge/pull ...\n>     ... (1) it merges cleanly;\n>     ... (2) you see conflicts and do not commit, or\n>     ... (3) you resolve conflicts and commit, while leaving the earlier\n>     ...     modified paths alone.\n>     ... In any of the three cases, you inspect the result, and say\n>     ... \"ah, crap!\"\n>     ... You want to go back to the state before the merge, i.e.\n>     ... target = HEAD^ in (1) or (3) above and target = HEAD in (2).\n>     $ git reset --merge $target\n\nI think that Daniel found out that the above \"reset --merge\" command did not \nworked well in case (2) before patch 2/4.\n\n> Recall that \"git merge/pull ...\" step does not even touch anything if you\n> have a dirty index (i.e. \"diff --cached\" is non-empty), so any difference\n> between the index and HEAD to reset the failed merge away must come from\n> the merge itself\n>\n> Immediately before you run \"reset --merge\" in the above sequence, you can\n> categorize the paths in various states this way:\n>\n>   work   index  HEAD  how that path got into this state...\n>     A      A     A    not involved in this merge.\n>     B      A     A    not involved in this merge, originally modified.\n>     B      B     A    cleanly merged.\n>     B      B     B    cleanly merged (and committed if (1) or (3)).\n>     C      U     A    merge left conflicts\n>\n> where U denotes unmerged path in the index, and C is a file in the work\n> tree with conflict markers.  The path had content A in HEAD before the\n> start of the merge that you are resetting away.\n>\n> Note that the target is A in all cases in the above table.\n>\n> We would want to go back to HEAD == index == target for all of these\n> cases, and discarding B in \"cleanly merged\" entries is absolutely the\n> right thing to do.  Clearly, --merge-safe is _not_ designed to work in\n> this scenario.\n\nYes.\n\n> Don't get me wrong.  I am not saying --merge-safe is unsafe nor useless.\n>\n> I am trying to show a way with an intended use-case (and the mechanical\n> notation you and Daniel wrote up, which is very nice to illustrate what\n> exactly happens) to explain how --merge works, and more importantly why\n> it works that way.\n>\n> That is because I would like to see an intended use case of this new\n> feature in a bigger picture.  With the table, I can see what it does (or\n> at least what you wanted it to do), but it does not clearly explain what\n> this new mode of operation is good for, in what context it is designed\n> to be used, and what problem it intends to solve.  I think you find it\n> very clear, by reading my explanation above, what Linus's --merge is\n> intended to solve:\n>\n> \tAfter a (possibly conflicted) merge modified my index and my work\n>         tree, \"reset --hard\" to the commit before I started the merge\n> will discard the local modifications I had in the work tree before the\n> merge.  \"reset --merge\" was invented to let me go back to the state\n> before the merge, without discarding such local changes I had before the\n> merge.\n>\n> I want to be able to explain to others what --merge-safe is for in a\n> similar way myself before I have it in my tree, and I can't (yet).\n\nI understand, and I think that Stephan designed the \"allow_dirty\" feature \n(that became --merge-safe in this patch series) because he wanted to let \nthe user do a cherry-pick or an improved cherry-pick (or even run the \nsequencer) with a dirty working tree or index without the risk of losing \nsome work.\n\nBut I think that it can be usefull in case like:\n\n$ \"hack something\"\n$ git commit ...\n$ \"hack something else\"\n$ git add \"some of the files\"\n$ \"find out previous commit was crap\"\n$ git reset --merge-safe HEAD^\n\nHere using \"--merge-safe\" can be usefull because you don't want to lose \nstuff in your current index and work tree.\n\nBest regards,\nChristian.\n"},{"id":"123407","messageId":"7veiq6kuv3.fsf@alter.siamese.dyndns.org","threadId":"20960","inReplyTo":"200909170554.49416.chriscool@tuxfamily.org","subject":"Re: [PATCH v2 3/4] reset: add option \"--merge-safe\" to \"git reset\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-17T05:23:12Z","receivedAt":"2009-09-17T05:23:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> But I think that it can be usefull in case like:\n>\n> $ \"hack something\"\n> $ git commit ...\n> $ \"hack something else\"\n> $ git add \"some of the files\"\n> $ \"find out previous commit was crap\"\n> $ git reset --merge-safe HEAD^\n>\n> Here using \"--merge-safe\" can be usefull because you don't want to lose \n> stuff in your current index and work tree.\n\nThat depends on the reason why you are resetting away the \"crap commit\"\nand what your next move is.\n\nIf you are going to run \"git commit\", perhaps after some further edit, in\norder to fix what the \"crap commit\" did not quite do right, what your new\noption does is actively a wrong thing to do.  \"Some of the files\" you\nadded after that \"crap commit\" are about \"hack something else\", i.e. not\npart of the work to fix the \"crap commit\", and their changes should not be\nincluded in the amended commit, no?\n\nIn general, \"reset\" should be about matching the index entries to the\nnamed commit, so that you can start from the clean, known slate to redo\nthe commit you wanted to make on top of it.  You would need a very good\nuse case to deviate from it and leave the index entry different from that\nof the commit you are switching to; otherwise you would greatly confuse\nthe end users.\n"}]}