{"thread":{"id":"60047","subject":"[PATCH 3/7] update-index: use unmerge_index_entry() to support removal","startedAt":"2023-07-31T22:45:06Z","lastAt":"2023-08-04T19:03:43Z","messageCount":10,"participants":["Junio C Hamano","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"480017","messageId":"20230731224409.4181277-4-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 3/7] update-index: use unmerge_index_entry() to support removal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:05Z","receivedAt":"2023-07-31T22:45:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"update-index --unresolve\" uses the unmerge_index_entry_at() that\nassumes that the path to be unresolved must be in the index, which\nmakes it impossible to unresolve a path that was resolved as removal.\n\nRewrite unresolve_one() to use the unmerge_index_entry() to support\nunresolving such a path.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/update-index.c | 38 ++++++++++++++++++++++----------------\n t/t2030-unresolve-info.sh | 37 +++++++++++++++++++++++++++++++++----\n 2 files changed, 55 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 47cd68e9d5..ecd1c0c2d3 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -660,26 +660,31 @@ static int unresolve_one(const char *path)\n \tint pos;\n \tint ret = 0;\n \tstruct cache_entry *ce_2 = NULL, *ce_3 = NULL;\n+\tstruct resolve_undo_info *ru = NULL;\n+\n+\tif (the_index.resolve_undo) {\n+\t\tstruct string_list_item *item;\n+\t\titem = string_list_lookup(the_index.resolve_undo, path);\n+\t\tif (item) {\n+\t\t\tru = item->util;\n+\t\t\titem->util = NULL;\n+\t\t}\n+\t}\n+\n+\t/* resolve-undo record exists for the path */\n+\tif (ru) {\n+\t\tret = unmerge_index_entry(&the_index, path, ru);\n+\t\tfree(ru);\n+\t\treturn ret;\n+\t}\n \n \t/* See if there is such entry in the index. */\n \tpos = index_name_pos(&the_index, path, namelen);\n \tif (0 <= pos) {\n-\t\t/* already merged */\n-\t\tpos = unmerge_index_entry_at(&the_index, pos);\n-\t\tif (pos < the_index.cache_nr) {\n-\t\t\tconst struct cache_entry *ce = the_index.cache[pos];\n-\t\t\tif (ce_stage(ce) &&\n-\t\t\t ce_namelen(ce) == namelen &&\n-\t\t\t !memcmp(ce->name, path, namelen))\n-\t\t\t\treturn 0;\n-\t\t}\n-\t\t/* no resolve-undo information; fall back */\n+\t\t; /* resolve-undo record was used already -- fall back */\n \t} else {\n-\t\t/* If there isn't, either it is unmerged, or\n-\t\t * resolved as \"removed\" by mistake. We do not\n-\t\t * want to do anything in the former case.\n-\t\t */\n-\t\tpos = -pos-1;\n+\t\t/* Is it unmerged? */\n+\t\tpos = -pos - 1;\n \t\tif (pos < the_index.cache_nr) {\n \t\t\tconst struct cache_entry *ce = the_index.cache[pos];\n \t\t\tif (ce_namelen(ce) == namelen &&\n@@ -687,9 +692,10 @@ static int unresolve_one(const char *path)\n \t\t\t\tfprintf(stderr,\n \t\t\t\t\t\"%s: skipping still unmerged path.\\n\",\n \t\t\t\t\tpath);\n-\t\t\t\tgoto free_return;\n \t\t\t}\n+\t\t\tgoto free_return;\n \t\t}\n+\t\t/* No, such a path does not exist -- removed */\n \t}\n \n \t/*\ndiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\nindex d4e7760df5..3eda385ca2 100755\n--- a/t/t2030-unresolve-info.sh\n+++ b/t/t2030-unresolve-info.sh\n@@ -37,11 +37,17 @@ prime_resolve_undo () {\n \tgit checkout second^0 &&\n \ttest_tick &&\n \ttest_must_fail git merge third^0 &&\n-\techo merge does not leave anything &&\n \tcheck_resolve_undo empty &&\n-\techo different >fi/le &&\n-\tgit add fi/le &&\n-\techo resolving records &&\n+\n+\t# how should the conflict be resolved?\n+\tcase \"$1\" in\n+\tremove)\n+\t\trm -f file/le && git rm fi/le\n+\t\t;;\n+\t*) # modify\n+\t\techo different >fi/le && git add fi/le\n+\t\t;;\n+\tesac\n \tcheck_resolve_undo recorded fi/le initial:fi/le second:fi/le third:fi/le\n }\n \n@@ -122,6 +128,8 @@ test_expect_success 'add records checkout -m undoes' '\n test_expect_success 'unmerge with plumbing' '\n \tprime_resolve_undo &&\n \tgit update-index --unresolve fi/le &&\n+\tgit ls-files --resolve-undo fi/le >actual &&\n+\ttest_must_be_empty actual &&\n \tgit ls-files -u >actual &&\n \ttest_line_count = 3 actual\n '\n@@ -130,6 +138,27 @@ test_expect_success 'unmerge can be done even after committing' '\n \tprime_resolve_undo &&\n \tgit commit -m \"record to nuke MERGE_HEAD\" &&\n \tgit update-index --unresolve fi/le &&\n+\tgit ls-files --resolve-undo fi/le >actual &&\n+\ttest_must_be_empty actual &&\n+\tgit ls-files -u >actual &&\n+\ttest_line_count = 3 actual\n+'\n+\n+test_expect_success 'unmerge removal' '\n+\tprime_resolve_undo remove &&\n+\tgit update-index --unresolve fi/le &&\n+\tgit ls-files --resolve-undo fi/le >actual &&\n+\ttest_must_be_empty actual &&\n+\tgit ls-files -u >actual &&\n+\ttest_line_count = 3 actual\n+'\n+\n+test_expect_success 'unmerge removal after committing' '\n+\tprime_resolve_undo remove &&\n+\tgit commit -m \"record to nuke MERGE_HEAD\" &&\n+\tgit update-index --unresolve fi/le &&\n+\tgit ls-files --resolve-undo fi/le >actual &&\n+\ttest_must_be_empty actual &&\n \tgit ls-files -u >actual &&\n \ttest_line_count = 3 actual\n '\n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480018","messageId":"20230731224409.4181277-3-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 2/7] resolve-undo: allow resurrecting conflicted state that resolved to deletion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:04Z","receivedAt":"2023-07-31T22:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The resolve-undo index extension records up to three (mode, object\nname) tuples for non-zero stages for each path that was resolved,\nto be used to recreate the original conflicted state later when the\nuser requests.\n\nThe unmerge_index_entry_at() function uses the resolve-undo data to\ndo so, but it assumes that the path for which the conflicted state\nneeds to be recreated can be specified by the position in the\nactive_cache[] array. This obviously cannot salvage the state of\nconflicted paths that were resolved by removing them. For example,\na delete-modify conflict, in which the change whose \"modify\" side\nmade is a trivial typofix, may legitimately be resolved to remove\nthe path, and resolve-undo extension does record the two (mode,\nobject name) tuples for the common ancestor version and their\nversion, lacking our version. But after recording such a removal of\nthe path, you should be able to use resolve-undo data to recreate\nthe conflicted state.\n\nIntroduce a new unmerge_index_entry() helper function that takes the\npath (which does not necessarily have to exist in the active_cache[]\narray) and resolve-undo data, and use it to reimplement unmerge_index()\npublic function that is used by \"git rerere\".\n\nThe limited interface is still kept for now, as it is used by \"git\ncheckout -m\" and \"git update-index --unmerge\", but these two codepaths\nwill be updated to lift the assumption to allow conflicts that resolved\nto deletion can be recreated.\n\nSigned-off-by: Junio C Hamano \n---\n resolve-undo.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----\n resolve-undo.h | 1 +\n 2 files changed, 44 insertions(+), 5 deletions(-)\n\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 70a6db526d..3b0244e210 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -182,19 +182,57 @@ void unmerge_marked_index(struct index_state *istate)\n \t}\n }\n \n+int unmerge_index_entry(struct index_state *istate, const char *path,\n+\t\t\tstruct resolve_undo_info *ru)\n+{\n+\tint i = index_name_pos(istate, path, strlen(path));\n+\n+\tif (i < 0) {\n+\t\t/* unmerged? */\n+\t\ti = -i - 1;\n+\t\tif (i < istate->cache_nr &&\n+\t\t !strcmp(istate->cache[i]->name, path))\n+\t\t\t/* yes, it is already unmerged */\n+\t\t\treturn 0;\n+\t\t/* fallthru: resolved to removal */\n+\t} else {\n+\t\t/* merged - remove it to replace it with unmerged entries */\n+\t\tremove_index_entry_at(istate, i);\n+\t}\n+\n+\tfor (i = 0; i < 3; i++) {\n+\t\tstruct cache_entry *ce;\n+\t\tif (!ru->mode[i])\n+\t\t\tcontinue;\n+\t\tce = make_cache_entry(istate, ru->mode[i], &ru->oid[i],\n+\t\t\t\t path, i + 1, 0);\n+\t\tif (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD))\n+\t\t\treturn error(\"cannot unmerge '%s'\", path);\n+\t}\n+\treturn 0;\n+}\n+\n void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)\n {\n-\tint i;\n+\tstruct string_list_item *item;\n \n \tif (!istate->resolve_undo)\n \t\treturn;\n \n \t/* TODO: audit for interaction with sparse-index. */\n \tensure_full_index(istate);\n-\tfor (i = 0; i < istate->cache_nr; i++) {\n-\t\tconst struct cache_entry *ce = istate->cache[i];\n-\t\tif (!ce_path_match(istate, ce, pathspec, NULL))\n+\n+\tfor_each_string_list_item(item, istate->resolve_undo) {\n+\t\tconst char *path = item->string;\n+\t\tstruct resolve_undo_info *ru = item->util;\n+\t\tif (!item->util)\n+\t\t\tcontinue;\n+\t\tif (!match_pathspec(istate, pathspec,\n+\t\t\t\t item->string, strlen(item->string),\n+\t\t\t\t 0, NULL, 0))\n \t\t\tcontinue;\n-\t\ti = unmerge_index_entry_at(istate, i);\n+\t\tunmerge_index_entry(istate, path, ru);\n+\t\tfree(ru);\n+\t\titem->util = NULL;\n \t}\n }\ndiff --git a/resolve-undo.h b/resolve-undo.h\nindex c5deafc92f..1ae321c88b 100644\n--- a/resolve-undo.h\n+++ b/resolve-undo.h\n@@ -18,6 +18,7 @@ void resolve_undo_write(struct strbuf *, struct string_list *);\n struct string_list *resolve_undo_read(const char *, unsigned long);\n void resolve_undo_clear_index(struct index_state *);\n int unmerge_index_entry_at(struct index_state *, int);\n+int unmerge_index_entry(struct index_state *, const char *, struct resolve_undo_info *);\n void unmerge_index(struct index_state *, const struct pathspec *);\n void unmerge_marked_index(struct index_state *);\n \n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480019","messageId":"20230731224409.4181277-5-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 4/7] update-index: remove stale fallback code for \"--unresolve\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:06Z","receivedAt":"2023-07-31T22:45:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"update-index --unresolve\" is a relatively old feature that was\nintroduced in Git v1.4.1 (June 2006), which predates the\nresolve-undo extension introduced in Git v1.7.0 (February 2010).\nThe original code that was limited only to work during a merge (and\nnot during a rebase or a cherry-pick) has been kept as the fallback\ncodepath to be used as a transition measure.\n\nBy now, for more than 10 years we have stored resolve-undo extension\nin the index file, and the fallback code way outlived its usefulness.\n\nRemove it, together with two file-scope static global variables.\nOne of these variables is still used by surviving function, but it\ndoes not have to be a global at all, so move it to local to that\nfunction.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/update-index.c | 114 +++++------------------------------------\n 1 file changed, 12 insertions(+), 102 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex ecd1c0c2d3..def7f98504 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -608,9 +608,6 @@ static const char * const update_index_usage[] = {\n \tNULL\n };\n \n-static struct object_id head_oid;\n-static struct object_id merge_head_oid;\n-\n static struct cache_entry *read_one_ent(const char *which,\n \t\t\t\t\tstruct object_id *ent, const char *path,\n \t\t\t\t\tint namelen, int stage)\n@@ -639,107 +636,19 @@ static struct cache_entry *read_one_ent(const char *which,\n \treturn ce;\n }\n \n-static int read_head_pointers(void)\n-{\n-\tstatic int result = -2; /* unknown yet */\n-\n-\tif (result == -2) {\n-\t\tresult = -1;\n-\t\tif (read_ref(\"HEAD\", &head_oid))\n-\t\t\treturn error(\"No HEAD -- no initial commit yet?\");\n-\t\tif (read_ref(\"MERGE_HEAD\", &merge_head_oid))\n-\t\t\treturn error(\"Not in the middle of a merge\");\n-\t\tresult = 0;\n-\t}\n-\treturn result;\n-}\n-\n static int unresolve_one(const char *path)\n {\n-\tint namelen = strlen(path);\n-\tint pos;\n-\tint ret = 0;\n-\tstruct cache_entry *ce_2 = NULL, *ce_3 = NULL;\n-\tstruct resolve_undo_info *ru = NULL;\n-\n-\tif (the_index.resolve_undo) {\n-\t\tstruct string_list_item *item;\n-\t\titem = string_list_lookup(the_index.resolve_undo, path);\n-\t\tif (item) {\n-\t\t\tru = item->util;\n-\t\t\titem->util = NULL;\n-\t\t}\n-\t}\n-\n-\t/* resolve-undo record exists for the path */\n-\tif (ru) {\n-\t\tret = unmerge_index_entry(&the_index, path, ru);\n-\t\tfree(ru);\n-\t\treturn ret;\n-\t}\n-\n-\t/* See if there is such entry in the index. */\n-\tpos = index_name_pos(&the_index, path, namelen);\n-\tif (0 <= pos) {\n-\t\t; /* resolve-undo record was used already -- fall back */\n-\t} else {\n-\t\t/* Is it unmerged? */\n-\t\tpos = -pos - 1;\n-\t\tif (pos < the_index.cache_nr) {\n-\t\t\tconst struct cache_entry *ce = the_index.cache[pos];\n-\t\t\tif (ce_namelen(ce) == namelen &&\n-\t\t\t !memcmp(ce->name, path, namelen)) {\n-\t\t\t\tfprintf(stderr,\n-\t\t\t\t\t\"%s: skipping still unmerged path.\\n\",\n-\t\t\t\t\tpath);\n-\t\t\t}\n-\t\t\tgoto free_return;\n-\t\t}\n-\t\t/* No, such a path does not exist -- removed */\n-\t}\n-\n-\t/*\n-\t * We are not using resolve-undo information but just\n-\t * populating the stages #2 and #3 from HEAD and MERGE_HEAD.\n-\t *\n-\t * This is a flawed replacement of true \"unresolve\", as we do\n-\t * not have a way to recreate the stage #1 for the common\n-\t * ancestor (which may not be a unique merge-base between the\n-\t * two).\n-\t */\n-\tif (read_head_pointers()) {\n-\t\tret = -1;\n-\t\tgoto free_return;\n-\t}\n-\n-\tce_2 = read_one_ent(\"our\", &head_oid, path, namelen, 2);\n-\tce_3 = read_one_ent(\"their\", &merge_head_oid, path, namelen, 3);\n-\n-\tif (!ce_2 || !ce_3) {\n-\t\tret = -1;\n-\t\tgoto free_return;\n-\t}\n-\tif (oideq(&ce_2->oid, &ce_3->oid) &&\n-\t ce_2->ce_mode == ce_3->ce_mode) {\n-\t\tfprintf(stderr, \"%s: identical in both, skipping.\\n\",\n-\t\t\tpath);\n-\t\tgoto free_return;\n-\t}\n-\n-\tremove_file_from_index(&the_index, path);\n-\tif (add_index_entry(&the_index, ce_2, ADD_CACHE_OK_TO_ADD)) {\n-\t\terror(\"%s: cannot add our version to the index.\", path);\n-\t\tret = -1;\n-\t\tgoto free_return;\n-\t}\n-\tif (!add_index_entry(&the_index, ce_3, ADD_CACHE_OK_TO_ADD))\n-\t\treturn 0;\n-\terror(\"%s: cannot add their version to the index.\", path);\n-\tret = -1;\n- free_return:\n-\tdiscard_cache_entry(ce_2);\n-\tdiscard_cache_entry(ce_3);\n-\treturn ret;\n+\tstruct string_list_item *item;\n+\tint res = 0;\n+\n+\tif (!the_index.resolve_undo)\n+\t\treturn res;\n+\titem = string_list_lookup(the_index.resolve_undo, path);\n+\tif (!item)\n+\t\treturn res; /* no resolve-undo record for the path */\n+\tres = unmerge_index_entry(&the_index, path, item->util);\n+\tFREE_AND_NULL(item->util);\n+\treturn res;\n }\n \n static int do_unresolve(int ac, const char **av,\n@@ -766,6 +675,7 @@ static int do_reupdate(const char **paths,\n \tint pos;\n \tint has_head = 1;\n \tstruct pathspec pathspec;\n+\tstruct object_id head_oid;\n \n \tparse_pathspec(&pathspec, 0,\n \t\t PATHSPEC_PREFER_CWD,\n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480020","messageId":"20230731224409.4181277-7-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 6/7] checkout/restore: add basic tests for --merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:08Z","receivedAt":"2023-07-31T22:45:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though \"checkout --merge -- paths\" had some tests, we never\nmade sure it worked to recreate the conflicted state _after_ the\nresolution was recorded in the index. Also \"restore --merge\" did\nnot even have any tests.\n\nCurrently these commands use the unmerge_marked_index() interface\nthat cannot handle paths that have been resolved as removal, and\ntests for that case are marked with test_expect_failure; these\nshould eventually be fixed, but not in this patch.\n\nSigned-off-by: Junio C Hamano \n---\n t/t2070-restore.sh | 64 ++++++++++++++++++++++++++++++++++++++++++++++\n t/t7201-co.sh | 42 ++++++++++++++++++++++++++++++\n 2 files changed, 106 insertions(+)\n\ndiff --git a/t/t2070-restore.sh b/t/t2070-restore.sh\nindex fd775807e7..d97ecc2483 100755\n--- a/t/t2070-restore.sh\n+++ b/t/t2070-restore.sh\n@@ -137,6 +137,70 @@ test_expect_success 'restore --staged invalidates cache tree for deletions' '\n \ttest_must_fail git rev-parse HEAD:new1\n '\n \n+test_expect_success 'restore --merge to unresolve' '\n+\tO=$(echo original | git hash-object -w --stdin) &&\n+\tA=$(echo ourside | git hash-object -w --stdin) &&\n+\tB=$(echo theirside | git hash-object -w --stdin) &&\n+\t{\n+\t\techo \"100644 $O 1\tfile\" &&\n+\t\techo \"100644 $A 2\tfile\" &&\n+\t\techo \"100644 $B 3\tfile\"\n+\t} | git update-index --index-info &&\n+\techo nothing >file &&\n+\tgit restore --worktree --merge file &&\n+\tcat >expect <<-\\EOF &&\n+\t<<<<<<< ours\n+\tourside\n+\t=======\n+\ttheirside\n+\t>>>>>>> theirs\n+\tEOF\n+\ttest_cmp expect file\n+'\n+\n+test_expect_success 'restore --merge to unresolve after (mistaken) resolution' '\n+\tO=$(echo original | git hash-object -w --stdin) &&\n+\tA=$(echo ourside | git hash-object -w --stdin) &&\n+\tB=$(echo theirside | git hash-object -w --stdin) &&\n+\t{\n+\t\techo \"100644 $O 1\tfile\" &&\n+\t\techo \"100644 $A 2\tfile\" &&\n+\t\techo \"100644 $B 3\tfile\"\n+\t} | git update-index --index-info &&\n+\techo nothing >file &&\n+\tgit add file &&\n+\tgit restore --worktree --merge file &&\n+\tcat >expect <<-\\EOF &&\n+\t<<<<<<< ours\n+\tourside\n+\t=======\n+\ttheirside\n+\t>>>>>>> theirs\n+\tEOF\n+\ttest_cmp expect file\n+'\n+\n+test_expect_failure 'restore --merge to unresolve after (mistaken) resolution' '\n+\tO=$(echo original | git hash-object -w --stdin) &&\n+\tA=$(echo ourside | git hash-object -w --stdin) &&\n+\tB=$(echo theirside | git hash-object -w --stdin) &&\n+\t{\n+\t\techo \"100644 $O 1\tfile\" &&\n+\t\techo \"100644 $A 2\tfile\" &&\n+\t\techo \"100644 $B 3\tfile\"\n+\t} | git update-index --index-info &&\n+\tgit rm -f file &&\n+\tgit restore --worktree --merge file &&\n+\tcat >expect <<-\\EOF &&\n+\t<<<<<<< ours\n+\tourside\n+\t=======\n+\ttheirside\n+\t>>>>>>> theirs\n+\tEOF\n+\ttest_cmp expect file\n+'\n+\n test_expect_success 'restore with merge options are incompatible with certain options' '\n \tfor opts in \\\n \t\t\"--staged --ours\" \\\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 23d4dadbcc..4b07a26c14 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -522,6 +522,48 @@ test_expect_success 'checkout with --merge' '\n \ttest_cmp merged file\n '\n \n+test_expect_success 'checkout -m works after (mistaken) resolution' '\n+\tsetup_conflicting_index &&\n+\techo \"none of the above\" >sample &&\n+\tcat sample >fild &&\n+\tcat sample >file &&\n+\tcat sample >filf &&\n+\t# resolve to something\n+\tgit add file &&\n+\tgit checkout --merge -- fild file filf &&\n+\t{\n+\t\techo \"<<<<<<< ours\" &&\n+\t\techo ourside &&\n+\t\techo \"=======\" &&\n+\t\techo theirside &&\n+\t\techo \">>>>>>> theirs\"\n+\t} >merged &&\n+\ttest_cmp expect fild &&\n+\ttest_cmp expect filf &&\n+\ttest_cmp merged file\n+'\n+\n+test_expect_failure 'checkout -m works after (mistaken) resolution to remove' '\n+\tsetup_conflicting_index &&\n+\techo \"none of the above\" >sample &&\n+\tcat sample >fild &&\n+\tcat sample >file &&\n+\tcat sample >filf &&\n+\t# resolve to remove\n+\tgit rm file &&\n+\tgit checkout --merge -- fild file filf &&\n+\t{\n+\t\techo \"<<<<<<< ours\" &&\n+\t\techo ourside &&\n+\t\techo \"=======\" &&\n+\t\techo theirside &&\n+\t\techo \">>>>>>> theirs\"\n+\t} >merged &&\n+\ttest_cmp expect fild &&\n+\ttest_cmp expect filf &&\n+\ttest_cmp merged file\n+'\n+\n test_expect_success 'checkout with --merge, in diff3 -m style' '\n \tgit config merge.conflictstyle diff3 &&\n \tsetup_conflicting_index &&\n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480021","messageId":"20230731224409.4181277-8-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 7/7] checkout: allow \"checkout -m path\" to unmerge removed paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:09Z","receivedAt":"2023-07-31T22:45:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git checkout -m -- path\" uses the unmerge_marked_index() API, whose\nimplementation is incapable of unresolving a path that was resolved\nas removed. Extend the unmerge_index() API function so that we can\nmark the ce_flags member of the cache entries we add to the index as\nunmerged, and replace use of unmerge_marked_index() with it.\n\nNow, together with its unmerge_index_entry_at() helper function,\nunmerge_marked_index() function is no longer called by anybody, and\ncan safely be removed.\n\nThis makes two known test failures in t2070 and t7201 to succeed.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/checkout.c | 6 ++--\n builtin/update-index.c | 2 +-\n rerere.c | 2 +-\n resolve-undo.c | 75 +++---------------------------------------\n resolve-undo.h | 6 ++--\n t/t2070-restore.sh | 2 +-\n t/t7201-co.sh | 2 +-\n 7 files changed, 13 insertions(+), 82 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b8dfba57c6..98fcf1220a 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -566,6 +566,8 @@ static int checkout_paths(const struct checkout_opts *opts,\n \n \tif (opts->source_tree)\n \t\tread_tree_some(opts->source_tree, &opts->pathspec);\n+\tif (opts->merge)\n+\t\tunmerge_index(&the_index, &opts->pathspec, CE_MATCHED);\n \n \tps_matched = xcalloc(opts->pathspec.nr, 1);\n \n@@ -589,10 +591,6 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t}\n \tfree(ps_matched);\n \n-\t/* \"checkout -m path\" to recreate conflicted state */\n-\tif (opts->merge)\n-\t\tunmerge_marked_index(&the_index);\n-\n \t/* Any unmerged paths? */\n \tfor (pos = 0; pos < the_index.cache_nr; pos++) {\n \t\tconst struct cache_entry *ce = the_index.cache[pos];\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex def7f98504..69fe9c8fcb 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -646,7 +646,7 @@ static int unresolve_one(const char *path)\n \titem = string_list_lookup(the_index.resolve_undo, path);\n \tif (!item)\n \t\treturn res; /* no resolve-undo record for the path */\n-\tres = unmerge_index_entry(&the_index, path, item->util);\n+\tres = unmerge_index_entry(&the_index, path, item->util, 0);\n \tFREE_AND_NULL(item->util);\n \treturn res;\n }\ndiff --git a/rerere.c b/rerere.c\nindex e968d413d6..b525dd9230 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -1112,7 +1112,7 @@ int rerere_forget(struct repository *r, struct pathspec *pathspec)\n \t * recover the original conflicted state and then\n \t * find the conflicted paths.\n \t */\n-\tunmerge_index(r->index, pathspec);\n+\tunmerge_index(r->index, pathspec, 0);\n \tfind_conflict(r, &conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tstruct string_list_item *it = &conflict.items[i];\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 3b0244e210..8e5a8072ed 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -115,75 +115,8 @@ void resolve_undo_clear_index(struct index_state *istate)\n \tistate->cache_changed |= RESOLVE_UNDO_CHANGED;\n }\n \n-int unmerge_index_entry_at(struct index_state *istate, int pos)\n-{\n-\tconst struct cache_entry *ce;\n-\tstruct string_list_item *item;\n-\tstruct resolve_undo_info *ru;\n-\tint i, err = 0, matched;\n-\tchar *name;\n-\n-\tif (!istate->resolve_undo)\n-\t\treturn pos;\n-\n-\tce = istate->cache[pos];\n-\tif (ce_stage(ce)) {\n-\t\t/* already unmerged */\n-\t\twhile ((pos < istate->cache_nr) &&\n-\t\t ! strcmp(istate->cache[pos]->name, ce->name))\n-\t\t\tpos++;\n-\t\treturn pos - 1; /* return the last entry processed */\n-\t}\n-\titem = string_list_lookup(istate->resolve_undo, ce->name);\n-\tif (!item)\n-\t\treturn pos;\n-\tru = item->util;\n-\tif (!ru)\n-\t\treturn pos;\n-\tmatched = ce->ce_flags & CE_MATCHED;\n-\tname = xstrdup(ce->name);\n-\tremove_index_entry_at(istate, pos);\n-\tfor (i = 0; i < 3; i++) {\n-\t\tstruct cache_entry *nce;\n-\t\tif (!ru->mode[i])\n-\t\t\tcontinue;\n-\t\tnce = make_cache_entry(istate,\n-\t\t\t\t ru->mode[i],\n-\t\t\t\t &ru->oid[i],\n-\t\t\t\t name, i + 1, 0);\n-\t\tif (matched)\n-\t\t\tnce->ce_flags |= CE_MATCHED;\n-\t\tif (add_index_entry(istate, nce, ADD_CACHE_OK_TO_ADD)) {\n-\t\t\terr = 1;\n-\t\t\terror(\"cannot unmerge '%s'\", name);\n-\t\t}\n-\t}\n-\tfree(name);\n-\tif (err)\n-\t\treturn pos;\n-\tfree(ru);\n-\titem->util = NULL;\n-\treturn unmerge_index_entry_at(istate, pos);\n-}\n-\n-void unmerge_marked_index(struct index_state *istate)\n-{\n-\tint i;\n-\n-\tif (!istate->resolve_undo)\n-\t\treturn;\n-\n-\t/* TODO: audit for interaction with sparse-index. */\n-\tensure_full_index(istate);\n-\tfor (i = 0; i < istate->cache_nr; i++) {\n-\t\tconst struct cache_entry *ce = istate->cache[i];\n-\t\tif (ce->ce_flags & CE_MATCHED)\n-\t\t\ti = unmerge_index_entry_at(istate, i);\n-\t}\n-}\n-\n int unmerge_index_entry(struct index_state *istate, const char *path,\n-\t\t\tstruct resolve_undo_info *ru)\n+\t\t\tstruct resolve_undo_info *ru, unsigned ce_flags)\n {\n \tint i = index_name_pos(istate, path, strlen(path));\n \n@@ -206,13 +139,15 @@ int unmerge_index_entry(struct index_state *istate, const char *path,\n \t\t\tcontinue;\n \t\tce = make_cache_entry(istate, ru->mode[i], &ru->oid[i],\n \t\t\t\t path, i + 1, 0);\n+\t\tce->ce_flags |= ce_flags;\n \t\tif (add_index_entry(istate, ce, ADD_CACHE_OK_TO_ADD))\n \t\t\treturn error(\"cannot unmerge '%s'\", path);\n \t}\n \treturn 0;\n }\n \n-void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)\n+void unmerge_index(struct index_state *istate, const struct pathspec *pathspec,\n+\t\t unsigned ce_flags)\n {\n \tstruct string_list_item *item;\n \n@@ -231,7 +166,7 @@ void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)\n \t\t\t\t item->string, strlen(item->string),\n \t\t\t\t 0, NULL, 0))\n \t\t\tcontinue;\n-\t\tunmerge_index_entry(istate, path, ru);\n+\t\tunmerge_index_entry(istate, path, ru, ce_flags);\n \t\tfree(ru);\n \t\titem->util = NULL;\n \t}\ndiff --git a/resolve-undo.h b/resolve-undo.h\nindex 1ae321c88b..f3f8462751 100644\n--- a/resolve-undo.h\n+++ b/resolve-undo.h\n@@ -17,9 +17,7 @@ void record_resolve_undo(struct index_state *, struct cache_entry *);\n void resolve_undo_write(struct strbuf *, struct string_list *);\n struct string_list *resolve_undo_read(const char *, unsigned long);\n void resolve_undo_clear_index(struct index_state *);\n-int unmerge_index_entry_at(struct index_state *, int);\n-int unmerge_index_entry(struct index_state *, const char *, struct resolve_undo_info *);\n-void unmerge_index(struct index_state *, const struct pathspec *);\n-void unmerge_marked_index(struct index_state *);\n+int unmerge_index_entry(struct index_state *, const char *, struct resolve_undo_info *, unsigned);\n+void unmerge_index(struct index_state *, const struct pathspec *, unsigned);\n \n #endif\ndiff --git a/t/t2070-restore.sh b/t/t2070-restore.sh\nindex d97ecc2483..16d6348b69 100755\n--- a/t/t2070-restore.sh\n+++ b/t/t2070-restore.sh\n@@ -180,7 +180,7 @@ test_expect_success 'restore --merge to unresolve after (mistaken) resolution' '\n \ttest_cmp expect file\n '\n \n-test_expect_failure 'restore --merge to unresolve after (mistaken) resolution' '\n+test_expect_success 'restore --merge to unresolve after (mistaken) resolution' '\n \tO=$(echo original | git hash-object -w --stdin) &&\n \tA=$(echo ourside | git hash-object -w --stdin) &&\n \tB=$(echo theirside | git hash-object -w --stdin) &&\ndiff --git a/t/t7201-co.sh b/t/t7201-co.sh\nindex 4b07a26c14..df582295df 100755\n--- a/t/t7201-co.sh\n+++ b/t/t7201-co.sh\n@@ -543,7 +543,7 @@ test_expect_success 'checkout -m works after (mistaken) resolution' '\n \ttest_cmp merged file\n '\n \n-test_expect_failure 'checkout -m works after (mistaken) resolution to remove' '\n+test_expect_success 'checkout -m works after (mistaken) resolution to remove' '\n \tsetup_conflicting_index &&\n \techo \"none of the above\" >sample &&\n \tcat sample >fild &&\n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480022","messageId":"20230731224409.4181277-1-gitster@pobox.com","threadId":"60047","inReplyTo":null,"subject":"[PATCH 0/7] unresolve removal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:02Z","receivedAt":"2023-07-31T22:48:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This series is a culmination of various draft patches I have sent\nout recently and have queued in two topics near the tip of 'seen'.\n\nThe primary objective is to allow undoing conflict resolution that\nwas done via \"git rm\". When a conflicted path is resolved with \"git\nadd\" (i.e. replace higher stage index entries with stage #0 entry),\n\"git checkout -m -- path\" and \"git update-index --unresolve path\"\nworked using the resolve-undo information stored in the index, but a\nconflicted path that is resolved with \"git rm\" (i.e. remove higher\nstage index, without adding any stage #0 entry for the path), these\ntwo commands did not work at all, or did not function correctly.\n\n\nJunio C Hamano (7):\n update-index: do not read HEAD and MERGE_HEAD unconditionally\n resolve-undo: allow resurrecting conflicted state that resolved to\n deletion\n update-index: use unmerge_index_entry() to support removal\n update-index: remove stale fallback code for \"--unresolve\"\n checkout/restore: refuse unmerging paths unless checking out of the\n index\n checkout/restore: add basic tests for --merge\n checkout: allow \"checkout -m path\" to unmerge removed paths\n\n Documentation/git-checkout.txt | 9 ++-\n Documentation/git-restore.txt | 4 ++\n builtin/checkout.c | 15 +++--\n builtin/update-index.c | 98 ++++----------------------------\n rerere.c | 2 +-\n resolve-undo.c | 101 ++++++++++++---------------------\n resolve-undo.h | 5 +-\n t/t2030-unresolve-info.sh | 45 +++++++++++++--\n t/t2070-restore.sh | 71 ++++++++++++++++++++++-\n t/t7201-co.sh | 47 +++++++++++++++\n 10 files changed, 230 insertions(+), 167 deletions(-)\n\n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480023","messageId":"20230731224409.4181277-2-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 1/7] update-index: do not read HEAD and MERGE_HEAD unconditionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:03Z","receivedAt":"2023-07-31T22:48:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When \"update-index --unresolve $path\" cannot find the resolve-undo\nrecord for the path the user requested to unresolve, it stuffs the\nblobs from HEAD and MERGE_HEAD to stage #2 and stage #3 as a\nfallback. For this reason, the operation does not even start unless\nboth \"HEAD\" and \"MERGE_HEAD\" exist.\n\nThis is suboptimal in a few ways:\n\n * It does not recreate stage #1. Even though it is a correct\n design decision not to do so (because it is impossible to\n recreate in general cases, without knowing how we got there,\n including what merge strategy was used), it is much less useful\n not to have that information in the index.\n\n * It limits the \"unresolve\" operation only during a conflicted \"git\n merge\" and nothing else. Other operations like \"rebase\",\n \"cherry-pick\", and \"switch -m\" may result in conflicts, and the\n user may want to unresolve the conflict that they incorrectly\n resolved in order to redo the resolution, but the fallback would\n not kick in.\n\n * Most importantly, the entire \"unresolve\" operation is disabled\n after a conflicted merge is committed and MERGE_HEAD is removed,\n even though the index has perfectly usable resolve-undo records.\n\nBy lazily reading the HEAD and MERGE_HEAD only when we need to go to\nthe fallback codepath, we will allow cases where resolve-undo\nrecords are available (which is 100% of the time, unless the user is\nreading from an index file created by Git more than 10 years ago) to\nproceed even after a conflicted merge was committed, during other\nmergy operations that do not use MERGE_HEAD, or after the result of\nsuch mergy operations has been committed.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/update-index.c | 46 ++++++++++++++++++++++++---------------\n t/t2030-unresolve-info.sh | 8 +++++++\n 2 files changed, 36 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 5fab9ad2ec..47cd68e9d5 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -639,6 +639,21 @@ static struct cache_entry *read_one_ent(const char *which,\n \treturn ce;\n }\n \n+static int read_head_pointers(void)\n+{\n+\tstatic int result = -2; /* unknown yet */\n+\n+\tif (result == -2) {\n+\t\tresult = -1;\n+\t\tif (read_ref(\"HEAD\", &head_oid))\n+\t\t\treturn error(\"No HEAD -- no initial commit yet?\");\n+\t\tif (read_ref(\"MERGE_HEAD\", &merge_head_oid))\n+\t\t\treturn error(\"Not in the middle of a merge\");\n+\t\tresult = 0;\n+\t}\n+\treturn result;\n+}\n+\n static int unresolve_one(const char *path)\n {\n \tint namelen = strlen(path);\n@@ -677,10 +692,20 @@ static int unresolve_one(const char *path)\n \t\t}\n \t}\n \n-\t/* Grab blobs from given path from HEAD and MERGE_HEAD,\n-\t * stuff HEAD version in stage #2,\n-\t * stuff MERGE_HEAD version in stage #3.\n+\t/*\n+\t * We are not using resolve-undo information but just\n+\t * populating the stages #2 and #3 from HEAD and MERGE_HEAD.\n+\t *\n+\t * This is a flawed replacement of true \"unresolve\", as we do\n+\t * not have a way to recreate the stage #1 for the common\n+\t * ancestor (which may not be a unique merge-base between the\n+\t * two).\n \t */\n+\tif (read_head_pointers()) {\n+\t\tret = -1;\n+\t\tgoto free_return;\n+\t}\n+\n \tce_2 = read_one_ent(\"our\", &head_oid, path, namelen, 2);\n \tce_3 = read_one_ent(\"their\", &merge_head_oid, path, namelen, 3);\n \n@@ -711,27 +736,12 @@ static int unresolve_one(const char *path)\n \treturn ret;\n }\n \n-static void read_head_pointers(void)\n-{\n-\tif (read_ref(\"HEAD\", &head_oid))\n-\t\tdie(\"No HEAD -- no initial commit yet?\");\n-\tif (read_ref(\"MERGE_HEAD\", &merge_head_oid)) {\n-\t\tfprintf(stderr, \"Not in the middle of a merge.\\n\");\n-\t\texit(0);\n-\t}\n-}\n-\n static int do_unresolve(int ac, const char **av,\n \t\t\tconst char *prefix, int prefix_length)\n {\n \tint i;\n \tint err = 0;\n \n-\t/* Read HEAD and MERGE_HEAD; if MERGE_HEAD does not exist, we\n-\t * are not doing a merge, so exit with success status.\n-\t */\n-\tread_head_pointers();\n-\n \tfor (i = 1; i < ac; i++) {\n \t\tconst char *arg = av[i];\n \t\tchar *p = prefix_path(prefix, prefix_length, arg);\ndiff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh\nindex 2d8c70b03a..d4e7760df5 100755\n--- a/t/t2030-unresolve-info.sh\n+++ b/t/t2030-unresolve-info.sh\n@@ -126,6 +126,14 @@ test_expect_success 'unmerge with plumbing' '\n \ttest_line_count = 3 actual\n '\n \n+test_expect_success 'unmerge can be done even after committing' '\n+\tprime_resolve_undo &&\n+\tgit commit -m \"record to nuke MERGE_HEAD\" &&\n+\tgit update-index --unresolve fi/le &&\n+\tgit ls-files -u >actual &&\n+\ttest_line_count = 3 actual\n+'\n+\n test_expect_success 'rerere and rerere forget' '\n \tmkdir .git/rr-cache &&\n \tprime_resolve_undo &&\n-- \n2.41.0-478-gee48e70a82\n\n"},{"id":"480024","messageId":"20230731224409.4181277-6-gitster@pobox.com","threadId":"60047","inReplyTo":"20230731224409.4181277-1-gitster@pobox.com","subject":"[PATCH 5/7] checkout/restore: refuse unmerging paths unless checking out of the index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T22:44:07Z","receivedAt":"2023-07-31T22:48:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Recreating unmerged index entries using resolve-undo data,\nrecreating conflicted working tree files using unmerged index\nentries, and writing data out of unmerged index entries, make\nsense only when we are checking paths out of the index and not when\nwe are checking paths out of a tree-ish.\n\nAdd an extra check to make sure \"--merge\" and \"--ours/--theirs\"\noptions are rejected when checking out from a tree-ish, update the\ndocument (especially the SYNOPSIS section) to highlight that they\nare incompatible, and add a few tests to make sure the combination\nfails.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-checkout.txt | 9 ++++++---\n Documentation/git-restore.txt | 4 ++++\n builtin/checkout.c | 9 +++++++++\n t/t2070-restore.sh | 7 +++++--\n t/t7201-co.sh | 5 +++++\n 5 files changed, 29 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 4af0904f47..a30e3ebc51 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -12,8 +12,10 @@ SYNOPSIS\n 'git checkout' [-q] [-f] [-m] --detach []\n 'git checkout' [-q] [-f] [-m] [--detach] \n 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] []\n-'git checkout' [-f|--ours|--theirs|-m|--conflict=