[PATCH/RFC 0/3] git rerere unresolve file

[PATCH/RFC 0/3] git rerere unresolve file

[Johannes Sixt]

When you commit an incorrect conflict resolution, git rerere gets in the way: 
rerere clear only clears cache entries of not yet resolved conflicts. But 
there is no other way to remove an incorrect resolution short of purging the 
whole rr-cache.

This series implements 'git rerere unresolve' that removes a recorded 
resolution.

This is RFC for two reasons:

- It changes the contents of MERGE_RR in a way that *may* interfer in unwanted 
ways with older versions that do not understand unresolve.

- rerere unresolve also restores the conflict regions. I'm not sure whether 
this is good because there is 'git checkout --conflict=merge file' that does 
it in a better way. See the commit message of patch 3.

If rerere unresolve does *not* restore conflict regions (and record the result 
as preimage right away) we are facing a problem: When the resolution is 
committed, the postimage has differences from the preimage that are not 
related to the conflict. This may inhibit reusing the resolution later when 
the file has new changes.

Johannes Sixt (3):
  rerere: keep a list of resolved files in MERGE_RR
  rerere: make recording of the preimage reusable
  git rerere unresolve file...

 Documentation/git-rerere.txt |   10 +++-
 builtin-rerere.c             |   13 +++-
 rerere.c                     |  153 ++++++++++++++++++++++++++++++++++-------
 rerere.h                     |    4 +-
 4 files changed, 148 insertions(+), 32 deletions(-)

[PATCH/RFC 1/3] rerere: keep a list of resolved files in MERGE_RR

[Johannes Sixt]

Previously, MERGE_RR entries that rerere detected as resolved were removed
from MERGE_RR so that it contained only entries for files that still had
conflicts.

This changes the "database" format to also keep the entries about files
for which resolutions have been recorded. The purpose is to allow that
resolved conflicts can be unresolved. To do that it is necessary to have
a mapping from the file name to the conflict hash.

The new format of MERGE_RR is:

1. Entries about unresolved conflicts.
2. An all-zeros SHA1 as boundary marker.
3. Entries about resolved conflicts.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 builtin-rerere.c |    4 +++-
 rerere.c         |   49 ++++++++++++++++++++++++++++++++++++++++---------
 rerere.h         |    2 +-
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index adfb7b5..275827d 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -101,12 +101,13 @@ static int diff_two(const char *file1, const char *label1,
 int cmd_rerere(int argc, const char **argv, const char *prefix)
 {
 	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list merge_rr_done = { NULL, 0, 0, 1 };
 	int i, fd;
 
 	if (argc < 2)
 		return rerere();
 
-	fd = setup_rerere(&merge_rr);
+	fd = setup_rerere(&merge_rr, &merge_rr_done);
 	if (fd < 0)
 		return 0;
 
@@ -132,5 +133,6 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		usage(git_rerere_usage);
 
 	string_list_clear(&merge_rr, 1);
+	string_list_clear(&merge_rr_done, 1);
 	return 0;
 }
diff --git a/rerere.c b/rerere.c
index 29f95f6..84d0bb7 100644
--- a/rerere.c
+++ b/rerere.c
@@ -23,8 +23,20 @@ int has_rerere_resolution(const char *hex)
 	return !stat(rerere_path(hex, "postimage"), &st);
 }
 
-static void read_rr(struct string_list *rr)
+/*
+ * If MERGE_RR is read and rewritten by an old version, the section bound
+ * would not be preserved, and file names from both sections would be
+ * interleaved. We must make sure that the section marker does not end up
+ * in an arbitrary position. In particular, the safest result is that all
+ * paths are now in the first ("not done") section. Therefore, we choose
+ * a file name that sorts last (for all practical purposes).
+ */
+static const char section_mark[] =
+	"0000000000000000000000000000000000000000\t\377\377\377\377";
+
+static void read_rr(struct string_list *rr, struct string_list *rr_done)
 {
+	struct string_list *section = rr;
 	unsigned char sha1[20];
 	char buf[PATH_MAX];
 	FILE *in = fopen(merge_rr_path, "r");
@@ -43,14 +55,19 @@ static void read_rr(struct string_list *rr)
 			; /* do nothing */
 		if (i == sizeof(buf))
 			die("filename too long");
-		string_list_insert(buf, rr)->util = name;
+		if (prefixcmp(section_mark, name)) {
+			string_list_insert(buf, section)->util = name;
+		} else {
+			free(name);
+			section = rr_done;
+		}
 	}
 	fclose(in);
 }
 
 static struct lock_file write_lock;
 
-static int write_rr(struct string_list *rr, int out_fd)
+static void write_rr_section(struct string_list *rr, int out_fd)
 {
 	int i;
 	for (i = 0; i < rr->nr; i++) {
@@ -65,6 +82,17 @@ static int write_rr(struct string_list *rr, int out_fd)
 		    write_in_full(out_fd, path, length) != length)
 			die("unable to write rerere record");
 	}
+}
+
+static int write_rr(struct string_list *rr,
+		    struct string_list *rr_done, int out_fd)
+{
+	int length = strlen(section_mark)+1;
+
+	write_rr_section(rr, out_fd);
+	if (write_in_full(out_fd, section_mark, length) != length)
+		die("unable to write rerere record");
+	write_rr_section(rr_done, out_fd);
 	if (commit_lock_file(&write_lock) != 0)
 		die("unable to write rerere record");
 	return 0;
@@ -263,7 +291,8 @@ static int update_paths(struct string_list *update)
 	return status;
 }
 
-static int do_plain_rerere(struct string_list *rr, int fd)
+static int do_plain_rerere(struct string_list *rr,
+			   struct string_list *rr_done, int fd)
 {
 	struct string_list conflict = { NULL, 0, 0, 1 };
 	struct string_list update = { NULL, 0, 0, 1 };
@@ -328,13 +357,14 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 		copy_file(rerere_path(name, "postimage"), path, 0666);
 	mark_resolved:
+		string_list_insert(path, rr_done)->util = rr->items[i].util;
 		rr->items[i].util = NULL;
 	}
 
 	if (update.nr)
 		update_paths(&update);
 
-	return write_rr(rr, fd);
+	return write_rr(rr, rr_done, fd);
 }
 
 static int git_rerere_config(const char *var, const char *value, void *cb)
@@ -367,7 +397,7 @@ static int is_rerere_enabled(void)
 	return 1;
 }
 
-int setup_rerere(struct string_list *merge_rr)
+int setup_rerere(struct string_list *merge_rr, struct string_list *merge_rr_done)
 {
 	int fd;
 
@@ -378,17 +408,18 @@ int setup_rerere(struct string_list *merge_rr)
 	merge_rr_path = git_pathdup("MERGE_RR");
 	fd = hold_lock_file_for_update(&write_lock, merge_rr_path,
 				       LOCK_DIE_ON_ERROR);
-	read_rr(merge_rr);
+	read_rr(merge_rr, merge_rr_done);
 	return fd;
 }
 
 int rerere(void)
 {
 	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list merge_rr_done = { NULL, 0, 0, 1 };
 	int fd;
 
-	fd = setup_rerere(&merge_rr);
+	fd = setup_rerere(&merge_rr, &merge_rr_done);
 	if (fd < 0)
 		return 0;
-	return do_plain_rerere(&merge_rr, fd);
+	return do_plain_rerere(&merge_rr, &merge_rr_done, fd);
 }
diff --git a/rerere.h b/rerere.h
index 13313f3..9bb2f13 100644
--- a/rerere.h
+++ b/rerere.h
@@ -3,7 +3,7 @@
 
 #include "string-list.h"
 
-extern int setup_rerere(struct string_list *);
+extern int setup_rerere(struct string_list *, struct string_list *);
 extern int rerere(void);
 extern const char *rerere_path(const char *hex, const char *file);
 extern int has_rerere_resolution(const char *hex);
-- 
1.6.5.2.182.ge039a

[PATCH/RFC 2/3] rerere: make recording of the preimage reusable

[Johannes Sixt]

This factors the code that computes the conflict hash and records the
preimage into a helper function.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 rerere.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/rerere.c b/rerere.c
index 84d0bb7..11fef88 100644
--- a/rerere.c
+++ b/rerere.c
@@ -229,6 +229,25 @@ static int find_conflict(struct string_list *conflict)
 	return 0;
 }
 
+static int record_preimage(struct string_list *rr, const char *path, int force)
+{
+	unsigned char sha1[20];
+	char *hex;
+	int ret;
+
+	ret = handle_file(path, sha1, NULL);
+	if (ret < 1)
+		return -1;
+
+	hex = xstrdup(sha1_to_hex(sha1));
+	string_list_insert(path, rr)->util = hex;
+	if (mkdir(git_path("rr-cache/%s", hex), 0755) && !force)
+		return -1;
+
+	handle_file(path, NULL, rerere_path(hex, "preimage"));
+	return 0;
+}
+
 static int merge(const char *name, const char *path)
 {
 	int ret;
@@ -310,17 +329,8 @@ static int do_plain_rerere(struct string_list *rr,
 	for (i = 0; i < conflict.nr; i++) {
 		const char *path = conflict.items[i].string;
 		if (!string_list_has_string(rr, path)) {
-			unsigned char sha1[20];
-			char *hex;
-			int ret;
-			ret = handle_file(path, sha1, NULL);
-			if (ret < 1)
-				continue;
-			hex = xstrdup(sha1_to_hex(sha1));
-			string_list_insert(path, rr)->util = hex;
-			if (mkdir(git_path("rr-cache/%s", hex), 0755))
+			if (record_preimage(rr, path, 0))
 				continue;
-			handle_file(path, NULL, rerere_path(hex, "preimage"));
 			fprintf(stderr, "Recorded preimage for '%s'\n", path);
 		}
 	}
-- 
1.6.5.2.182.ge039a

[PATCH/RFC 3/3] git rerere unresolve file...

[Johannes Sixt]

There was no way to remove a recorded resolution short of removing the
entire .git/rr-cache. This gets in the way when an incorrect resolution
was recorded.

This implements the subcommand 'git rerere unresolve' that selectively
removes the recorded resolution of the specified files. It also reverts
the resolution so that the files again have conflict markers. However,
these unresolved conflict markers not necessarily reflect "ours" and
"theirs" correctly because the preimage that was stored in the cache has
the conflicted sides in a canonical order.

In handle_file(), the checks for the beginning and end of a conflict
region have to be loosened slightly -- there can be any whitespace,
including a LF, not just a blank -- because after the conflict regions are
restored, there are no trailing blanks and a subsequent 'git rerere' would
not recognize them anymore.

'git checkout --conflict=merge path...' can restore the conflicted file
as well, but it does not remove the recorded resolution.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Documentation/git-rerere.txt |   10 +++++-
 builtin-rerere.c             |    9 +++--
 rerere.c                     |   74 ++++++++++++++++++++++++++++++++++++++----
 rerere.h                     |    2 +
 4 files changed, 84 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index 7dd515b..c9bf3f1 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -7,7 +7,7 @@ git-rerere - Reuse recorded resolution of conflicted merges
 
 SYNOPSIS
 --------
-'git rerere' ['clear'|'diff'|'status'|'gc']
+'git rerere' ['clear'|'diff'|'status'|'unresolve file...'|'gc']
 
 DESCRIPTION
 -----------
@@ -52,6 +52,14 @@ conflicts.  Additional arguments are passed directly to the system
 Like 'diff', but this only prints the filenames that will be tracked
 for resolutions.
 
+'unresolve'::
+
+Removes the resolution that was recorded for the specified files.
+The conflict sections are restored as well, although the direction of
+the "ours" and "theirs" sections may be inverted.
+You can use 'git checkout --conflict=merge file' to restore the
+original conflict markers in the correct direction.
+
 'gc'::
 
 This prunes records of conflicted merges that
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 275827d..5d3a3a5 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -7,7 +7,7 @@
 #include "xdiff-interface.h"
 
 static const char git_rerere_usage[] =
-"git rerere [clear | status | diff | gc]";
+"git rerere [clear | status | diff | unresolve file... | gc]";
 
 /* these values are days */
 static int cutoff_noresolve = 15;
@@ -102,7 +102,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 {
 	struct string_list merge_rr = { NULL, 0, 0, 1 };
 	struct string_list merge_rr_done = { NULL, 0, 0, 1 };
-	int i, fd;
+	int i, fd, ret = 0;
 
 	if (argc < 2)
 		return rerere();
@@ -129,10 +129,13 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 			const char *name = (const char *)merge_rr.items[i].util;
 			diff_two(rerere_path(name, "preimage"), path, path, path);
 		}
+	else if (!strcmp(argv[1], "unresolve"))
+		ret = unresolve_rerere(&merge_rr, &merge_rr_done,
+				       get_pathspec(prefix, &argv[2]), fd);
 	else
 		usage(git_rerere_usage);
 
 	string_list_clear(&merge_rr, 1);
 	string_list_clear(&merge_rr_done, 1);
-	return 0;
+	return ret;
 }
diff --git a/rerere.c b/rerere.c
index 11fef88..b31008d 100644
--- a/rerere.c
+++ b/rerere.c
@@ -140,7 +140,7 @@ static int handle_file(const char *path,
 		git_SHA1_Init(&ctx);
 
 	while (fgets(buf, sizeof(buf), f)) {
-		if (!prefixcmp(buf, "<<<<<<< ")) {
+		if (!prefixcmp(buf, "<<<<<<<") && isspace(buf[7])) {
 			if (hunk != RR_CONTEXT)
 				goto bad;
 			hunk = RR_SIDE_1;
@@ -152,7 +152,7 @@ static int handle_file(const char *path,
 			if (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)
 				goto bad;
 			hunk = RR_SIDE_2;
-		} else if (!prefixcmp(buf, ">>>>>>> ")) {
+		} else if (!prefixcmp(buf, ">>>>>>>") && isspace(buf[7])) {
 			if (hunk != RR_SIDE_2)
 				goto bad;
 			if (strbuf_cmp(&one, &two) > 0)
@@ -248,23 +248,29 @@ static int record_preimage(struct string_list *rr, const char *path, int 
force)
 	return 0;
 }
 
-static int merge(const char *name, const char *path)
+#define RESOLVE 0
+#define UNRESOLVE 1
+#define UNRESOLVE_CHECK 2
+
+static int merge(const char *name, const char *path, int mode)
 {
 	int ret;
 	mmfile_t cur, base, other;
 	mmbuffer_t result = {NULL, 0};
 	xpparam_t xpp = {XDF_NEED_MINIMAL};
+	const char *basename = mode == RESOLVE ? "preimage" : "postimage";
+	const char *othername = mode == RESOLVE ? "postimage" : "preimage";
 
 	if (handle_file(path, NULL, rerere_path(name, "thisimage")) < 0)
 		return 1;
 
 	if (read_mmfile(&cur, rerere_path(name, "thisimage")) ||
-			read_mmfile(&base, rerere_path(name, "preimage")) ||
-			read_mmfile(&other, rerere_path(name, "postimage")))
+			read_mmfile(&base, rerere_path(name, basename)) ||
+			read_mmfile(&other, rerere_path(name, othername)))
 		return 1;
 	ret = xdl_merge(&base, &cur, "", &other, "",
 			&xpp, XDL_MERGE_ZEALOUS, &result);
-	if (!ret) {
+	if (!ret && mode != UNRESOLVE_CHECK) {
 		FILE *f = fopen(path, "w");
 		if (!f)
 			return error("Could not open %s: %s", path,
@@ -347,7 +353,7 @@ static int do_plain_rerere(struct string_list *rr,
 		const char *name = (const char *)rr->items[i].util;
 
 		if (has_rerere_resolution(name)) {
-			if (!merge(name, path)) {
+			if (!merge(name, path, RESOLVE)) {
 				if (rerere_autoupdate)
 					string_list_insert(path, &update);
 				fprintf(stderr,
@@ -433,3 +439,57 @@ int rerere(void)
 		return 0;
 	return do_plain_rerere(&merge_rr, &merge_rr_done, fd);
 }
+
+static int unresolve_check(struct string_list *rr_done, const char *path)
+{
+	struct string_list_item *item = string_list_lookup(path, rr_done);
+
+	if (!item) {
+		warning("not resolved using a previous resolution: %s", path);
+		return 0;
+	}
+	if (!has_rerere_resolution(item->util))
+		return error("no resolution found for %s", path);
+	if (merge(item->util, path, UNRESOLVE_CHECK))
+		return error("cannot revert resolution of %s", path);
+	return 0;
+}
+
+int unresolve_rerere_1(struct string_list *rr, struct string_list *rr_done,
+		       const char *path)
+{
+	struct string_list_item *item = string_list_lookup(path, rr_done);
+
+	if (!item)
+		return 0;
+	if (merge(item->util, path, UNRESOLVE))
+		return -1;
+
+	if (record_preimage(rr, path, 1))
+		return error("no conflict markers found: %s", path);
+
+	item->util = NULL;
+	unlink_or_warn(rerere_path(item->util, "postimage"));
+	return 0;
+}
+
+int unresolve_rerere(struct string_list *rr, struct string_list *rr_done,
+		     const char **pathspec, int fd)
+{
+	int i, ret = 0;
+
+	if (!pathspec)
+		return error("unresolve what?");
+
+	for (i = 0; pathspec[i]; i++) {
+		ret |= unresolve_check(rr_done, pathspec[i]);
+	}
+	if (ret)
+		return ret;
+
+	for (i = 0; !ret && pathspec[i]; i++) {
+		ret = unresolve_rerere_1(rr, rr_done, pathspec[i]);
+	}
+	ret |= write_rr(rr, rr_done, fd);
+	return ret;
+}
diff --git a/rerere.h b/rerere.h
index 9bb2f13..a0fa50f 100644
--- a/rerere.h
+++ b/rerere.h
@@ -7,5 +7,7 @@ extern int setup_rerere(struct string_list *, struct string_list *);
 extern int rerere(void);
 extern const char *rerere_path(const char *hex, const char *file);
 extern int has_rerere_resolution(const char *hex);
+extern int unresolve_rerere(struct string_list *, struct string_list *,
+	const char **, int);
 
 #endif
-- 
1.6.5.2.182.ge039a

Re: [PATCH/RFC 0/3] git rerere unresolve file

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> ... But 
> there is no other way to remove an incorrect resolution short of purging the 
> whole rr-cache.

No, no no, no.  You do not have to.

As long as you can find out the <preimage,postimage> pair, iow, the
subdirectory used by .git/rr-cache/, you can remove only "postimage" from
there and redo your merge.  "ls -t1 .git/rr-cache/*/thisimage | head"
would be one way to manually find out which one it is.

For example, let's take this merge as an example.

commit 697eb20061dfa00838df32ac24c414dfb4a1f920
Merge: bba637b 46b77a6
Author: Junio C Hamano <gitster@pobox.com>

    Merge branch 'jk/1.7.0-status' into jch

You may not have rerere entries for this, so follow me along for a while.

 $ git checkout bba637b
 $ git merge 46b77a6

This will have a conflict.  We are priming your rerere database, so we
cheat (notice the last dot ".").

 $ git checkout 697eb200 -- .
 $ git rerere

Now reset it away, and redo the merge

 $ git reset --hard
 $ git checkout bba637b
 $ git merge 46b77a6

It will autoresolve the way I resolved in 697eb200

Suppose you do not like the resolution; you can do:

    $ git checkout --conflict=merge Documentation/git-commit.txt
    $ git rerere

to force updating "thisimage".  The newest one is the entry you are
looking for.

    $ ls -1t .git/rr-cache/*/thisimage | head -n 1
    .git/rr-cache/02aac459b0c777f92a8ca6f1e449e6760d366c20/thisimage

I can remove "postimage" from the directory, recreate conflict again,
run rerere to remember it anew, resolve and have rerere remember the new
resolution.

A tool support to compute the conflict hash would be part of a nice
solution.

    $ git checkout --conflict=merge Documentation/git-commit.txt
    $ git rerere hash
    02aac459b0c777f92a8ca6f1e449e6760d366c20 Documentation/git-commit.txt    

Instead of giving "hash" subcommand and having the user to remove
postimage from the directory, the tool can remove it, of course.

    $ git checkout --conflict=merge Documentation/git-commit.txt
    $ git rerere forget Documentation/git-commit.txt
    02aac459b0c777f92a8ca6f1e449e6760d366c20 Documentation/git-commit.txt    
    $ edit Documentation/git-commit.txt ;# come up with a better resolution 
    $ git rerere ;# record it

The attached are rough sketch of such "hash/forget" subcommands.

I however think the best user experience would go like this:

 * Run "git merge"; rerere replays an earlier resolution.

   $ git merge ...

   User decides that it is not a desirable one.

 * User fixes it and creates a better resolution in the work tree.  The
   user may fix-up the autoresolution or the user may first do "checkout
   --conflict=merge" and resolves the conflict from scratch.

   It does not matter how the updated resolution is prepared in the work
   tree.

 * Then the user tells rerere that "this is the corrected resolution",
   perhaps

   $ git rerere update Documentation/git-commit.txt

   This will

   - Internally recompute the original conflicted state, i.e. run
     "checkout --conflict=merge Documentation/git-commit.txt" in-core;

   - feed it to rerere.c::handle_file() to learn the conflict hash;

   - read the user's updated resolution from the work tree, and update
     "postimage" with it.

There is no need to change MERGE_RR at all if you did it the way outlined
in the above.

My "hash" would be more-or-less a useless command.  It still might be
useful as a plumbing command, but it is mostly for debugging.

The "forget" subcommand may be useful, but the real implementation should
be the first half of the "update", iow, recreate conflict in-core in order
to compute the conflict hash, and drop existing "postimage", without
replacing it from the work tree.  We should leave it up to the user using
"checkout --conflict" to reproduce the conflict in the work tree.

 builtin-rerere.c |    4 +++
 rerere.c         |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rerere.h         |    2 +
 3 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 343d6cd..8865313 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -108,6 +108,10 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 
 	if (!strcmp(argv[1], "-h"))
 		usage(git_rerere_usage);
+	else if (!strcmp(argv[1], "hash"))
+		return rerere_report_hash();
+	else if (!strcmp(argv[1], "forget"))
+		return rerere_forget(argv + 2);
 
 	fd = setup_rerere(&merge_rr);
 	if (fd < 0)
diff --git a/rerere.c b/rerere.c
index 29f95f6..4899219 100644
--- a/rerere.c
+++ b/rerere.c
@@ -392,3 +392,61 @@ int rerere(void)
 		return 0;
 	return do_plain_rerere(&merge_rr, fd);
 }
+
+int rerere_forget(const char **path)
+{
+	struct string_list conflict = { NULL, 0, 0, 1 };
+	int i;
+
+	find_conflict(&conflict);
+
+	/*
+	 * Note note note.  I am being lazy here.  The loop should not
+	 * iterate over "path" like this.
+	 *
+	 * It should instead iterate over conflict entries, using "path"
+	 * as pathspec to filter, so that you can say "I am done with
+	 * all the conflict in Documentation/ area".
+	 */
+	for (i = 0; path[i]; i++) { 
+		unsigned char sha1[20];
+		const char *postimage;
+		int ret;
+
+		if (!string_list_has_string(&conflict, path[i])) {
+			error("No such conflicted path %s\n", path[i]);
+			continue;
+		}
+		ret = handle_file(path[i], sha1, NULL);
+		if (ret < 1) {
+			error("No conflict in work tree %s\n", path[i]);
+			continue;
+		}
+		postimage = rerere_path(sha1_to_hex(sha1), "postimage");
+		if (!unlink(postimage))
+			fprintf(stderr, "forgot resolution for %s\n", path[i]);
+		else if (errno == ENOENT)
+			error("no remembered resolution for %s", path[i]);
+		else
+			error("cannot unlink %s: %s", postimage, strerror(errno));
+	}
+	return 0;
+}
+
+int rerere_report_hash(void)
+{
+	struct string_list conflict = { NULL, 0, 0, 1 };
+	int i;
+
+	find_conflict(&conflict);
+	for (i = 0; i < conflict.nr; i++) {
+		const char *path = conflict.items[i].string;
+		unsigned char sha1[20];
+		int ret;
+		ret = handle_file(path, sha1, NULL);
+		if (ret < 1)
+			continue;
+		printf("%s %s\n", sha1_to_hex(sha1), path);
+	}
+	return 0;
+}
diff --git a/rerere.h b/rerere.h
index 13313f3..a10dea9 100644
--- a/rerere.h
+++ b/rerere.h
@@ -7,5 +7,7 @@ extern int setup_rerere(struct string_list *);
 extern int rerere(void);
 extern const char *rerere_path(const char *hex, const char *file);
 extern int has_rerere_resolution(const char *hex);
+extern int rerere_report_hash(void);
+extern int rerere_forget(const char **path);
 
 #endif

Re: [PATCH/RFC 0/3] git rerere unresolve file

[Johannes Sixt]

On Sonntag, 22. November 2009, Junio C Hamano wrote:
> Johannes Sixt <j6t@kdbg.org> writes:
> > ... But
> > there is no other way to remove an incorrect resolution short of purging
> > the whole rr-cache.
>
> No, no no, no.  You do not have to.

Oh, yeah, I know. But even for people who know how to drive plumbing commands, 
this:

> "ls -t1 .git/rr-cache/*/thisimage | head"
> would be one way to manually find out which one it is.

does not count as "you can" in my book. ;) It assumes that there are at most a 
handful conflicted files. In my case, for example, I have to fix a merge 
where there are ~100 conflicted files.

>  * Then the user tells rerere that "this is the corrected resolution",
>    perhaps
>
>    $ git rerere update Documentation/git-commit.txt
>
>    This will
>
>    - Internally recompute the original conflicted state, i.e. run
>      "checkout --conflict=merge Documentation/git-commit.txt" in-core;
>
>    - feed it to rerere.c::handle_file() to learn the conflict hash;
>
>    - read the user's updated resolution from the work tree, and update
>      "postimage" with it.
>
> ...
>
> The "forget" subcommand may be useful, but the real implementation should
> be the first half of the "update", iow, recreate conflict in-core in order
> to compute the conflict hash, and drop existing "postimage", without
> replacing it from the work tree.  We should leave it up to the user using
> "checkout --conflict" to reproduce the conflict in the work tree.

Thank you for your elaborate feedback and the guidelines about how to move 
forward.

I actually think that 'forget' should be the primary subcommand. Because after 
the postimage was purged, the next implicit or explicit 'git rerere' will 
record the resolution anyway. I'll see what I can do.

-- Hannes

Re: [PATCH/RFC 0/3] git rerere unresolve file

[Nanako Shiraishi]

Quoting Johannes Sixt <j6t@kdbg.org>

> On Sonntag, 22. November 2009, Junio C Hamano wrote:
> [snip]
>>  * Then the user tells rerere that "this is the corrected resolution",
>>    perhaps
>>
>>    $ git rerere update Documentation/git-commit.txt
>>
>>    This will
>>
>>    - Internally recompute the original conflicted state, i.e. run
>>      "checkout --conflict=merge Documentation/git-commit.txt" in-core;
>>
>>    - feed it to rerere.c::handle_file() to learn the conflict hash;
>>
>>    - read the user's updated resolution from the work tree, and update
>>      "postimage" with it.
>>
>> ...
>>
>> The "forget" subcommand may be useful, but the real implementation should
>> be the first half of the "update", iow, recreate conflict in-core in order
>> to compute the conflict hash, and drop existing "postimage", without
>> replacing it from the work tree.  We should leave it up to the user using
>> "checkout --conflict" to reproduce the conflict in the work tree.
>
> Thank you for your elaborate feedback and the guidelines about how to move 
> forward.
>
> I actually think that 'forget' should be the primary subcommand. Because after 
> the postimage was purged, the next implicit or explicit 'git rerere' will 
> record the resolution anyway. I'll see what I can do.
>
> -- Hannes

I think 'forget' should be the primary interface to this new feature too. If I mistakenly recorded an incorrect merge in the past and I'm trying to do a better job this time, I wouldn't be confident enough to be able to say 'update' to mark the attempt I made this time, and it may take more than one attempts for me to reach a good result. Running 'forget' once will let me retry until I run 'rerere' again.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

[PATCH 0/9] Undoing conflict resolution

[Junio C Hamano]

This series resurrects the topic we discussed later November.

I wrote a bit about this in http://gitster.livejournal.com/43665.html but
as usual with my patches, they do come with minimum set of tests but it
contains no documentation updates.  The user visible changes are:

 - ls-files has a new option primarily for debugging;
 - update-index has a new option --clear-resolve-undo;
 - rerere has a new subcommand forget.

Junio C Hamano (9):
  builtin-merge.c: use standard active_cache macros
  resolve-undo: record resolved conflicts in a new index extension
    section
  resolve-undo: basic tests
  resolve-undo: allow plumbing to clear the information
  resolve-undo: "checkout -m path" uses resolve-undo information
  resolve-undo: teach "update-index --unresolve" to use resolve-undo
    info
  rerere: remove silly 1024-byte line limit
  rerere: refactor rerere logic to make it independent from I/O
  rerere forget path: forget recorded resolution

 Makefile                  |    2 +
 builtin-checkout.c        |    6 +
 builtin-ls-files.c        |   43 +++++++-
 builtin-merge.c           |    9 +-
 builtin-read-tree.c       |    2 +
 builtin-rerere.c          |    2 +
 builtin-update-index.c    |   18 +++-
 cache.h                   |    4 +
 read-cache.c              |   18 +++
 rerere.c                  |  259 +++++++++++++++++++++++++++++++++++++-------
 rerere.h                  |    1 +
 resolve-undo.c            |  176 ++++++++++++++++++++++++++++++
 resolve-undo.h            |   16 +++
 t/t2030-unresolve-info.sh |  140 ++++++++++++++++++++++++
 14 files changed, 648 insertions(+), 48 deletions(-)
 create mode 100644 resolve-undo.c
 create mode 100644 resolve-undo.h
 create mode 100755 t/t2030-unresolve-info.sh

[PATCH 1/9] builtin-merge.c: use standard active_cache macros

[Junio C Hamano]

Instead of using the low-level index_state interface, use the bog standard
active_cache and active_nr macros to access the cache entries when using the
default one.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index f1c84d7..6cb804b 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -618,11 +618,10 @@ static void count_diff_files(struct diff_queue_struct *q,
 
 static int count_unmerged_entries(void)
 {
-	const struct index_state *state = &the_index;
 	int i, ret = 0;
 
-	for (i = 0; i < state->cache_nr; i++)
-		if (ce_stage(state->cache[i]))
+	for (i = 0; i < active_nr; i++)
+		if (ce_stage(active_cache[i]))
 			ret++;
 
 	return ret;
-- 
1.6.6

[PATCH 2/9] resolve-undo: record resolved conflicts in a new index extension section

[Junio C Hamano]

When resolving a conflict using "git add" to create a stage #0 entry, or
"git rm" to remove entries at higher stages, remove_index_entry_at()
function is eventually called to remove unmerged (i.e. higher stage)
entries from the index.  Introduce a "resolve_undo_info" structure and
keep track of the removed cache entries, and save it in a new index
extension section in the index_state.

Operations like "read-tree -m", "merge", "checkout [-m] <branch>" and
"reset" are signs that recorded information in the index is no longer
necessary.  The data is removed from the index extension when operations
start; they may leave conflicted entries in the index, and later user
actions like "git add" will record their conflicted states afresh.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile            |    2 +
 builtin-checkout.c  |    2 +
 builtin-merge.c     |    4 +-
 builtin-read-tree.c |    2 +
 cache.h             |    2 +
 read-cache.c        |   18 ++++++++
 resolve-undo.c      |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++
 resolve-undo.h      |   14 ++++++
 8 files changed, 160 insertions(+), 1 deletions(-)
 create mode 100644 resolve-undo.c
 create mode 100644 resolve-undo.h

diff --git a/Makefile b/Makefile
index 4a1e5bc..762898a 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,7 @@ LIB_H += reflog-walk.h
 LIB_H += refs.h
 LIB_H += remote.h
 LIB_H += rerere.h
+LIB_H += resolve-undo.h
 LIB_H += revision.h
 LIB_H += run-command.h
 LIB_H += sha1-lookup.h
@@ -578,6 +579,7 @@ LIB_OBJS += refs.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
 LIB_OBJS += rerere.o
+LIB_OBJS += resolve-undo.o
 LIB_OBJS += revision.o
 LIB_OBJS += run-command.o
 LIB_OBJS += server-info.o
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..a0fe7a4 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -17,6 +17,7 @@
 #include "blob.h"
 #include "xdiff-interface.h"
 #include "ll-merge.h"
+#include "resolve-undo.h"
 
 static const char * const checkout_usage[] = {
 	"git checkout [options] <branch>",
@@ -370,6 +371,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 	if (read_cache_preload(NULL) < 0)
 		return error("corrupt index file");
 
+	resolve_undo_clear();
 	if (opts->force) {
 		ret = reset_tree(new->commit->tree, opts, 1);
 		if (ret)
diff --git a/builtin-merge.c b/builtin-merge.c
index 6cb804b..6bc2f7a 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -24,6 +24,7 @@
 #include "rerere.h"
 #include "help.h"
 #include "merge-recursive.h"
+#include "resolve-undo.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -604,6 +605,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 		discard_cache();
 		if (read_cache() < 0)
 			die("failed to read the cache");
+		resolve_undo_clear();
 		return ret;
 	}
 }
@@ -851,7 +853,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (read_cache_unmerged())
 		die("You are in the middle of a conflicted merge."
 				" (index unmerged)");
-
+	resolve_undo_clear();
 	/*
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
 	 * current branch.
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 2a3a32c..7d378b7 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -13,6 +13,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "parse-options.h"
+#include "resolve-undo.h"
 
 static int nr_trees;
 static struct tree *trees[MAX_UNPACK_TREES];
@@ -122,6 +123,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			die("You need to resolve your current index first");
 		stage = opts.merge = 1;
 	}
+	resolve_undo_clear();
 
 	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/cache.h b/cache.h
index bf468e5..310d9e6 100644
--- a/cache.h
+++ b/cache.h
@@ -282,6 +282,7 @@ static inline int ce_to_dtype(const struct cache_entry *ce)
 struct index_state {
 	struct cache_entry **cache;
 	unsigned int cache_nr, cache_alloc, cache_changed;
+	struct string_list *resolve_undo;
 	struct cache_tree *cache_tree;
 	struct cache_time timestamp;
 	void *alloc;
@@ -336,6 +337,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
 #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
 #define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
+#define resolve_undo_clear() resolve_undo_clear_index(&the_index)
 #endif
 
 enum object_type {
diff --git a/read-cache.c b/read-cache.c
index 1bbaf1c..9e0fb04 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -14,6 +14,7 @@
 #include "diffcore.h"
 #include "revision.h"
 #include "blob.h"
+#include "resolve-undo.h"
 
 /* Index extensions.
  *
@@ -26,6 +27,7 @@
 
 #define CACHE_EXT(s) ( (s[0]<<24)|(s[1]<<16)|(s[2]<<8)|(s[3]) )
 #define CACHE_EXT_TREE 0x54524545	/* "TREE" */
+#define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* "REUN" */
 
 struct index_state the_index;
 
@@ -449,6 +451,7 @@ int remove_index_entry_at(struct index_state *istate, int pos)
 {
 	struct cache_entry *ce = istate->cache[pos];
 
+	record_resolve_undo(istate, ce);
 	remove_name_hash(ce);
 	istate->cache_changed = 1;
 	istate->cache_nr--;
@@ -1170,6 +1173,9 @@ static int read_index_extension(struct index_state *istate,
 	case CACHE_EXT_TREE:
 		istate->cache_tree = cache_tree_read(data, sz);
 		break;
+	case CACHE_EXT_RESOLVE_UNDO:
+		istate->resolve_undo = resolve_undo_read(data, sz);
+		break;
 	default:
 		if (*ext < 'A' || 'Z' < *ext)
 			return error("index uses %.4s extension, which we do not understand",
@@ -1349,6 +1355,7 @@ int is_index_unborn(struct index_state *istate)
 
 int discard_index(struct index_state *istate)
 {
+	resolve_undo_clear_index(istate);
 	istate->cache_nr = 0;
 	istate->cache_changed = 0;
 	istate->timestamp.sec = 0;
@@ -1574,6 +1581,17 @@ int write_index(struct index_state *istate, int newfd)
 		if (err)
 			return -1;
 	}
+	if (istate->resolve_undo) {
+		struct strbuf sb = STRBUF_INIT;
+
+		resolve_undo_write(&sb, istate->resolve_undo);
+		err = write_index_ext_header(&c, newfd, CACHE_EXT_RESOLVE_UNDO,
+					     sb.len) < 0
+			|| ce_write(&c, newfd, sb.buf, sb.len) < 0;
+		strbuf_release(&sb);
+		if (err)
+			return -1;
+	}
 
 	if (ce_flush(&c, newfd) || fstat(newfd, &st))
 		return -1;
diff --git a/resolve-undo.c b/resolve-undo.c
new file mode 100644
index 0000000..86e8547
--- /dev/null
+++ b/resolve-undo.c
@@ -0,0 +1,117 @@
+#include "cache.h"
+#include "resolve-undo.h"
+#include "string-list.h"
+
+/* The only error case is to run out of memory in string-list */
+void record_resolve_undo(struct index_state *istate, struct cache_entry *ce)
+{
+	struct string_list_item *lost;
+	struct resolve_undo_info *ui;
+	struct string_list *resolve_undo;
+	int stage = ce_stage(ce);
+
+	if (!stage)
+		return;
+
+	if (!istate->resolve_undo) {
+		resolve_undo = xcalloc(1, sizeof(*resolve_undo));
+		resolve_undo->strdup_strings = 1;
+		istate->resolve_undo = resolve_undo;
+	}
+	resolve_undo = istate->resolve_undo;
+	lost = string_list_insert(ce->name, resolve_undo);
+	if (!lost->util)
+		lost->util = xcalloc(1, sizeof(*ui));
+	ui = lost->util;
+	hashcpy(ui->sha1[stage - 1], ce->sha1);
+	ui->mode[stage - 1] = ce->ce_mode;
+}
+
+static int write_one(struct string_list_item *item, void *cbdata)
+{
+	struct strbuf *sb = cbdata;
+	struct resolve_undo_info *ui = item->util;
+	int i;
+
+	if (!ui)
+		return 0;
+	strbuf_addstr(sb, item->string);
+	strbuf_addch(sb, 0);
+	for (i = 0; i < 3; i++)
+		strbuf_addf(sb, "%o%c", ui->mode[i], 0);
+	for (i = 0; i < 3; i++) {
+		if (!ui->mode[i])
+			continue;
+		strbuf_add(sb, ui->sha1[i], 20);
+	}
+	return 0;
+}
+
+void resolve_undo_write(struct strbuf *sb, struct string_list *resolve_undo)
+{
+	for_each_string_list(write_one, resolve_undo, sb);
+}
+
+struct string_list *resolve_undo_read(void *data, unsigned long size)
+{
+	struct string_list *resolve_undo;
+	size_t len;
+	char *endptr;
+	int i;
+
+	resolve_undo = xcalloc(1, sizeof(*resolve_undo));
+	resolve_undo->strdup_strings = 1;
+
+	while (size) {
+		struct string_list_item *lost;
+		struct resolve_undo_info *ui;
+
+		len = strlen(data) + 1;
+		if (size <= len)
+			goto error;
+		lost = string_list_insert(data, resolve_undo);
+		if (!lost->util)
+			lost->util = xcalloc(1, sizeof(*ui));
+		ui = lost->util;
+		size -= len;
+		data += len;
+
+		for (i = 0; i < 3; i++) {
+			ui->mode[i] = strtoul(data, &endptr, 8);
+			if (!endptr || endptr == data || *endptr)
+				goto error;
+			len = (endptr + 1) - (char*)data;
+			if (size <= len)
+				goto error;
+			size -= len;
+			data += len;
+		}
+
+		for (i = 0; i < 3; i++) {
+			if (!ui->mode[i])
+				continue;
+			if (size < 20)
+				goto error;
+			hashcpy(ui->sha1[i], data);
+			size -= 20;
+			data += 20;
+		}
+	}
+	return resolve_undo;
+
+error:
+	string_list_clear(resolve_undo, 1);
+	error("Index records invalid resolve-undo information");
+	return NULL;
+}
+
+void resolve_undo_clear_index(struct index_state *istate)
+{
+	struct string_list *resolve_undo = istate->resolve_undo;
+	if (!resolve_undo)
+		return;
+	string_list_clear(resolve_undo, 1);
+	free(resolve_undo);
+	istate->resolve_undo = NULL;
+	istate->cache_changed = 1;
+}
diff --git a/resolve-undo.h b/resolve-undo.h
new file mode 100644
index 0000000..74194d0
--- /dev/null
+++ b/resolve-undo.h
@@ -0,0 +1,14 @@
+#ifndef RESOLVE_UNDO_H
+#define RESOLVE_UNDO_H
+
+struct resolve_undo_info {
+	unsigned int mode[3];
+	unsigned char sha1[3][20];
+};
+
+extern void record_resolve_undo(struct index_state *, struct cache_entry *);
+extern void resolve_undo_write(struct strbuf *, struct string_list *);
+extern struct string_list *resolve_undo_read(void *, unsigned long);
+extern void resolve_undo_clear_index(struct index_state *);
+
+#endif
-- 
1.6.6

[PATCH 3/9] resolve-undo: basic tests

[Junio C Hamano]

Make sure that resolving a failed merge with git add records
the conflicted state, committing the result keeps that state,
and checking out another commit clears the state.

"git ls-files" learns a new option --resolve-undo to show the
recorded information.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-ls-files.c        |   43 +++++++++++++++++++++-
 t/t2030-unresolve-info.sh |   88 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 1 deletions(-)
 create mode 100755 t/t2030-unresolve-info.sh

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index c9a03e5..ef3a068 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -11,6 +11,8 @@
 #include "builtin.h"
 #include "tree.h"
 #include "parse-options.h"
+#include "resolve-undo.h"
+#include "string-list.h"
 
 static int abbrev;
 static int show_deleted;
@@ -18,6 +20,7 @@ static int show_cached;
 static int show_others;
 static int show_stage;
 static int show_unmerged;
+static int show_resolve_undo;
 static int show_modified;
 static int show_killed;
 static int show_valid_bit;
@@ -37,6 +40,7 @@ static const char *tag_removed = "";
 static const char *tag_other = "";
 static const char *tag_killed = "";
 static const char *tag_modified = "";
+static const char *tag_resolve_undo = "";
 
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
 {
@@ -155,6 +159,38 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	write_name_quoted(ce->name + offset, stdout, line_terminator);
 }
 
+static int show_one_ru(struct string_list_item *item, void *cbdata)
+{
+	int offset = prefix_offset;
+	const char *path = item->string;
+	struct resolve_undo_info *ui = item->util;
+	int i, len;
+
+	len = strlen(path);
+	if (len < prefix_len)
+		return 0; /* outside of the prefix */
+	if (!match_pathspec(pathspec, path, len, prefix_len, ps_matched))
+		return 0; /* uninterested */
+	for (i = 0; i < 3; i++) {
+		if (!ui->mode[i])
+			continue;
+		printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i],
+		       abbrev
+		       ? find_unique_abbrev(ui->sha1[i], abbrev)
+		       : sha1_to_hex(ui->sha1[i]),
+		       i + 1);
+		write_name_quoted(path + offset, stdout, line_terminator);
+	}
+	return 0;
+}
+
+static void show_ru_info(const char *prefix)
+{
+	if (!the_index.resolve_undo)
+		return;
+	for_each_string_list(show_one_ru, the_index.resolve_undo, NULL);
+}
+
 static void show_files(struct dir_struct *dir, const char *prefix)
 {
 	int i;
@@ -454,6 +490,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			DIR_HIDE_EMPTY_DIRECTORIES),
 		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
 			"show unmerged files in the output"),
+		OPT_BOOLEAN(0, "resolve-undo", &show_resolve_undo,
+			    "show resolve-undo information"),
 		{ OPTION_CALLBACK, 'x', "exclude", &dir.exclude_list[EXC_CMDL], "pattern",
 			"skip files matching pattern",
 			0, option_parse_exclude },
@@ -490,6 +528,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		tag_modified = "C ";
 		tag_other = "? ";
 		tag_killed = "K ";
+		tag_resolve_undo = "U ";
 	}
 	if (show_modified || show_others || show_deleted || (dir.flags & DIR_SHOW_IGNORED) || show_killed)
 		require_work_tree = 1;
@@ -529,7 +568,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 
 	/* With no flags, we default to showing the cached files */
 	if (!(show_stage | show_deleted | show_others | show_unmerged |
-	      show_killed | show_modified))
+	      show_killed | show_modified | show_resolve_undo))
 		show_cached = 1;
 
 	if (prefix)
@@ -544,6 +583,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		overlay_tree_on_cache(with_tree, prefix);
 	}
 	show_files(&dir, prefix);
+	if (show_resolve_undo)
+		show_ru_info(prefix);
 
 	if (ps_matched) {
 		int bad;
diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh
new file mode 100755
index 0000000..785c8b3
--- /dev/null
+++ b/t/t2030-unresolve-info.sh
@@ -0,0 +1,88 @@
+#!/bin/sh
+
+test_description='undoing resolution'
+
+. ./test-lib.sh
+
+check_resolve_undo () {
+	msg=$1
+	shift
+	while case $# in
+	0)	break ;;
+	1|2|3)	die "Bug in check-resolve-undo test" ;;
+	esac
+	do
+		path=$1
+		shift
+		for stage in 1 2 3
+		do
+			sha1=$1
+			shift
+			case "$sha1" in
+			'') continue ;;
+			esac
+			sha1=$(git rev-parse --verify "$sha1")
+			printf "100644 %s %s\t%s\n" $sha1 $stage $path
+		done
+	done >"$msg.expect" &&
+	git ls-files --resolve-undo >"$msg.actual" &&
+	test_cmp "$msg.expect" "$msg.actual"
+}
+
+prime_resolve_undo () {
+	git reset --hard &&
+	git checkout second^0 &&
+	test_tick &&
+	test_must_fail git merge third^0 &&
+	echo merge does not leave anything &&
+	check_resolve_undo empty &&
+	echo different >file &&
+	git add file &&
+	echo resolving records &&
+	check_resolve_undo recorded file initial:file second:file third:file
+}
+
+test_expect_success setup '
+	test_commit initial file first &&
+	git branch side &&
+	git branch another &&
+	test_commit second file second &&
+	git checkout side &&
+	test_commit third file third &&
+	git checkout another &&
+	test_commit fourth file fourth &&
+	git checkout master
+'
+
+test_expect_success 'add records switch clears' '
+	prime_resolve_undo &&
+	test_tick &&
+	git commit -m merged &&
+	echo committing keeps &&
+	check_resolve_undo kept file initial:file second:file third:file &&
+	git checkout second^0 &&
+	echo switching clears &&
+	check_resolve_undo cleared
+'
+
+test_expect_success 'rm records reset clears' '
+	prime_resolve_undo &&
+	test_tick &&
+	git commit -m merged &&
+	echo committing keeps &&
+	check_resolve_undo kept file initial:file second:file third:file &&
+
+	echo merge clears upfront &&
+	test_must_fail git merge fourth^0 &&
+	check_resolve_undo nuked &&
+
+	git rm -f file &&
+	echo resolving records &&
+	check_resolve_undo recorded file initial:file HEAD:file fourth:file &&
+
+	git reset --hard &&
+	echo resetting discards &&
+	check_resolve_undo discarded
+'
+
+test_done
-- 
1.6.6

[PATCH 4/9] resolve-undo: allow plumbing to clear the information

[Junio C Hamano]

At the Porcelain level, operations such as merge that populate an
initially cleanly merged index with conflicted entries clear the
resolve-undo information upfront.  Give scripted Porcelains a way
to do the same, by implementing "update-index --clear-resolve-info".

With this, a scripted Porcelain may "update-index --clear-resolve-info"
first and repeatedly run "update-index --cacheinfo" to stuff unmerged
entries to the index, to be resolved by the user with "git add" and
stuff.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-update-index.c    |    5 +++++
 t/t2030-unresolve-info.sh |   12 ++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/builtin-update-index.c b/builtin-update-index.c
index a6b7f2d..a19e786 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -9,6 +9,7 @@
 #include "tree-walk.h"
 #include "builtin.h"
 #include "refs.h"
+#include "resolve-undo.h"
 
 /*
  * Default to not allowing changes to the list of files. The
@@ -703,6 +704,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 				verbose = 1;
 				continue;
 			}
+			if (!strcmp(path, "--clear-resolve-undo")) {
+				resolve_undo_clear();
+				continue;
+			}
 			if (!strcmp(path, "-h") || !strcmp(path, "--help"))
 				usage(update_index_usage);
 			die("unknown option %s", path);
diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh
index 785c8b3..9844802 100755
--- a/t/t2030-unresolve-info.sh
+++ b/t/t2030-unresolve-info.sh
@@ -85,4 +85,16 @@ test_expect_success 'rm records reset clears' '
 	check_resolve_undo discarded
 '
 
+test_expect_success 'plumbing clears' '
+	prime_resolve_undo &&
+	test_tick &&
+	git commit -m merged &&
+	echo committing keeps &&
+	check_resolve_undo kept file initial:file second:file third:file &&
+
+	echo plumbing clear &&
+	git update-index --clear-resolve-undo &&
+	check_resolve_undo cleared
+'
+
 test_done
-- 
1.6.6

[PATCH 5/9] resolve-undo: "checkout -m path" uses resolve-undo information

[Junio C Hamano]

Once you resolved conflicts by "git add path", you cannot recreate the
conflicted state with "git checkout -m path", because you lost information
from higher stages in the index when you resolved them.

Since we record the necessary information in the resolve-undo index
extension these days, we can reproduce the unmerged state in the index and
check it out.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c        |    4 +++
 cache.h                   |    1 +
 resolve-undo.c            |   59 +++++++++++++++++++++++++++++++++++++++++++++
 resolve-undo.h            |    2 +
 t/t2030-unresolve-info.sh |   11 ++++++++
 5 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index a0fe7a4..bdef1aa 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -235,6 +235,10 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	if (report_path_error(ps_matched, pathspec, 0))
 		return 1;
 
+	/* "checkout -m path" to recreate conflicted state */
+	if (opts->merge)
+		unmerge_cache(pathspec);
+
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
diff --git a/cache.h b/cache.h
index 310d9e6..f479f09 100644
--- a/cache.h
+++ b/cache.h
@@ -338,6 +338,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
 #define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
 #define resolve_undo_clear() resolve_undo_clear_index(&the_index)
+#define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
 #endif
 
 enum object_type {
diff --git a/resolve-undo.c b/resolve-undo.c
index 86e8547..37d73cd 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "dir.h"
 #include "resolve-undo.h"
 #include "string-list.h"
 
@@ -115,3 +116,61 @@ void resolve_undo_clear_index(struct index_state *istate)
 	istate->resolve_undo = NULL;
 	istate->cache_changed = 1;
 }
+
+int unmerge_index_entry_at(struct index_state *istate, int pos)
+{
+	struct cache_entry *ce;
+	struct string_list_item *item;
+	struct resolve_undo_info *ru;
+	int i, err = 0;
+
+	if (!istate->resolve_undo)
+		return pos;
+
+	ce = istate->cache[pos];
+	if (ce_stage(ce)) {
+		/* already unmerged */
+		while ((pos < istate->cache_nr) &&
+		       ! strcmp(istate->cache[pos]->name, ce->name))
+			pos++;
+		return pos - 1; /* return the last entry processed */
+	}
+	item = string_list_lookup(ce->name, istate->resolve_undo);
+	if (!item)
+		return pos;
+	ru = item->util;
+	if (!ru)
+		return pos;
+	remove_index_entry_at(istate, pos);
+	for (i = 0; i < 3; i++) {
+		struct cache_entry *nce;
+		if (!ru->mode[i])
+			continue;
+		nce = make_cache_entry(ru->mode[i], ru->sha1[i],
+				       ce->name, i + 1, 0);
+		if (add_index_entry(istate, nce, ADD_CACHE_OK_TO_ADD)) {
+			err = 1;
+			error("cannot unmerge '%s'", ce->name);
+		}
+	}
+	if (err)
+		return pos;
+	free(ru);
+	item->util = NULL;
+	return unmerge_index_entry_at(istate, pos);
+}
+
+void unmerge_index(struct index_state *istate, const char **pathspec)
+{
+	int i;
+
+	if (!istate->resolve_undo)
+		return;
+
+	for (i = 0; i < istate->cache_nr; i++) {
+		struct cache_entry *ce = istate->cache[i];
+		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL))
+			continue;
+		i = unmerge_index_entry_at(istate, i);
+	}
+}
diff --git a/resolve-undo.h b/resolve-undo.h
index 74194d0..e4e5c1b 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -10,5 +10,7 @@ extern void record_resolve_undo(struct index_state *, struct cache_entry *);
 extern void resolve_undo_write(struct strbuf *, struct string_list *);
 extern struct string_list *resolve_undo_read(void *, unsigned long);
 extern void resolve_undo_clear_index(struct index_state *);
+extern int unmerge_index_entry_at(struct index_state *, int);
+extern void unmerge_index(struct index_state *, const char **);
 
 #endif
diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh
index 9844802..ea65f39 100755
--- a/t/t2030-unresolve-info.sh
+++ b/t/t2030-unresolve-info.sh
@@ -97,4 +97,15 @@ test_expect_success 'plumbing clears' '
 	check_resolve_undo cleared
 '
 
+test_expect_success 'add records checkout -m undoes' '
+	prime_resolve_undo &&
+	git diff HEAD &&
+	git checkout --conflict=merge file &&
+	echo checkout used the record and removed it &&
+	check_resolve_undo removed &&
+	echo the index and the work tree is unmerged again &&
+	git diff >actual &&
+	grep "^++<<<<<<<" actual
+'
+
 test_done
-- 
1.6.6

[PATCH 6/9] resolve-undo: teach "update-index --unresolve" to use resolve-undo info

[Junio C Hamano]

The update-index plumbing command had a hacky --unresolve implementation
that was written back in the days when merge was the only way for users to
end up with higher stages in the index, and assumed that stage #2 must
have come from HEAD, stage #3 from MERGE_HEAD and didn't bother to compute
the stage #1 information.

There were several issues with this approach:

 - These days, merge is not the only command, and conflicts coming from
   commands like cherry-pick, "am -3", etc. cannot be recreated by looking
   at MERGE_HEAD;

 - For a conflict that came from a merge that had renames, picking up the
   same path from MERGE_HEAD and HEAD wouldn't help recreating it, either;

 - It may have been Ok not to recreate stage #1 back when it was written,
   because "diff --ours/--theirs" were the only availble ways to review
   conflicts and they don't need stage #1 information.  "diff --cc" that
   was invented much later is a lot more useful way but it needs stage #1.

We can use resolve-undo information recorded in the index extension to
solve all of these issues.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-update-index.c    |   13 ++++++++++++-
 cache.h                   |    1 +
 t/t2030-unresolve-info.sh |    7 +++++++
 3 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/builtin-update-index.c b/builtin-update-index.c
index a19e786..750db16 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -433,7 +433,18 @@ static int unresolve_one(const char *path)
 
 	/* See if there is such entry in the index. */
 	pos = cache_name_pos(path, namelen);
-	if (pos < 0) {
+	if (0 <= pos) {
+		/* already merged */
+		pos = unmerge_cache_entry_at(pos);
+		if (pos < active_nr) {
+			struct cache_entry *ce = active_cache[pos];
+			if (ce_stage(ce) &&
+			    ce_namelen(ce) == namelen &&
+			    !memcmp(ce->name, path, namelen))
+				return 0;
+		}
+		/* no resolve-undo information; fall back */
+	} else {
 		/* If there isn't, either it is unmerged, or
 		 * resolved as "removed" by mistake.  We do not
 		 * want to do anything in the former case.
diff --git a/cache.h b/cache.h
index f479f09..97b4a74 100644
--- a/cache.h
+++ b/cache.h
@@ -338,6 +338,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
 #define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
 #define resolve_undo_clear() resolve_undo_clear_index(&the_index)
+#define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
 #define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
 #endif
 
diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh
index ea65f39..28e2eb1 100755
--- a/t/t2030-unresolve-info.sh
+++ b/t/t2030-unresolve-info.sh
@@ -108,4 +108,11 @@ test_expect_success 'add records checkout -m undoes' '
 	grep "^++<<<<<<<" actual
 '
 
+test_expect_success 'unmerge with plumbing' '
+	prime_resolve_undo &&
+	git update-index --unresolve file &&
+	git ls-files -u >actual &&
+	test $(wc -l <actual) = 3
+'
+
 test_done
-- 
1.6.6

[PATCH 7/9] rerere: remove silly 1024-byte line limit

[Junio C Hamano]

Ever since 658f365 (Make git-rerere a builtin, 2006-12-20) rewrote it, it
kept this line-length limit regression, even after we started using strbuf
in the same function in 19b358e (Use strbuf API in buitin-rerere.c,
2007-09-06).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/rerere.c b/rerere.c
index 29f95f6..88bb4f1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -87,12 +87,12 @@ static int handle_file(const char *path,
 	 unsigned char *sha1, const char *output)
 {
 	git_SHA_CTX ctx;
-	char buf[1024];
 	int hunk_no = 0;
 	enum {
 		RR_CONTEXT = 0, RR_SIDE_1, RR_SIDE_2, RR_ORIGINAL,
 	} hunk = RR_CONTEXT;
 	struct strbuf one = STRBUF_INIT, two = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
 	FILE *f = fopen(path, "r");
 	FILE *out = NULL;
 	int wrerror = 0;
@@ -111,20 +111,20 @@ static int handle_file(const char *path,
 	if (sha1)
 		git_SHA1_Init(&ctx);
 
-	while (fgets(buf, sizeof(buf), f)) {
-		if (!prefixcmp(buf, "<<<<<<< ")) {
+	while (!strbuf_getwholeline(&buf, f, '\n')) {
+		if (!prefixcmp(buf.buf, "<<<<<<< ")) {
 			if (hunk != RR_CONTEXT)
 				goto bad;
 			hunk = RR_SIDE_1;
-		} else if (!prefixcmp(buf, "|||||||") && isspace(buf[7])) {
+		} else if (!prefixcmp(buf.buf, "|||||||") && isspace(buf.buf[7])) {
 			if (hunk != RR_SIDE_1)
 				goto bad;
 			hunk = RR_ORIGINAL;
-		} else if (!prefixcmp(buf, "=======") && isspace(buf[7])) {
+		} else if (!prefixcmp(buf.buf, "=======") && isspace(buf.buf[7])) {
 			if (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)
 				goto bad;
 			hunk = RR_SIDE_2;
-		} else if (!prefixcmp(buf, ">>>>>>> ")) {
+		} else if (!prefixcmp(buf.buf, ">>>>>>> ")) {
 			if (hunk != RR_SIDE_2)
 				goto bad;
 			if (strbuf_cmp(&one, &two) > 0)
@@ -147,13 +147,13 @@ static int handle_file(const char *path,
 			strbuf_reset(&one);
 			strbuf_reset(&two);
 		} else if (hunk == RR_SIDE_1)
-			strbuf_addstr(&one, buf);
+			strbuf_addstr(&one, buf.buf);
 		else if (hunk == RR_ORIGINAL)
 			; /* discard */
 		else if (hunk == RR_SIDE_2)
-			strbuf_addstr(&two, buf);
+			strbuf_addstr(&two, buf.buf);
 		else if (out)
-			ferr_puts(buf, out, &wrerror);
+			ferr_puts(buf.buf, out, &wrerror);
 		continue;
 	bad:
 		hunk = 99; /* force error exit */
@@ -161,6 +161,7 @@ static int handle_file(const char *path,
 	}
 	strbuf_release(&one);
 	strbuf_release(&two);
+	strbuf_release(&buf);
 
 	fclose(f);
 	if (wrerror)
-- 
1.6.6

[PATCH 8/9] rerere: refactor rerere logic to make it independent from I/O

[Junio C Hamano]

This splits the handle_file() function into in-core part and I/O
parts of the logic to create the preimage, so that we can compute
the conflict identifier without having to use temporary files.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c |  117 +++++++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 82 insertions(+), 35 deletions(-)

diff --git a/rerere.c b/rerere.c
index 88bb4f1..f013ae7 100644
--- a/rerere.c
+++ b/rerere.c
@@ -83,8 +83,41 @@ static inline void ferr_puts(const char *s, FILE *fp, int *err)
 	ferr_write(s, strlen(s), fp, err);
 }
 
-static int handle_file(const char *path,
-	 unsigned char *sha1, const char *output)
+struct rerere_io {
+	int (*getline)(struct strbuf *, struct rerere_io *);
+	void (*putstr)(const char *, struct rerere_io *);
+	void (*putmem)(const char *, size_t, struct rerere_io *);
+	/* some more stuff */
+};
+
+struct rerere_io_file {
+	struct rerere_io io;
+	FILE *input;
+	FILE *output;
+	int wrerror;
+};
+
+static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)
+{
+	struct rerere_io_file *io = (struct rerere_io_file *)io_;
+	return strbuf_getwholeline(sb, io->input, '\n');
+}
+
+static void rerere_file_putstr(const char *str, struct rerere_io *io_)
+{
+	struct rerere_io_file *io = (struct rerere_io_file *)io_;
+	if (io->output)
+		ferr_puts(str, io->output, &io->wrerror);
+}
+
+static void rerere_file_putmem(const char *mem, size_t sz, struct rerere_io *io_)
+{
+	struct rerere_io_file *io = (struct rerere_io_file *)io_;
+	if (io->output)
+		ferr_write(mem, sz, io->output, &io->wrerror);
+}
+
+static int handle_path(unsigned char *sha1, struct rerere_io *io)
 {
 	git_SHA_CTX ctx;
 	int hunk_no = 0;
@@ -93,25 +126,11 @@ static int handle_file(const char *path,
 	} hunk = RR_CONTEXT;
 	struct strbuf one = STRBUF_INIT, two = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
-	FILE *f = fopen(path, "r");
-	FILE *out = NULL;
-	int wrerror = 0;
-
-	if (!f)
-		return error("Could not open %s", path);
-
-	if (output) {
-		out = fopen(output, "w");
-		if (!out) {
-			fclose(f);
-			return error("Could not write %s", output);
-		}
-	}
 
 	if (sha1)
 		git_SHA1_Init(&ctx);
 
-	while (!strbuf_getwholeline(&buf, f, '\n')) {
+	while (!io->getline(&buf, io)) {
 		if (!prefixcmp(buf.buf, "<<<<<<< ")) {
 			if (hunk != RR_CONTEXT)
 				goto bad;
@@ -131,13 +150,11 @@ static int handle_file(const char *path,
 				strbuf_swap(&one, &two);
 			hunk_no++;
 			hunk = RR_CONTEXT;
-			if (out) {
-				ferr_puts("<<<<<<<\n", out, &wrerror);
-				ferr_write(one.buf, one.len, out, &wrerror);
-				ferr_puts("=======\n", out, &wrerror);
-				ferr_write(two.buf, two.len, out, &wrerror);
-				ferr_puts(">>>>>>>\n", out, &wrerror);
-			}
+			io->putstr("<<<<<<<\n", io);
+			io->putmem(one.buf, one.len, io);
+			io->putstr("=======\n", io);
+			io->putmem(two.buf, two.len, io);
+			io->putstr(">>>>>>>\n", io);
 			if (sha1) {
 				git_SHA1_Update(&ctx, one.buf ? one.buf : "",
 					    one.len + 1);
@@ -152,8 +169,8 @@ static int handle_file(const char *path,
 			; /* discard */
 		else if (hunk == RR_SIDE_2)
 			strbuf_addstr(&two, buf.buf);
-		else if (out)
-			ferr_puts(buf.buf, out, &wrerror);
+		else
+			io->putstr(buf.buf, io);
 		continue;
 	bad:
 		hunk = 99; /* force error exit */
@@ -163,21 +180,51 @@ static int handle_file(const char *path,
 	strbuf_release(&two);
 	strbuf_release(&buf);
 
-	fclose(f);
-	if (wrerror)
-		error("There were errors while writing %s (%s)",
-		      path, strerror(wrerror));
-	if (out && fclose(out))
-		wrerror = error("Failed to flush %s: %s",
-				path, strerror(errno));
 	if (sha1)
 		git_SHA1_Final(sha1, &ctx);
-	if (hunk != RR_CONTEXT) {
+	if (hunk != RR_CONTEXT)
+		return -1;
+	return hunk_no;
+}
+
+static int handle_file(const char *path, unsigned char *sha1, const char *output)
+{
+	int hunk_no = 0;
+	struct rerere_io_file io;
+
+	memset(&io, 0, sizeof(io));
+	io.io.getline = rerere_file_getline;
+	io.io.putstr = rerere_file_putstr;
+	io.io.putmem = rerere_file_putmem;
+	io.input = fopen(path, "r");
+	io.wrerror = 0;
+	if (!io.input)
+		return error("Could not open %s", path);
+
+	if (output) {
+		io.output = fopen(output, "w");
+		if (!io.output) {
+			fclose(io.input);
+			return error("Could not write %s", output);
+		}
+	}
+
+	hunk_no = handle_path(sha1, (struct rerere_io *)&io);
+
+	fclose(io.input);
+	if (io.wrerror)
+		error("There were errors while writing %s (%s)",
+		      path, strerror(io.wrerror));
+	if (io.output && fclose(io.output))
+		io.wrerror = error("Failed to flush %s: %s",
+				   path, strerror(errno));
+
+	if (hunk_no < 0) {
 		if (output)
 			unlink_or_warn(output);
 		return error("Could not parse conflict hunks in %s", path);
 	}
-	if (wrerror)
+	if (io.wrerror)
 		return -1;
 	return hunk_no;
 }
-- 
1.6.6

[PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

After you find out an earlier resolution you told rerere to use was a
mismerge, there is no easy way to clear it.  A new subcommand "forget" can
be used to tell git to forget a recorded resolution, so that you can redo
the merge from scratch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-rerere.c          |    2 +
 rerere.c                  |  127 +++++++++++++++++++++++++++++++++++++++++++++
 rerere.h                  |    1 +
 t/t2030-unresolve-info.sh |   22 ++++++++
 4 files changed, 152 insertions(+), 0 deletions(-)

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 2be9ffb..0253abf 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -110,6 +110,8 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 
 	if (!strcmp(argv[1], "-h"))
 		usage(git_rerere_usage);
+	else if (!strcmp(argv[1], "forget"))
+		return rerere_forget(argv + 2);
 
 	fd = setup_rerere(&merge_rr);
 	if (fd < 0)
diff --git a/rerere.c b/rerere.c
index f013ae7..c1da6f6 100644
--- a/rerere.c
+++ b/rerere.c
@@ -3,6 +3,9 @@
 #include "rerere.h"
 #include "xdiff/xdiff.h"
 #include "xdiff-interface.h"
+#include "dir.h"
+#include "resolve-undo.h"
+#include "ll-merge.h"
 
 /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */
 static int rerere_enabled = -1;
@@ -229,6 +232,95 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output
 	return hunk_no;
 }
 
+struct rerere_io_mem {
+	struct rerere_io io;
+	struct strbuf input;
+};
+
+static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_)
+{
+	struct rerere_io_mem *io = (struct rerere_io_mem *)io_;
+	char *ep;
+	size_t len;
+
+	strbuf_release(sb);
+	if (!io->input.len)
+		return -1;
+	ep = strchrnul(io->input.buf, '\n');
+	if (*ep == '\n')
+		ep++;
+	len = ep - io->input.buf;
+	strbuf_add(sb, io->input.buf, len);
+	strbuf_remove(&io->input, 0, len);
+	return 0;
+}
+
+static void rerere_mem_putstr(const char *str, struct rerere_io *io_)
+{
+	; /* no-op */
+}
+
+
+static void rerere_mem_putmem(const char *mem, size_t sz, struct rerere_io *io_)
+{
+	; /* no-op */
+}
+
+static int handle_cache(const char *path, unsigned char *sha1)
+{
+	mmfile_t mmfile[3];
+	mmbuffer_t result = {NULL, 0};
+	struct cache_entry *ce;
+	int pos, len, i, hunk_no;
+	struct rerere_io_mem io;
+
+	/*
+	 * Reproduce the conflicted merge in-core
+	 */
+	len = strlen(path);
+	pos = cache_name_pos(path, len);
+	if (0 <= pos)
+		return -1;
+	pos = -pos - 1;
+
+	for (i = 0; i < 3; i++) {
+		enum object_type type;
+		unsigned long size;
+
+		mmfile[i].size = 0;
+		mmfile[i].ptr = NULL;
+		if (active_nr <= pos)
+			break;
+		ce = active_cache[pos++];
+		if (ce_namelen(ce) != len || memcmp(ce->name, path, len)
+		    || ce_stage(ce) != i + 1)
+			break;
+		mmfile[i].ptr = read_sha1_file(ce->sha1, &type, &size);
+		mmfile[i].size = size;
+	}
+	for (i = 0; i < 3; i++) {
+		if (!mmfile[i].ptr && !mmfile[i].size)
+			mmfile[i].ptr = xstrdup("");
+	}
+	ll_merge(&result, path, &mmfile[0],
+		 &mmfile[1], "ours",
+		 &mmfile[2], "theirs", 0);
+	for (i = 0; i < 3; i++)
+		free(mmfile[i].ptr);
+
+	memset(&io, 0, sizeof(&io));
+	io.io.getline = rerere_mem_getline;
+	io.io.putstr = rerere_mem_putstr;
+	io.io.putmem = rerere_mem_putmem;
+
+	strbuf_init(&io.input, 0);
+	strbuf_attach(&io.input, result.ptr, result.size, result.size);
+
+	hunk_no = handle_path(sha1, (struct rerere_io *)&io);
+	strbuf_release(&io.input);
+	return hunk_no;
+}
+
 static int find_conflict(struct string_list *conflict)
 {
 	int i;
@@ -440,3 +532,38 @@ int rerere(void)
 		return 0;
 	return do_plain_rerere(&merge_rr, fd);
 }
+
+int rerere_forget(const char **pathspec)
+{
+	int i;
+	struct string_list conflict = { NULL, 0, 0, 1 };
+
+	if (read_cache() < 0)
+		return error("Could not read index");
+
+	unmerge_cache(pathspec);
+	find_conflict(&conflict);
+	for (i = 0; i < conflict.nr; i++) {
+		unsigned char sha1[20];
+		const char *postimage;
+		int ret;
+		struct string_list_item *it = &conflict.items[i];
+		if (!match_pathspec(pathspec, it->string, strlen(it->string),
+				    0, NULL))
+			continue;
+		ret = handle_cache(it->string, sha1);
+		if (ret < 1) {
+			error("Could not parse conflict hunks in '%s'",
+			      it->string);
+			continue;
+		}
+		postimage = rerere_path(sha1_to_hex(sha1), "postimage");
+		if (!unlink(postimage))
+			fprintf(stderr, "forgot resolution for %s\n", it->string);
+		else if (errno == ENOENT)
+			error("no remembered resolution for %s", it->string);
+		else
+			error("cannot unlink %s: %s", postimage, strerror(errno));
+	}
+	return 0;
+}
diff --git a/rerere.h b/rerere.h
index 13313f3..36560ff 100644
--- a/rerere.h
+++ b/rerere.h
@@ -7,5 +7,6 @@ extern int setup_rerere(struct string_list *);
 extern int rerere(void);
 extern const char *rerere_path(const char *hex, const char *file);
 extern int has_rerere_resolution(const char *hex);
+extern int rerere_forget(const char **);
 
 #endif
diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh
index 28e2eb1..92e006b 100755
--- a/t/t2030-unresolve-info.sh
+++ b/t/t2030-unresolve-info.sh
@@ -115,4 +115,26 @@ test_expect_success 'unmerge with plumbing' '
 	test $(wc -l <actual) = 3
 '
 
+test_expect_success 'rerere and rerere --forget' '
+	mkdir .git/rr-cache &&
+	prime_resolve_undo &&
+	echo record the resolution &&
+	git rerere &&
+	rerere_id=$(cd .git/rr-cache && echo */postimage) &&
+	rerere_id=${rerere_id%/postimage} &&
+	test -f .git/rr-cache/$rerere_id/postimage &&
+	git checkout -m file &&
+	echo resurrect the conflict &&
+	grep "^=======" file &&
+	echo reresolve the conflict &&
+	git rerere &&
+	test "z$(cat file)" = zdifferent &&
+	echo register the resolution again &&
+	git add file &&
+	check_resolve_undo kept file initial:file second:file third:file &&
+	test -z "$(git ls-files -u)" &&
+	git rerere forget file &&
+	! test -f .git/rr-cache/$rerere_id/postimage
+'
+
 test_done
-- 
1.6.6

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Johannes Sixt]

Junio C Hamano schrieb:
> After you find out an earlier resolution you told rerere to use was a
> mismerge, there is no easy way to clear it.  A new subcommand "forget" can
> be used to tell git to forget a recorded resolution, so that you can redo
> the merge from scratch.
> ...
> diff --git a/rerere.c b/rerere.c
> index f013ae7..c1da6f6 100644
> --- a/rerere.c
> +++ b/rerere.c
> ...
> +static int handle_cache(const char *path, unsigned char *sha1)
> +{
> +...
> +	ll_merge(&result, path, &mmfile[0],
> +		 &mmfile[1], "ours",
> +		 &mmfile[2], "theirs", 0);

When you simply call ll_merge(), will it obey any merge drivers that are 
defined in .gitattributes? Do we care about them?

I already had an implementation of "rerere forget" before you presented 
this solution, but it relies on that the user calls "checkout 
--conflict=merge" first. One reason (besides its simplicity) was that it 
does not have to care how the merge is computed.

[I haven't submitted my solution, yet, because I haven't had the time to 
do this large merge where I expect to make use "rerere forget", testing 
its usefulness.]

-- Hannes

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Junio C Hamano schrieb:
>> After you find out an earlier resolution you told rerere to use was a
>> mismerge, there is no easy way to clear it.  A new subcommand "forget" can
>> be used to tell git to forget a recorded resolution, so that you can redo
>> the merge from scratch.
>> ...
>> diff --git a/rerere.c b/rerere.c
>> index f013ae7..c1da6f6 100644
>> --- a/rerere.c
>> +++ b/rerere.c
>> ...
>> +static int handle_cache(const char *path, unsigned char *sha1)
>> +{
>> +...
>> +	ll_merge(&result, path, &mmfile[0],
>> +		 &mmfile[1], "ours",
>> +		 &mmfile[2], "theirs", 0);
>
> When you simply call ll_merge(), will it obey any merge drivers that
> are defined in .gitattributes? Do we care about them?
>
> I already had an implementation of "rerere forget" before you
> presented this solution, but it relies on that the user calls
> "checkout --conflict=merge" first. One reason (besides its simplicity)
> was that it does not have to care how the merge is computed.

Doesn't "checkout --conflict=merge" use the same ll_merge() machinery?

> [I haven't submitted my solution, yet, because I haven't had the time
> to do this large merge where I expect to make use "rerere forget",
> testing its usefulness.]
>
> -- Hannes

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Johannes Sixt]

Junio C Hamano schrieb:
> Johannes Sixt <j6t@kdbg.org> writes:
>> When you simply call ll_merge(), will it obey any merge drivers that
>> are defined in .gitattributes? Do we care about them?
>>
>> I already had an implementation of "rerere forget" before you
>> presented this solution, but it relies on that the user calls
>> "checkout --conflict=merge" first. One reason (besides its simplicity)
>> was that it does not have to care how the merge is computed.
> 
> Doesn't "checkout --conflict=merge" use the same ll_merge() machinery?

It does, without setting up .gitattributes, either (IIUC), which is a bug IMO.

That said, I consider your solution superior because it works without 
depending on conflict markers in the file in the worktree. Nevertheless, 
we should think about whether merge drivers of .gitattributes should be 
obeyed. I think they should: For example, a specialized XML merge driver 
could leave conflicts that are different from those that merge-recursive 
generates.

-- Hannes

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

>> Doesn't "checkout --conflict=merge" use the same ll_merge() machinery?
>
> It does, without setting up .gitattributes, either (IIUC), which is a bug IMO.

I don't think there is any bug there; git_checkattr() that is used by
git_path_check_merge() autoinitializes the attribute machinery.

However, I think you need this fix for "checkout -m" to work as intended,
if your custom merge driver has recursive attribute set to 'binary' or
something:

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..50f5079 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -167,7 +167,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	fill_mm(active_cache[pos+2]->sha1, &theirs);
 
 	status = ll_merge(&result_buf, path, &ancestor,
-			  &ours, "ours", &theirs, "theirs", 1);
+			  &ours, "ours", &theirs, "theirs", 0);
 	free(ancestor.ptr);
 	free(ours.ptr);
 	free(theirs.ptr);

I don't know why I decided to pass "1" to ll_merge() there; it doesn't
make any sense and looks like an untested bug to me.

The ll_merge() call jc/cache-unmerge topic adds for "rerere forget"
doesn't share this problem.

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> However, I think you need this fix for "checkout -m" to work as intended,

Here is a cleaner reroll with tests, meant to be applied on v1.6.1-rc1 or
later.

-- >8 --
Subject: [PATCH] checkout -m path: fix recreating conflicts

We should tell ll_merge() that the 3-way merge between stages #2 and #3 is
an outermost merge, not a virtual-ancestor creation.

Back when this code was originally written, users couldn't write custom
merge drivers easily, so the bug didn't matter, but these days it does.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |    2 +-
 t/t7201-co.sh      |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 7f3bd7b..e41e73b 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -179,7 +179,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	fill_mm(active_cache[pos+2]->sha1, &theirs);
 
 	status = ll_merge(&result_buf, path, &ancestor,
-			  &ours, "ours", &theirs, "theirs", 1);
+			  &ours, "ours", &theirs, "theirs", 0);
 	free(ancestor.ptr);
 	free(ours.ptr);
 	free(theirs.ptr);
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0e21632..3214ad2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -534,4 +534,61 @@ test_expect_success 'failing checkout -b should not break working tree' '
 
 '
 
+(
+ echo "#!$SHELL_PATH"
+ cat <<\EOF
+O=$1 A=$2 B=$3
+cat "$A" >.tmp
+exec >"$A"
+echo '<<<<<<< filfre-theirs'
+cat "$B"
+echo '||||||| filfre-common'
+cat "$O"
+echo '======='
+cat ".tmp"
+echo '>>>>>>> filfre-ours'
+rm -f .tmp
+exit 1
+EOF
+) >filfre.sh
+chmod +x filfre.sh
+
+test_expect_success 'custom merge driver with checkout -m' '
+	git reset --hard &&
+
+	git config merge.filfre.driver "./filfre.sh %O %A %B" &&
+	git config merge.filfre.name "Feel-free merge driver" &&
+	git config merge.filfre.recursive binary &&
+	echo "arm merge=filfre" >.gitattributes &&
+
+	git checkout -b left &&
+	echo neutral >arm &&
+	git add arm .gitattributes &&
+	test_tick &&
+	git commit -m neutral &&
+	git branch right &&
+
+	echo left >arm &&
+	test_tick &&
+	git commit -a -m left &&
+	git checkout right &&
+
+	echo right >arm &&
+	test_tick &&
+	git commit -a -m right &&
+
+	test_must_fail git merge left &&
+	(
+		for t in filfre-common left right
+		do
+			grep $t arm || exit 1
+		done
+		exit 0
+	) &&
+
+	mv arm expect &&
+	git checkout -m arm &&
+	test_cmp expect arm
+'
+
 test_done
-- 
1.6.6.184.ge163d

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Johannes Sixt]

Your implementation forgets to re-insert the forgotten resolutions into
MERGE_RR, therefore, the next 'git rerere' does not record the new
resolution.

In my implementation of 'rerere forget', I had the following tests.
The latter two of the three new tests fail with your implementation.

-- Hannes

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index a6bc028..a3f0c18 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -75,8 +75,9 @@ test_expect_success 'no postimage or thisimage yet' \
 test_expect_success 'preimage has right number of lines' '
 
 	cnt=$(sed -ne "/^<<<<<<</,/^>>>>>>>/p" $rr/preimage | wc -l) &&
-	test $cnt = 13
-
+	test $cnt = 13 &&
+	cnt=$(grep "To die! T" $rr/preimage | wc -l) &&
+	test $cnt = 1
 '
 
 git show first:a1 > a1
@@ -142,7 +143,23 @@ test_expect_success 'rerere kicked in' "! grep ^=======$ a1"
 
 test_expect_success 'rerere prefers first change' 'test_cmp a1 expect'
 
-rm $rr/postimage
+test_expect_success 'rerere forget drops postimage' '
+	git rerere forget a1 &&
+	! test -f $rr/postimage
+'
+
+test_expect_success 'conflict hash is the same as before' '
+	test $sha1 = "$(perl -pe "s/	.*//" .git/MERGE_RR)"
+'
+
+test_expect_success 'preimage was updated' '
+	cnt=$(sed -ne "/^<<<<<<</,/^>>>>>>>/p" $rr/preimage | wc -l) &&
+	test $cnt = 13 &&
+	cnt=$(grep "To die! T" $rr/preimage | wc -l) &&
+	test $cnt = 2
+'
+
+rm -f $rr/postimage
 echo "$sha1	a1" | perl -pe 'y/\012/\000/' > .git/MERGE_RR
 
 test_expect_success 'rerere clear' 'git rerere clear'

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Your implementation forgets to re-insert the forgotten resolutions into
> MERGE_RR, therefore, the next 'git rerere' does not record the new
> resolution.
>
> In my implementation of 'rerere forget', I had the following tests.

Please filfre to roll a patch that adds the tests and code that inserts
necessary MERGE_RR entries, if the feature is pressing; unfortunately I
don't think I will have much git time during the coming weekend.

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Your implementation forgets to re-insert the forgotten resolutions into
>> MERGE_RR, therefore, the next 'git rerere' does not record the new
>> resolution.
>>
>> In my implementation of 'rerere forget', I had the following tests.
>
> Please filfre to roll a patch that adds the tests and code that inserts
> necessary MERGE_RR entries, if the feature is pressing; unfortunately I
> don't think I will have much git time during the coming weekend.

I ended up doing this myself.  As we are dropping the postimage and adding
a new MERGE_RR entry, I also think that it is safer to update the preimage
with the conflict we got for this round, so I added that as well.

However, I think there is a room for improvement in preimage handling.

Currently, the rerere database is indexed with the conflict hash and for
each conflict hash you can record a single preimage-postimage pair to
replay.  But you can have conflicts with the same conflict hash, but with
slightly different contexts outside the conflicted region, and the right
resolution can be different depending on the outside context.

In the traditional implementation, I punted this issue by noticing
conflicts in the three-way merge between pre, post and this images.  If
preimage is too different from the conflicted contents we got during this
merge, then the previous resolution should not apply.

But I think the right solution would be to have more than one preimage and
postimage pairs (preimage.0 vs postimage.0,... etc.) and try to use each
of them in handle_path() until it finds one that can be used to cleanly
merge with the conflict we got in thisimage during this round.

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Johannes Sixt]

On Montag, 11. Januar 2010, Junio C Hamano wrote:
> I ended up doing this myself.  As we are dropping the postimage and adding
> a new MERGE_RR entry, I also think that it is safer to update the preimage
> with the conflict we got for this round, so I added that as well.

Thank you, it appears to work as expected. It is actually very important to 
update the preimage as well, otherwise, the new postimage can contain 
unrelated additional changes.

> However, I think there is a room for improvement in preimage handling.
>
> Currently, the rerere database is indexed with the conflict hash and for
> each conflict hash you can record a single preimage-postimage pair to
> replay.  But you can have conflicts with the same conflict hash, but with
> slightly different contexts outside the conflicted region, and the right
> resolution can be different depending on the outside context.

I did encounter a case where the same resolution would apply to all conflicts 
that have the same conflict hash, so it's not quite what you talk about. But 
not all conflicts were automatically resolved. I haven't yet analyzed what 
happened - it could just be that the xdl_merge call fails due to the 
differences in the text immediately outside the conflict markers.

> In the traditional implementation, I punted this issue by noticing
> conflicts in the three-way merge between pre, post and this images.  If
> preimage is too different from the conflicted contents we got during this
> merge, then the previous resolution should not apply.
>
> But I think the right solution would be to have more than one preimage and
> postimage pairs (preimage.0 vs postimage.0,... etc.) and try to use each
> of them in handle_path() until it finds one that can be used to cleanly
> merge with the conflict we got in thisimage during this round.

The situation happens rarely, so I don't know if we should care. OTOH, *when* 
the situation arises, and a recorded resolution is applied incorrectly, it 
may be quite annoying. Dunno.

-- Hannes

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> I did encounter a case where the same resolution would apply to all
> conflicts that have the same conflict hash, so it's not quite what you
> talk about. But not all conflicts were automatically resolved. I haven't
> yet analyzed what happened - it could just be that the xdl_merge call
> fails due to the differences in the text immediately outside the
> conflict markers.

Actually it is _very_ easy to fool rerere to do something totally
unexpected, and I have been thinking about using the similarity comparison
algorithm on the region outside the conflicted area between preimage and
thisimage and reject use of rerere.

Try this in an empty directory.

-- >8 --

#!/bin/sh

git init

create_numbers () {
	for n in 0 1 2 3 4 "$1" 5 6 7 8 9
	do
		echo $n
	done >numbers.txt
}

create_letters () {
	for l in a b c d e "$1" f g h i j
	do
		echo $l
	done >letters.txt
}

create_files () {
	create_numbers "$1"
	create_letters "$1"
}

create_files ""
git add numbers.txt letters.txt
git commit -m initial
git branch side

create_files "+"
git commit -a -m master

git checkout side
create_files "-"
git commit -a -m side

mkdir -p .git/rr-cache

# On this history we changed an empty line to +; merge
# with another history that changed it to -
git checkout master^0
git merge side

# The above should have conflicted.  The resolution is to '='

create_numbers "="
git rerere

git rerere status
git rerere diff
cat numbers.txt
cat letters.txt

-- 8< --

Now, immediately after this sequence, rerere will give you an disaster.

Re: [PATCH 9/9] rerere forget path: forget recorded resolution

[Johannes Sixt]

On Montag, 11. Januar 2010, Junio C Hamano wrote:
> Actually it is _very_ easy to fool rerere to do something totally
> unexpected, and I have been thinking about using the similarity comparison
> algorithm on the region outside the conflicted area between preimage and
> thisimage and reject use of rerere.
>
> Try this in an empty directory.
>[snip]
> Now, immediately after this sequence, rerere will give you an disaster.

Indeed. The problem here is that two entirely different resolutions are 
recorded for the same conflict hash *in one run of rerere*. The damage can be 
avoided if conflict hashes are not reused in do_plain_rerere (in the first 
loop). (Though, I'm currently not in the mood to look into this in more 
depth.)

Of course, this does not mean that *both* conflicts can be resolved 
automatically when the merge is repeated. In my use-case this would have been 
desirable (and even your example would suggest it is, but that is not 
generally true).

-- Hannes