{"thread":{"id":"60032","subject":"[PATCH] update-index: do not read HEAD and MERGE_HEAD unconditionally","startedAt":"2023-07-27T19:24:07Z","lastAt":"2023-07-27T19:24:07Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479936","messageId":"xmqqila5b1ke.fsf@gitster.g","threadId":"60032","inReplyTo":null,"subject":"[PATCH] update-index: do not read HEAD and MERGE_HEAD unconditionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-27T19:24:01Z","receivedAt":"2023-07-27T19:24:07Z","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 <gitster@pobox.com>\n---\n * Once the codepath starts using resolve-undo data for recovering\n   from \"resolved to removal\", I think we can fully remove this\n   fallback codepath.  But we'd need to first move away from the\n   unmerge_index_entry_at() that assumes that the path to be\n   unresolved must be in the index.  That should happen in future\n   patches.\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 aee3cb8cbd..853ec9eb7a 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -640,6 +640,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 comit 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@@ -678,10 +693,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@@ -712,27 +737,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-459-gb4fce4b6e4\n\n"}]}