update-index: do not read HEAD and MERGE_HEAD unconditionally

When "update-index --unresolve $path" cannot find the resolve-undo record for the path the user requested to unresolve, it stuffs the blobs from HEAD and MERGE_HEAD to stage #2 and stage #3 as a fallback. For this reason, the operation does not even start unless both "HEAD" and "MERGE_HEAD" exist. This is suboptimal in a few ways: * It does not recreate stage #1. Even though it is a correct design decision not to do so (because it is impossible to recreate in general cases, without knowing how we got there, including what merge strategy was used), it is much less useful not to have that information in the index. * It limits the "unresolve" operation only during a conflicted "git merge" and nothing else. Other operations like "rebase", "cherry-pick", and "switch -m" may result in conflicts, and the user may want to unresolve the conflict that they incorrectly resolved in order to redo the resolution, but the fallback would not kick in. * Most importantly, the entire "unresolve" operation is disabled after a conflicted merge is committed and MERGE_HEAD is removed, even though the index has perfectly usable resolve-undo records. By lazily reading the HEAD and MERGE_HEAD only when we need to go to the fallback codepath, we will allow cases where resolve-undo records are available (which is 100% of the time, unless the user is reading from an index file created by Git more than 10 years ago) to proceed even after a conflicted merge was committed, during other mergy operations that do not use MERGE_HEAD, or after the result of such mergy operations has been committed. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jul 31, 2023 at 15:44 UTC 91e07058c5663d01d39a00418c91415d0227d53c
2 files changed +36 -18
builtin/update-index.c
+28 -18
@@ -639,6 +639,21 @@ static struct cache_entry *read_one_ent(const char *which,
639 return ce;
640 }
641
642 +static int read_head_pointers(void)
643 +{
644 + static int result = -2; /* unknown yet */
645 +
646 + if (result == -2) {
647 + result = -1;
648 + if (read_ref("HEAD", &head_oid))
649 + return error("No HEAD -- no initial commit yet?");
650 + if (read_ref("MERGE_HEAD", &merge_head_oid))
651 + return error("Not in the middle of a merge");
652 + result = 0;
653 + }
654 + return result;
655 +}
656 +
657 static int unresolve_one(const char *path)
658 {
659 int namelen = strlen(path);
@@ -677,10 +692,20 @@ static int unresolve_one(const char *path)
692 }
693 }
694
680 - /* Grab blobs from given path from HEAD and MERGE_HEAD,
681 - * stuff HEAD version in stage #2,
682 - * stuff MERGE_HEAD version in stage #3.
695 + /*
696 + * We are not using resolve-undo information but just
697 + * populating the stages #2 and #3 from HEAD and MERGE_HEAD.
698 + *
699 + * This is a flawed replacement of true "unresolve", as we do
700 + * not have a way to recreate the stage #1 for the common
701 + * ancestor (which may not be a unique merge-base between the
702 + * two).
703 */
704 + if (read_head_pointers()) {
705 + ret = -1;
706 + goto free_return;
707 + }
708 +
709 ce_2 = read_one_ent("our", &head_oid, path, namelen, 2);
710 ce_3 = read_one_ent("their", &merge_head_oid, path, namelen, 3);
711
@@ -711,27 +736,12 @@ static int unresolve_one(const char *path)
736 return ret;
737 }
738
714 -static void read_head_pointers(void)
715 -{
716 - if (read_ref("HEAD", &head_oid))
717 - die("No HEAD -- no initial commit yet?");
718 - if (read_ref("MERGE_HEAD", &merge_head_oid)) {
719 - fprintf(stderr, "Not in the middle of a merge.\n");
720 - exit(0);
721 - }
722 -}
723 -
739 static int do_unresolve(int ac, const char **av,
740 const char *prefix, int prefix_length)
741 {
742 int i;
743 int err = 0;
744
730 - /* Read HEAD and MERGE_HEAD; if MERGE_HEAD does not exist, we
731 - * are not doing a merge, so exit with success status.
732 - */
733 - read_head_pointers();
734 -
745 for (i = 1; i < ac; i++) {
746 const char *arg = av[i];
747 char *p = prefix_path(prefix, prefix_length, arg);
t/t2030-unresolve-info.sh
+8
@@ -126,6 +126,14 @@ test_expect_success 'unmerge with plumbing' '
126 test_line_count = 3 actual
127 '
128
129 +test_expect_success 'unmerge can be done even after committing' '
130 + prime_resolve_undo &&
131 + git commit -m "record to nuke MERGE_HEAD" &&
132 + git update-index --unresolve fi/le &&
133 + git ls-files -u >actual &&
134 + test_line_count = 3 actual
135 +'
136 +
137 test_expect_success 'rerere and rerere forget' '
138 mkdir .git/rr-cache &&
139 prime_resolve_undo &&