Currently when a user doesn't resolve a conflict in a file, but
commits the file with the conflict markers, and later the file ends up
in a state in which rerere can't handle it, subsequent rerere
operations that are interested in that path, such as 'rerere clear' or
'rerere forget <path>' will fail, or even worse in the case of 'rerere
clear' segfault.
Such states include nested conflicts, or an extra conflict marker that
doesn't have any match.
This is because the first 'git rerere' when there was only one
conflict in the file leaves an entry in the MERGE_RR file behind. The
next 'git rerere' will then pick the rerere ID for that file up, and
not assign a new ID as it can't successfully calculate one. It will
however still try to do the rerere operation, because of the existing
ID. As the handle_file function fails, it will remove the 'preimage'
for the ID in the process, while leaving the ID in the MERGE_RR file.
Now when 'rerere clear' for example is run, it will segfault in
'has_rerere_resolution', because status is NULL.
To fix this, remove the rerere ID from the MERGE_RR file in the case
when we can't handle it, and remove the corresponding variant from
.git/rr-cache/. Removing it unconditionally is fine here, because if
the user would have resolved the conflict and ran rerere, the entry
would no longer be in the MERGE_RR file, so we wouldn't have this
problem in the first place, while if the conflict was not resolved,
the only thing that's left in the folder is the 'preimage', which by
itself will be regenerated by git if necessary, so the user won't
loose any work.
Note that other variants that have the same conflict ID will not be
touched.
Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
---
rerere.c | 12 +++++++-----
t/t4200-rerere.sh | 22 ++++++++++++++++++++++
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/rerere.c b/rerere.c
index da1ab54027..895ad80c0c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -823,10 +823,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
struct rerere_id *id;
unsigned char sha1[20];
const char *path = conflict.items[i].string;
- int ret;
-
- if (string_list_has_string(rr, path))
- continue;
+ int ret, has_string;
/*
* Ask handle_file() to scan and assign a
@@ -834,7 +831,12 @@ static int do_plain_rerere(struct string_list *rr, int fd)
* yet.
*/
ret = handle_file(path, sha1, NULL);
- if (ret < 1)
+ has_string = string_list_has_string(rr, path);
+ if (ret < 0 && has_string) {
+ remove_variant(string_list_lookup(rr, path)->util);
+ string_list_remove(rr, path, 1);
+ }
+ if (ret < 1 || has_string)
continue;
id = new_rerere_id(sha1);
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 8417e5a4b1..34f0518a5e 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -580,4 +580,26 @@ test_expect_success 'multiple identical conflicts' '
count_pre_post 0 0
'
+test_expect_success 'rerere with unexpected conflict markers does not crash' '
+ git reset --hard &&
+
+ git checkout -b branch-1 master &&
+ echo "bar" >test &&
+ git add test &&
+ git commit -q -m two &&
+
+ git reset --hard &&
+ git checkout -b branch-2 master &&
+ echo "foo" >test &&
+ git add test &&
+ git commit -q -a -m one &&
+
+ test_must_fail git merge branch-1 &&
+ sed "s/bar/>>>>>>> a/" >test.tmp <test &&
+ mv test.tmp test &&
+ git rerere &&
+
+ git rerere clear
+'
+
test_done
--
2.17.0.410.g65aef3a6c4
