[PATCH 0/7] saving and replaying multiple variants with rerere

[PATCH 0/7] saving and replaying multiple variants with rerere

[Junio C Hamano]

This is a continuation of jc/rerere topic that is now in 'next'.
The old topic is purely about refactoring and code cleaning and this
series builds on top of it.

We record a previously seen conflict and its resolution in a
subdirectory of $GIT_DIR/rr-cache, and the name of the subdirectory
is computed by hashing the "shape" of the conflicted hunk.  When a
similar-looking conflict is encountered later in the working tree,
"git rerere" tries to replay the recorded resolution by running a
three-way merge to apply the change to go from the previous conflict
to the previous resolution to the current conflicted state.

Depending on what appears in the context around the conflicted hunk,
however, this three-way merge itself can conflict and fail to apply
to a seemingly identical conflict.  You _could_ argue that you could
merge with reduced context lines to work around the problem, but
that is a risky proposition.

As a safer approach, these patches allow us to keep more than one
pairs of <preimage,postimage> (i.e. conflicted state and its
resolution) in the same "shape".  If two paths have identical
looking conflict and one's resolution does not cleanly replay to the
other's conflicted state, we now save the resolutions of these two
paths as different "variants" so that they can be applied when we
see these conflicts again.

I think the basic "record and replay" part is done with this round,
but I haven't really reviewed the part that deals with garbage
collection of old rerere database entries, which probably needs more
work (and could use help, hints hints...).

Junio C Hamano (7):
  rerere: split conflict ID further
  rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id
  rerere: handle leftover rr-cache/$ID directory and postimage files
  rerere: delay the recording of preimage
  rerere: allow multiple variants to exist
  t4200: rerere a merge with two identical conflicts
  rerere: do use multiple variants

 rerere.c          | 270 +++++++++++++++++++++++++++++++++++++++++++++---------
 rerere.h          |   4 +-
 t/t4200-rerere.sh |  92 ++++++++++++++++++-
 3 files changed, 321 insertions(+), 45 deletions(-)

-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 1/7] rerere: split conflict ID further

[Junio C Hamano]

The plan is to keep assigning the backward compatible conflict ID
based on the hash of the (normalized) text of conflicts, keep using
that conflict ID as the directory name under $GIT_DIR/rr-cache/, but
allow each conflicted path to use a separate "variant" to record
resolutions, i.e. having more than one <preimage,postimage> pairs
under $GIT_DIR/rr-cache/$ID/ directory.  As the first step in that
direction, separate the shared "conflict ID" out of the rerere_id
structure.

The plan is to keep information per $ID in rerere_dir, that can be
shared among rerere_id that is per conflicted path.

When we are done with rerere(), which can be directly called from
other programs like "git apply", "git commit" and "git merge", the
shared rerere_dir structures can be freed entirely, so they are not
reference-counted and they are not freed when we release rerere_id's
that reference them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 rerere.h |  3 ++-
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/rerere.c b/rerere.c
index 9bef24f..a5d8a06 100644
--- a/rerere.c
+++ b/rerere.c
@@ -8,6 +8,7 @@
 #include "ll-merge.h"
 #include "attr.h"
 #include "pathspec.h"
+#include "sha1-lookup.h"
 
 #define RESOLVED 0
 #define PUNTED 1
@@ -22,6 +23,23 @@ static int rerere_autoupdate;
 
 static char *merge_rr_path;
 
+static int rerere_dir_nr;
+static int rerere_dir_alloc;
+
+static struct rerere_dir {
+	unsigned char sha1[20];
+} **rerere_dir;
+
+static void free_rerere_dirs(void)
+{
+	int i;
+	for (i = 0; i < rerere_dir_nr; i++)
+		free(rerere_dir[i]);
+	free(rerere_dir);
+	rerere_dir_nr = rerere_dir_alloc = 0;
+	rerere_dir = NULL;
+}
+
 static void free_rerere_id(struct string_list_item *item)
 {
 	free(item->util);
@@ -29,7 +47,7 @@ static void free_rerere_id(struct string_list_item *item)
 
 static const char *rerere_id_hex(const struct rerere_id *id)
 {
-	return id->hex;
+	return sha1_to_hex(id->collection->sha1);
 }
 
 const char *rerere_path(const struct rerere_id *id, const char *file)
@@ -40,6 +58,37 @@ const char *rerere_path(const struct rerere_id *id, const char *file)
 	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
 }
 
+static const unsigned char *rerere_dir_sha1(size_t i, void *table)
+{
+	struct rerere_dir **rr_dir = table;
+	return rr_dir[i]->sha1;
+}
+
+static struct rerere_dir *find_rerere_dir(const char *hex)
+{
+	unsigned char sha1[20];
+	struct rerere_dir *rr_dir;
+	int pos;
+
+	if (get_sha1_hex(hex, sha1))
+		return NULL; /* BUG */
+	pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr, rerere_dir_sha1);
+	if (pos < 0) {
+		rr_dir = xmalloc(sizeof(*rr_dir));
+		hashcpy(rr_dir->sha1, sha1);
+		pos = -1 - pos;
+
+		/* Make sure the array is big enough ... */
+		ALLOC_GROW(rerere_dir, rerere_dir_nr + 1, rerere_dir_alloc);
+		/* ... and add it in. */
+		rerere_dir_nr++;
+		memmove(rerere_dir + pos + 1, rerere_dir + pos,
+			(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
+		rerere_dir[pos] = rr_dir;
+	}
+	return rerere_dir[pos];
+}
+
 static int has_rerere_resolution(const struct rerere_id *id)
 {
 	struct stat st;
@@ -50,7 +99,7 @@ static int has_rerere_resolution(const struct rerere_id *id)
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
-	strcpy(id->hex, hex);
+	id->collection = find_rerere_dir(hex);
 	return id;
 }
 
@@ -810,12 +859,14 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 int rerere(int flags)
 {
 	struct string_list merge_rr = STRING_LIST_INIT_DUP;
-	int fd;
+	int fd, status;
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
-	return do_plain_rerere(&merge_rr, fd);
+	status = do_plain_rerere(&merge_rr, fd);
+	free_rerere_dirs();
+	return status;
 }
 
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
@@ -900,7 +951,7 @@ int rerere_forget(struct pathspec *pathspec)
 static struct rerere_id *dirname_to_id(const char *name)
 {
 	static struct rerere_id id;
-	strcpy(id.hex, name);
+	id.collection = find_rerere_dir(name);
 	return &id;
 }
 
diff --git a/rerere.h b/rerere.h
index ce545d0..faf5a23 100644
--- a/rerere.h
+++ b/rerere.h
@@ -15,8 +15,9 @@ struct pathspec;
  */
 extern void *RERERE_RESOLVED;
 
+struct rerere_dir;
 struct rerere_id {
-	char hex[41];
+	struct rerere_dir *collection;
 };
 
 extern int setup_rerere(struct string_list *, int);
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 2/7] rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id

[Junio C Hamano]

This will help fixing bootstrap corner-case issues, e.g. having an
empty $GIT_DIR/rr-cache/$ID directory would fail to record a
preimage, in later changes in this series.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/rerere.c b/rerere.c
index a5d8a06..fbdade8 100644
--- a/rerere.c
+++ b/rerere.c
@@ -26,8 +26,11 @@ static char *merge_rr_path;
 static int rerere_dir_nr;
 static int rerere_dir_alloc;
 
+#define RR_HAS_POSTIMAGE 1
+#define RR_HAS_PREIMAGE 2
 static struct rerere_dir {
 	unsigned char sha1[20];
+	unsigned char status;
 } **rerere_dir;
 
 static void free_rerere_dirs(void)
@@ -58,6 +61,27 @@ const char *rerere_path(const struct rerere_id *id, const char *file)
 	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
 }
 
+static int is_rr_file(const char *name, const char *filename)
+{
+	return !strcmp(name, filename);
+}
+
+static void scan_rerere_dir(struct rerere_dir *rr_dir)
+{
+	struct dirent *de;
+	DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->sha1)));
+
+	if (!dir)
+		return;
+	while ((de = readdir(dir)) != NULL) {
+		if (is_rr_file(de->d_name, "postimage"))
+			rr_dir->status |= RR_HAS_POSTIMAGE;
+		else if (is_rr_file(de->d_name, "preimage"))
+			rr_dir->status |= RR_HAS_PREIMAGE;
+	}
+	closedir(dir);
+}
+
 static const unsigned char *rerere_dir_sha1(size_t i, void *table)
 {
 	struct rerere_dir **rr_dir = table;
@@ -76,6 +100,7 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 	if (pos < 0) {
 		rr_dir = xmalloc(sizeof(*rr_dir));
 		hashcpy(rr_dir->sha1, sha1);
+		rr_dir->status = 0;
 		pos = -1 - pos;
 
 		/* Make sure the array is big enough ... */
@@ -85,15 +110,14 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 		memmove(rerere_dir + pos + 1, rerere_dir + pos,
 			(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
 		rerere_dir[pos] = rr_dir;
+		scan_rerere_dir(rr_dir);
 	}
 	return rerere_dir[pos];
 }
 
 static int has_rerere_resolution(const struct rerere_id *id)
 {
-	struct stat st;
-
-	return !stat(rerere_path(id, "postimage"), &st);
+	return (id->collection->status & RR_HAS_POSTIMAGE);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -737,6 +761,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	} else if (!handle_file(path, NULL, NULL)) {
 		/* The user has resolved it. */
 		copy_file(rerere_path(id, "postimage"), path, 0666);
+		id->collection->status |= RR_HAS_POSTIMAGE;
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	} else {
 		return;
@@ -797,6 +822,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		 * normalized contents to the "preimage" file.
 		 */
 		handle_file(path, NULL, rerere_path(id, "preimage"));
+		id->collection->status |= RR_HAS_PREIMAGE;
 		fprintf(stderr, "Recorded preimage for '%s'\n", path);
 	}
 
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 3/7] rerere: handle leftover rr-cache/$ID directory and postimage files

[Junio C Hamano]

If by some accident there is only $GIT_DIR/rr-cache/$ID directory
existed, we wouldn't have recorded a preimage for a conflict that
is newly encountered, which would mean after a manual resolution,
we wouldn't have recorded it by storing the postimage, because the
logic used to be "if there is no rr-cache/$ID directory, then we are
the first so record the preimage".  Instead, record preimage if we
do not have one.

In addition, if there is only $GIT_DIR/rr-cache/$ID/postimage
without corresponding preimage, we would have tried to call into
merge() and punted.

These would have been a situation frustratingly hard to recover
from.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c          | 42 +++++++++++++++++++++++++-----------------
 t/t4200-rerere.sh | 18 +++++++++++++++++-
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/rerere.c b/rerere.c
index fbdade8..a1e2963 100644
--- a/rerere.c
+++ b/rerere.c
@@ -117,7 +117,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 
 static int has_rerere_resolution(const struct rerere_id *id)
 {
-	return (id->collection->status & RR_HAS_POSTIMAGE);
+	const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+
+	return ((id->collection->status & both) == both);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -806,24 +808,30 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		string_list_insert(rr, path)->util = id;
 
 		/*
-		 * If the directory does not exist, create
-		 * it.  mkdir_in_gitdir() will fail with
-		 * EEXIST if there already is one.
-		 *
-		 * NEEDSWORK: make sure "gc" does not remove
-		 * preimage without removing the directory.
+		 * Ensure that the directory exists.
+		 * mkdir_in_gitdir() will fail with EEXIST if there
+		 * already is one.
 		 */
-		if (mkdir_in_gitdir(rerere_path(id, NULL)))
-			continue;
+		if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
+		    errno != EEXIST)
+			continue; /* NEEDSWORK: perhaps we should die? */
 
-		/*
-		 * We are the first to encounter this
-		 * conflict.  Ask handle_file() to write the
-		 * normalized contents to the "preimage" file.
-		 */
-		handle_file(path, NULL, rerere_path(id, "preimage"));
-		id->collection->status |= RR_HAS_PREIMAGE;
-		fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		if (id->collection->status & RR_HAS_PREIMAGE) {
+			;
+		} else {
+			/*
+			 * We are the first to encounter this
+			 * conflict.  Ask handle_file() to write the
+			 * normalized contents to the "preimage" file.
+			 *
+			 * NEEDSWORK: what should happen if we had a
+			 * leftover postimage that is totally
+			 * unrelated?  Perhaps we should unlink it?
+			 */
+			handle_file(path, NULL, rerere_path(id, "preimage"));
+			id->collection->status |= RR_HAS_PREIMAGE;
+			fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		}
 	}
 
 	for (i = 0; i < rr->nr; i++)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index ed9c91e..52a8e09 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -184,12 +184,28 @@ test_expect_success 'rerere updates postimage timestamp' '
 '
 
 test_expect_success 'rerere clear' '
-	rm $rr/postimage &&
+	cp $rr/preimage .git/pre-saved &&
+	mv $rr/postimage .git/post-saved &&
 	echo "$sha1	a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
 	git rerere clear &&
 	! test -d $rr
 '
 
+test_expect_success 'leftover directory' '
+	git reset --hard &&
+	mkdir -p $rr &&
+	test_must_fail git merge first &&
+	test -f $rr/preimage
+'
+
+test_expect_success 'missing preimage' '
+	git reset --hard &&
+	mkdir -p $rr &&
+	cp .git/post-saved $rr/postimage &&
+	test_must_fail git merge first &&
+	test -f $rr/preimage
+'
+
 test_expect_success 'set up for garbage collection tests' '
 	mkdir -p $rr &&
 	echo Hello >$rr/preimage &&
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 4/7] rerere: delay the recording of preimage

[Junio C Hamano]

We record the preimage only when there is no directory to record the
conflict we encountered, i.e. when $GIT_DIR/rr-cache/$ID does not
exist.  As the plan is to allow multiple <preimage,postimage> pairs
as variants for the same conflict ID eventually, this logic needs to
go.

As the first step in that direction, stop the "did we create the
directory?  Then we record the preimage" logic.  Instead, we record
if a preimage does not exist when we saw a conflict in a path.  Also
make sure that we create a new preimage under $ID with existing
stale postimage, which most likely is totally unrelated to the
resolution of this new conflict.

In later patches, we will further update this logic to be "do we
have <preimage,postimage> pair that cleanly resolve the current
conflicts?  If not, record a new preimage as a new variant", but
that does not happen at this stage yet.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 52 +++++++++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/rerere.c b/rerere.c
index a1e2963..c0482b8 100644
--- a/rerere.c
+++ b/rerere.c
@@ -122,6 +122,11 @@ static int has_rerere_resolution(const struct rerere_id *id)
 	return ((id->collection->status & both) == both);
 }
 
+static int has_rerere_preimage(const struct rerere_id *id)
+{
+	return (id->collection->status & RR_HAS_PREIMAGE);
+}
+
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
@@ -749,8 +754,24 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	const char *path = rr_item->string;
 	const struct rerere_id *id = rr_item->util;
 
-	/* Is there a recorded resolution we could attempt to apply? */
-	if (has_rerere_resolution(id)) {
+	if (!has_rerere_preimage(id)) {
+		/*
+		 * We are the first to encounter this conflict.  Ask
+		 * handle_file() to write the normalized contents to
+		 * the "preimage" file.
+		 */
+		handle_file(path, NULL, rerere_path(id, "preimage"));
+		if (id->collection->status & RR_HAS_POSTIMAGE) {
+			const char *path = rerere_path(id, "postimage");
+			if (unlink(path))
+				die_errno("cannot unlink stray '%s'", path);
+			id->collection->status &= ~RR_HAS_PREIMAGE;
+		}
+		id->collection->status |= RR_HAS_PREIMAGE;
+		fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		return;
+	} else if (has_rerere_resolution(id)) {
+		/* Is there a recorded resolution we could attempt to apply? */
 		if (merge(id, path))
 			return; /* failed to replay */
 
@@ -807,31 +828,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		id = new_rerere_id(sha1);
 		string_list_insert(rr, path)->util = id;
 
-		/*
-		 * Ensure that the directory exists.
-		 * mkdir_in_gitdir() will fail with EEXIST if there
-		 * already is one.
-		 */
-		if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
-		    errno != EEXIST)
-			continue; /* NEEDSWORK: perhaps we should die? */
-
-		if (id->collection->status & RR_HAS_PREIMAGE) {
-			;
-		} else {
-			/*
-			 * We are the first to encounter this
-			 * conflict.  Ask handle_file() to write the
-			 * normalized contents to the "preimage" file.
-			 *
-			 * NEEDSWORK: what should happen if we had a
-			 * leftover postimage that is totally
-			 * unrelated?  Perhaps we should unlink it?
-			 */
-			handle_file(path, NULL, rerere_path(id, "preimage"));
-			id->collection->status |= RR_HAS_PREIMAGE;
-			fprintf(stderr, "Recorded preimage for '%s'\n", path);
-		}
+		/* Ensure that the directory exists. */
+		mkdir_in_gitdir(rerere_path(id, NULL));
 	}
 
 	for (i = 0; i < rr->nr; i++)
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 5/7] rerere: allow multiple variants to exist

[Junio C Hamano]

The shape of the conflict in a path determines the conflict ID.  The
preimage and postimage pair that was recorded for the conflict ID
previously may or may not replay well for the conflict we just saw.

Currently, we punt when the previous resolution does not cleanly
replay, but ideally we should then be able to record the currently
conflicted path by assigning a new 'variant', and then record the
resolution the user is going to make.

Introduce a mechanism to have more than one variant for a given
conflict ID; we do not actually assign any variant other than 0th
variant yet at this step.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 rerere.h |   1 +
 2 files changed, 106 insertions(+), 22 deletions(-)

diff --git a/rerere.c b/rerere.c
index c0482b8..f674461 100644
--- a/rerere.c
+++ b/rerere.c
@@ -30,14 +30,17 @@ static int rerere_dir_alloc;
 #define RR_HAS_PREIMAGE 2
 static struct rerere_dir {
 	unsigned char sha1[20];
-	unsigned char status;
+	int status_alloc, status_nr;
+	unsigned char *status;
 } **rerere_dir;
 
 static void free_rerere_dirs(void)
 {
 	int i;
-	for (i = 0; i < rerere_dir_nr; i++)
+	for (i = 0; i < rerere_dir_nr; i++) {
+		free(rerere_dir[i]->status);
 		free(rerere_dir[i]);
+	}
 	free(rerere_dir);
 	rerere_dir_nr = rerere_dir_alloc = 0;
 	rerere_dir = NULL;
@@ -53,17 +56,59 @@ static const char *rerere_id_hex(const struct rerere_id *id)
 	return sha1_to_hex(id->collection->sha1);
 }
 
+static void fit_variant(struct rerere_dir *rr_dir, int variant)
+{
+	variant++;
+	ALLOC_GROW(rr_dir->status, variant, rr_dir->status_alloc);
+	if (rr_dir->status_nr < variant) {
+		memset(rr_dir->status + rr_dir->status_nr,
+		       '\0', variant - rr_dir->status_nr);
+		rr_dir->status_nr = variant;
+	}
+}
+
+static void assign_variant(struct rerere_id *id)
+{
+	int variant;
+	struct rerere_dir *rr_dir = id->collection;
+
+	variant = id->variant;
+	if (variant < 0) {
+		variant = 0; /* for now */
+	}
+	fit_variant(rr_dir, variant);
+	id->variant = variant;
+}
+
 const char *rerere_path(const struct rerere_id *id, const char *file)
 {
 	if (!file)
 		return git_path("rr-cache/%s", rerere_id_hex(id));
 
-	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+	if (id->variant <= 0)
+		return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+
+	return git_path("rr-cache/%s/%s.%d",
+			rerere_id_hex(id), file, id->variant);
 }
 
-static int is_rr_file(const char *name, const char *filename)
+static int is_rr_file(const char *name, const char *filename, int *variant)
 {
-	return !strcmp(name, filename);
+	const char *suffix;
+	char *ep;
+
+	if (!strcmp(name, filename)) {
+		*variant = 0;
+		return 1;
+	}
+	if (!skip_prefix(name, filename, &suffix) || *suffix != '.')
+		return 0;
+
+	errno = 0;
+	*variant = strtol(suffix + 1, &ep, 10);
+	if (errno || *ep)
+		return 0;
+	return 1;
 }
 
 static void scan_rerere_dir(struct rerere_dir *rr_dir)
@@ -74,10 +119,15 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
 	if (!dir)
 		return;
 	while ((de = readdir(dir)) != NULL) {
-		if (is_rr_file(de->d_name, "postimage"))
-			rr_dir->status |= RR_HAS_POSTIMAGE;
-		else if (is_rr_file(de->d_name, "preimage"))
-			rr_dir->status |= RR_HAS_PREIMAGE;
+		int variant;
+
+		if (is_rr_file(de->d_name, "postimage", &variant)) {
+			fit_variant(rr_dir, variant);
+			rr_dir->status[variant] |= RR_HAS_POSTIMAGE;
+		} else if (is_rr_file(de->d_name, "preimage", &variant)) {
+			fit_variant(rr_dir, variant);
+			rr_dir->status[variant] |= RR_HAS_PREIMAGE;
+		}
 	}
 	closedir(dir);
 }
@@ -100,7 +150,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 	if (pos < 0) {
 		rr_dir = xmalloc(sizeof(*rr_dir));
 		hashcpy(rr_dir->sha1, sha1);
-		rr_dir->status = 0;
+		rr_dir->status = NULL;
+		rr_dir->status_nr = 0;
+		rr_dir->status_alloc = 0;
 		pos = -1 - pos;
 
 		/* Make sure the array is big enough ... */
@@ -118,19 +170,27 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 static int has_rerere_resolution(const struct rerere_id *id)
 {
 	const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+	int variant = id->variant;
 
-	return ((id->collection->status & both) == both);
+	if (variant < 0)
+		return 0;
+	return ((id->collection->status[variant] & both) == both);
 }
 
 static int has_rerere_preimage(const struct rerere_id *id)
 {
-	return (id->collection->status & RR_HAS_PREIMAGE);
+	int variant = id->variant;
+
+	if (variant < 0)
+		return 0;
+	return (id->collection->status[variant] & RR_HAS_PREIMAGE);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
 	id->collection = find_rerere_dir(hex);
+	id->variant = -1; /* not known yet */
 	return id;
 }
 
@@ -157,16 +217,26 @@ static void read_rr(struct string_list *rr)
 		char *path;
 		unsigned char sha1[20];
 		struct rerere_id *id;
+		int variant;
 
 		/* There has to be the hash, tab, path and then NUL */
 		if (buf.len < 42 || get_sha1_hex(buf.buf, sha1))
 			die("corrupt MERGE_RR");
 
-		if (buf.buf[40] != '\t')
+		if (buf.buf[40] != '.') {
+			variant = 0;
+			path = buf.buf + 40;
+		} else {
+			errno = 0;
+			variant = strtol(buf.buf + 41, &path, 10);
+			if (errno)
+				die("corrupt MERGE_RR");
+		}
+		if (*(path++) != '\t')
 			die("corrupt MERGE_RR");
 		buf.buf[40] = '\0';
-		path = buf.buf + 41;
 		id = new_rerere_id_hex(buf.buf);
+		id->variant = variant;
 		string_list_insert(rr, path)->util = id;
 	}
 	strbuf_release(&buf);
@@ -187,9 +257,16 @@ static int write_rr(struct string_list *rr, int out_fd)
 		id = rr->items[i].util;
 		if (!id)
 			continue;
-		strbuf_addf(&buf, "%s\t%s%c",
-			    rerere_id_hex(id),
-			    rr->items[i].string, 0);
+		assert(id->variant >= 0);
+		if (0 < id->variant)
+			strbuf_addf(&buf, "%s.%d\t%s%c",
+				    rerere_id_hex(id), id->variant,
+				    rr->items[i].string, 0);
+		else
+			strbuf_addf(&buf, "%s\t%s%c",
+				    rerere_id_hex(id),
+				    rr->items[i].string, 0);
+
 		if (write_in_full(out_fd, buf.buf, buf.len) != buf.len)
 			die("unable to write rerere record");
 
@@ -752,7 +829,12 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 			       struct string_list *update)
 {
 	const char *path = rr_item->string;
-	const struct rerere_id *id = rr_item->util;
+	struct rerere_id *id = rr_item->util;
+	int variant;
+
+	if (id->variant < 0)
+		assign_variant(id);
+	variant = id->variant;
 
 	if (!has_rerere_preimage(id)) {
 		/*
@@ -761,13 +843,13 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 		 * the "preimage" file.
 		 */
 		handle_file(path, NULL, rerere_path(id, "preimage"));
-		if (id->collection->status & RR_HAS_POSTIMAGE) {
+		if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
 			const char *path = rerere_path(id, "postimage");
 			if (unlink(path))
 				die_errno("cannot unlink stray '%s'", path);
-			id->collection->status &= ~RR_HAS_PREIMAGE;
+			id->collection->status[variant] &= ~RR_HAS_PREIMAGE;
 		}
-		id->collection->status |= RR_HAS_PREIMAGE;
+		id->collection->status[variant] |= RR_HAS_PREIMAGE;
 		fprintf(stderr, "Recorded preimage for '%s'\n", path);
 		return;
 	} else if (has_rerere_resolution(id)) {
@@ -784,7 +866,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	} else if (!handle_file(path, NULL, NULL)) {
 		/* The user has resolved it. */
 		copy_file(rerere_path(id, "postimage"), path, 0666);
-		id->collection->status |= RR_HAS_POSTIMAGE;
+		id->collection->status[variant] |= RR_HAS_POSTIMAGE;
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	} else {
 		return;
@@ -919,6 +1001,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 
 	/* Nuke the recorded resolution for the conflict */
 	id = new_rerere_id(sha1);
+	id->variant = 0; /* for now */
 	filename = rerere_path(id, "postimage");
 	if (unlink(filename))
 		return (errno == ENOENT
diff --git a/rerere.h b/rerere.h
index faf5a23..64d38d9 100644
--- a/rerere.h
+++ b/rerere.h
@@ -18,6 +18,7 @@ extern void *RERERE_RESOLVED;
 struct rerere_dir;
 struct rerere_id {
 	struct rerere_dir *collection;
+	int variant;
 };
 
 extern int setup_rerere(struct string_list *, int);
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 6/7] t4200: rerere a merge with two identical conflicts

[Junio C Hamano]

When the context of multiple identical conflicts are different, two
seemingly the same conflict resolution cannot be safely applied.

In such a case, at least we should be able to record these two
resolutions separately in the rerere database, and reuse them when
we see the same conflict later.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4200-rerere.sh | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 52a8e09..8d52854 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -407,4 +407,78 @@ test_expect_success 'rerere -h' '
 	test_i18ngrep [Uu]sage help
 '
 
+concat_insert () {
+	last=$1
+	shift
+	cat early && printf "%s\n" "$@" && cat late "$last"
+}
+
+test_expect_failure 'multiple identical conflicts' '
+	git reset --hard &&
+
+	test_seq 1 6 >early &&
+	>late &&
+	test_seq 11 15 >short &&
+	test_seq 111 120 >long &&
+	concat_insert short >file1 &&
+	concat_insert long >file2 &&
+	git add file1 file2 &&
+	git commit -m base &&
+	git tag base &&
+	git checkout -b six.1 &&
+	concat_insert short 6.1 >file1 &&
+	concat_insert long 6.1 >file2 &&
+	git add file1 file2 &&
+	git commit -m 6.1 &&
+	git checkout -b six.2 HEAD^ &&
+	concat_insert short 6.2 >file1 &&
+	concat_insert long 6.2 >file2 &&
+	git add file1 file2 &&
+	git commit -m 6.2 &&
+
+	# At this point, six.1 and six.2
+	# - derive from common ancestor that has two files
+	#   1...6 7 11..15 (file1) and 1...6 7 111..120 (file2)
+	# - six.1 replaces these 7s with 6.1
+	# - six.2 replaces these 7s with 6.2
+
+	test_must_fail git merge six.1 &&
+
+	# Check that rerere knows that file1 and file2 have conflicts
+
+	printf "%s\n" file1 file2 >expect &&
+	git ls-files -u | sed -e "s/^.*	//" | sort -u >actual &&
+	test_cmp expect actual &&
+
+	git rerere status | sort >actual &&
+	test_cmp expect actual &&
+
+	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
+	concat_insert short 6.1 6.2 >file1 &&
+	concat_insert long 6.1 6.2 >file2 &&
+
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	# We resolved file1 and file2
+	git rerere &&
+	>expect &&
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	# Now we should be able to resolve them both
+	git reset --hard &&
+	test_must_fail git merge six.1 &&
+	git rerere &&
+
+	>expect &&
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	concat_insert short 6.1 6.2 >file1.expect &&
+	concat_insert long 6.1 6.2 >file2.expect &&
+	test_cmp file1.expect file1 &&
+	test_cmp file2.expect file2
+'
+
 test_done
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH 7/7] rerere: do use multiple variants

[Junio C Hamano]

This enables the multiple-variant support for real.  Multiple
conflicts of the same shape can have differences in contexts where
they appear, interfering the replaying of recorded resolution of one
conflict to another, and in such a case, their resolutions are
recorded as different variants under the same conflict ID.

We still need to adjust garbage collection codepaths for this
change, but the basic "replay" functionality is functional with
this change.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c          | 94 +++++++++++++++++++++++++++++++++----------------------
 t/t4200-rerere.sh |  2 +-
 2 files changed, 57 insertions(+), 39 deletions(-)

diff --git a/rerere.c b/rerere.c
index f674461..cefc865 100644
--- a/rerere.c
+++ b/rerere.c
@@ -74,7 +74,9 @@ static void assign_variant(struct rerere_id *id)
 
 	variant = id->variant;
 	if (variant < 0) {
-		variant = 0; /* for now */
+		for (variant = 0; variant < rr_dir->status_nr; variant++)
+			if (!rr_dir->status[variant])
+				break;
 	}
 	fit_variant(rr_dir, variant);
 	id->variant = variant;
@@ -177,15 +179,6 @@ static int has_rerere_resolution(const struct rerere_id *id)
 	return ((id->collection->status[variant] & both) == both);
 }
 
-static int has_rerere_preimage(const struct rerere_id *id)
-{
-	int variant = id->variant;
-
-	if (variant < 0)
-		return 0;
-	return (id->collection->status[variant] & RR_HAS_PREIMAGE);
-}
-
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
@@ -818,6 +811,13 @@ static void update_paths(struct string_list *update)
 		rollback_lock_file(&index_lock);
 }
 
+static void remove_variant(struct rerere_id *id)
+{
+	unlink_or_warn(rerere_path(id, "postimage"));
+	unlink_or_warn(rerere_path(id, "preimage"));
+	id->collection->status[id->variant] = 0;
+}
+
 /*
  * The path indicated by rr_item may still have conflict for which we
  * have a recorded resolution, in which case replay it and optionally
@@ -830,32 +830,42 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 {
 	const char *path = rr_item->string;
 	struct rerere_id *id = rr_item->util;
+	struct rerere_dir *rr_dir = id->collection;
 	int variant;
 
-	if (id->variant < 0)
-		assign_variant(id);
 	variant = id->variant;
 
-	if (!has_rerere_preimage(id)) {
+	/* Has the user resolved it already? */
+	if (variant >= 0) {
+		if (!handle_file(path, NULL, NULL)) {
+			copy_file(rerere_path(id, "postimage"), path, 0666);
+			id->collection->status[variant] |= RR_HAS_POSTIMAGE;
+			fprintf(stderr, "Recorded resolution for '%s'.\n", path);
+			free_rerere_id(rr_item);
+			rr_item->util = NULL;
+			return;
+		}
 		/*
-		 * We are the first to encounter this conflict.  Ask
-		 * handle_file() to write the normalized contents to
-		 * the "preimage" file.
+		 * There may be other variants that can cleanly
+		 * replay.  Try them and update the variant number for
+		 * this one.
 		 */
-		handle_file(path, NULL, rerere_path(id, "preimage"));
-		if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
-			const char *path = rerere_path(id, "postimage");
-			if (unlink(path))
-				die_errno("cannot unlink stray '%s'", path);
-			id->collection->status[variant] &= ~RR_HAS_PREIMAGE;
-		}
-		id->collection->status[variant] |= RR_HAS_PREIMAGE;
-		fprintf(stderr, "Recorded preimage for '%s'\n", path);
-		return;
-	} else if (has_rerere_resolution(id)) {
-		/* Is there a recorded resolution we could attempt to apply? */
-		if (merge(id, path))
-			return; /* failed to replay */
+	}
+
+	/* Does any existing resolution apply cleanly? */
+	for (variant = 0; variant < rr_dir->status_nr; variant++) {
+		const int both = RR_HAS_PREIMAGE | RR_HAS_POSTIMAGE;
+		struct rerere_id vid = *id;
+
+		if ((rr_dir->status[variant] & both) != both)
+			continue;
+
+		vid.variant = variant;
+		if (merge(&vid, path))
+			continue; /* failed to replay */
+
+		if (0 <= id->variant && id->variant != variant)
+			remove_variant(id);
 
 		if (rerere_autoupdate)
 			string_list_insert(update, path);
@@ -863,16 +873,24 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 			fprintf(stderr,
 				"Resolved '%s' using previous resolution.\n",
 				path);
-	} else if (!handle_file(path, NULL, NULL)) {
-		/* The user has resolved it. */
-		copy_file(rerere_path(id, "postimage"), path, 0666);
-		id->collection->status[variant] |= RR_HAS_POSTIMAGE;
-		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-	} else {
+		free_rerere_id(rr_item);
+		rr_item->util = NULL;
 		return;
 	}
-	free_rerere_id(rr_item);
-	rr_item->util = NULL;
+
+	/* None of the existing one applies; we need a new variant */
+	assign_variant(id);
+
+	variant = id->variant;
+	handle_file(path, NULL, rerere_path(id, "preimage"));
+	if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
+		const char *path = rerere_path(id, "postimage");
+		if (unlink(path))
+			die_errno("cannot unlink stray '%s'", path);
+		id->collection->status[variant] &= ~RR_HAS_PREIMAGE;
+	}
+	id->collection->status[variant] |= RR_HAS_PREIMAGE;
+	fprintf(stderr, "Recorded preimage for '%s'\n", path);
 }
 
 static int do_plain_rerere(struct string_list *rr, int fd)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 8d52854..5649829 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -413,7 +413,7 @@ concat_insert () {
 	cat early && printf "%s\n" "$@" && cat late "$last"
 }
 
-test_expect_failure 'multiple identical conflicts' '
+test_expect_success 'multiple identical conflicts' '
 	git reset --hard &&
 
 	test_seq 1 6 >early &&
-- 
2.6.0-rc2-164-gdcd5d00

[PATCH v2 00/11] saving and replaying multiple variants with rerere

[Junio C Hamano]

Take two after several months.  That is what you get when you end up
running a busy project, instead of staying to be an individual
contributor ;-).

The previous round starts at the message this is a response to and
is found at http://thread.gmane.org/gmane.comp.version-control.git/277878

The previous round completed the basic "record and replay", while
leaving "gc" and "forget" as known-to-be-stale bits.  This round
attempts to complete them. 

Junio C Hamano (11):
  rerere: split conflict ID further
  rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id
  rerere: handle leftover rr-cache/$ID directory and postimage files
  rerere: delay the recording of preimage
  rerere: allow multiple variants to exist
  t4200: rerere a merge with two identical conflicts
  rerere: do use multiple variants
  rerere: gc and clear
  rerere: move code related to "forget" together
  rerere: split code to call ll_merge() further
  rerere: adjust 'forget' to multi-variant world order

 rerere.c          | 626 +++++++++++++++++++++++++++++++++++++-----------------
 rerere.h          |   4 +-
 t/t4200-rerere.sh | 159 +++++++++++++-
 3 files changed, 593 insertions(+), 196 deletions(-)

-- 
2.8.0-215-g046a488

[PATCH v2 01/11] rerere: split conflict ID further

[Junio C Hamano]

The plan is to keep assigning the backward compatible conflict ID
based on the hash of the (normalized) text of conflicts, keep using
that conflict ID as the directory name under $GIT_DIR/rr-cache/, but
allow each conflicted path to use a separate "variant" to record
resolutions, i.e. having more than one <preimage,postimage> pairs
under $GIT_DIR/rr-cache/$ID/ directory.  As the first step in that
direction, separate the shared "conflict ID" out of the rerere_id
structure.

The plan is to keep information per $ID in rerere_dir, that can be
shared among rerere_id that is per conflicted path.

When we are done with rerere(), which can be directly called from
other programs like "git apply", "git commit" and "git merge", the
shared rerere_dir structures can be freed entirely, so they are not
reference-counted and they are not freed when we release rerere_id's
that reference them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is designed to be applied to a slightly newer codebase than
   the previous round, and a few removed lines that used to call
   strcpy() in the preimage shows up as using xsnprintf() in the
   patch.  We no longer use a simple string anyway, so the postimages
   are the same between rerolls.

 rerere.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 rerere.h |  3 ++-
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/rerere.c b/rerere.c
index 3d0fa8f..a5d8a06 100644
--- a/rerere.c
+++ b/rerere.c
@@ -8,6 +8,7 @@
 #include "ll-merge.h"
 #include "attr.h"
 #include "pathspec.h"
+#include "sha1-lookup.h"
 
 #define RESOLVED 0
 #define PUNTED 1
@@ -22,6 +23,23 @@ static int rerere_autoupdate;
 
 static char *merge_rr_path;
 
+static int rerere_dir_nr;
+static int rerere_dir_alloc;
+
+static struct rerere_dir {
+	unsigned char sha1[20];
+} **rerere_dir;
+
+static void free_rerere_dirs(void)
+{
+	int i;
+	for (i = 0; i < rerere_dir_nr; i++)
+		free(rerere_dir[i]);
+	free(rerere_dir);
+	rerere_dir_nr = rerere_dir_alloc = 0;
+	rerere_dir = NULL;
+}
+
 static void free_rerere_id(struct string_list_item *item)
 {
 	free(item->util);
@@ -29,7 +47,7 @@ static void free_rerere_id(struct string_list_item *item)
 
 static const char *rerere_id_hex(const struct rerere_id *id)
 {
-	return id->hex;
+	return sha1_to_hex(id->collection->sha1);
 }
 
 const char *rerere_path(const struct rerere_id *id, const char *file)
@@ -40,6 +58,37 @@ const char *rerere_path(const struct rerere_id *id, const char *file)
 	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
 }
 
+static const unsigned char *rerere_dir_sha1(size_t i, void *table)
+{
+	struct rerere_dir **rr_dir = table;
+	return rr_dir[i]->sha1;
+}
+
+static struct rerere_dir *find_rerere_dir(const char *hex)
+{
+	unsigned char sha1[20];
+	struct rerere_dir *rr_dir;
+	int pos;
+
+	if (get_sha1_hex(hex, sha1))
+		return NULL; /* BUG */
+	pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr, rerere_dir_sha1);
+	if (pos < 0) {
+		rr_dir = xmalloc(sizeof(*rr_dir));
+		hashcpy(rr_dir->sha1, sha1);
+		pos = -1 - pos;
+
+		/* Make sure the array is big enough ... */
+		ALLOC_GROW(rerere_dir, rerere_dir_nr + 1, rerere_dir_alloc);
+		/* ... and add it in. */
+		rerere_dir_nr++;
+		memmove(rerere_dir + pos + 1, rerere_dir + pos,
+			(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
+		rerere_dir[pos] = rr_dir;
+	}
+	return rerere_dir[pos];
+}
+
 static int has_rerere_resolution(const struct rerere_id *id)
 {
 	struct stat st;
@@ -50,7 +99,7 @@ static int has_rerere_resolution(const struct rerere_id *id)
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
-	xsnprintf(id->hex, sizeof(id->hex), "%s", hex);
+	id->collection = find_rerere_dir(hex);
 	return id;
 }
 
@@ -810,12 +859,14 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 int rerere(int flags)
 {
 	struct string_list merge_rr = STRING_LIST_INIT_DUP;
-	int fd;
+	int fd, status;
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
-	return do_plain_rerere(&merge_rr, fd);
+	status = do_plain_rerere(&merge_rr, fd);
+	free_rerere_dirs();
+	return status;
 }
 
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
@@ -900,7 +951,7 @@ int rerere_forget(struct pathspec *pathspec)
 static struct rerere_id *dirname_to_id(const char *name)
 {
 	static struct rerere_id id;
-	xsnprintf(id.hex, sizeof(id.hex), "%s", name);
+	id.collection = find_rerere_dir(name);
 	return &id;
 }
 
diff --git a/rerere.h b/rerere.h
index ce545d0..faf5a23 100644
--- a/rerere.h
+++ b/rerere.h
@@ -15,8 +15,9 @@ struct pathspec;
  */
 extern void *RERERE_RESOLVED;
 
+struct rerere_dir;
 struct rerere_id {
-	char hex[41];
+	struct rerere_dir *collection;
 };
 
 extern int setup_rerere(struct string_list *, int);
-- 
2.8.0-215-g046a488

[PATCH v2 02/11] rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id

[Junio C Hamano]

This will help fixing bootstrap corner-case issues, e.g. having an
empty $GIT_DIR/rr-cache/$ID directory would fail to record a
preimage, in later changes in this series.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Unchanged.

 rerere.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/rerere.c b/rerere.c
index a5d8a06..fbdade8 100644
--- a/rerere.c
+++ b/rerere.c
@@ -26,8 +26,11 @@ static char *merge_rr_path;
 static int rerere_dir_nr;
 static int rerere_dir_alloc;
 
+#define RR_HAS_POSTIMAGE 1
+#define RR_HAS_PREIMAGE 2
 static struct rerere_dir {
 	unsigned char sha1[20];
+	unsigned char status;
 } **rerere_dir;
 
 static void free_rerere_dirs(void)
@@ -58,6 +61,27 @@ const char *rerere_path(const struct rerere_id *id, const char *file)
 	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
 }
 
+static int is_rr_file(const char *name, const char *filename)
+{
+	return !strcmp(name, filename);
+}
+
+static void scan_rerere_dir(struct rerere_dir *rr_dir)
+{
+	struct dirent *de;
+	DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->sha1)));
+
+	if (!dir)
+		return;
+	while ((de = readdir(dir)) != NULL) {
+		if (is_rr_file(de->d_name, "postimage"))
+			rr_dir->status |= RR_HAS_POSTIMAGE;
+		else if (is_rr_file(de->d_name, "preimage"))
+			rr_dir->status |= RR_HAS_PREIMAGE;
+	}
+	closedir(dir);
+}
+
 static const unsigned char *rerere_dir_sha1(size_t i, void *table)
 {
 	struct rerere_dir **rr_dir = table;
@@ -76,6 +100,7 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 	if (pos < 0) {
 		rr_dir = xmalloc(sizeof(*rr_dir));
 		hashcpy(rr_dir->sha1, sha1);
+		rr_dir->status = 0;
 		pos = -1 - pos;
 
 		/* Make sure the array is big enough ... */
@@ -85,15 +110,14 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 		memmove(rerere_dir + pos + 1, rerere_dir + pos,
 			(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
 		rerere_dir[pos] = rr_dir;
+		scan_rerere_dir(rr_dir);
 	}
 	return rerere_dir[pos];
 }
 
 static int has_rerere_resolution(const struct rerere_id *id)
 {
-	struct stat st;
-
-	return !stat(rerere_path(id, "postimage"), &st);
+	return (id->collection->status & RR_HAS_POSTIMAGE);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -737,6 +761,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	} else if (!handle_file(path, NULL, NULL)) {
 		/* The user has resolved it. */
 		copy_file(rerere_path(id, "postimage"), path, 0666);
+		id->collection->status |= RR_HAS_POSTIMAGE;
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	} else {
 		return;
@@ -797,6 +822,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		 * normalized contents to the "preimage" file.
 		 */
 		handle_file(path, NULL, rerere_path(id, "preimage"));
+		id->collection->status |= RR_HAS_PREIMAGE;
 		fprintf(stderr, "Recorded preimage for '%s'\n", path);
 	}
 
-- 
2.8.0-215-g046a488

[PATCH v2 03/11] rerere: handle leftover rr-cache/$ID directory and postimage files

[Junio C Hamano]

If by some accident there is only $GIT_DIR/rr-cache/$ID directory
existed, we wouldn't have recorded a preimage for a conflict that
is newly encountered, which would mean after a manual resolution,
we wouldn't have recorded it by storing the postimage, because the
logic used to be "if there is no rr-cache/$ID directory, then we are
the first so record the preimage".  Instead, record preimage if we
do not have one.

In addition, if there is only $GIT_DIR/rr-cache/$ID/postimage
without corresponding preimage, we would have tried to call into
merge() and punted.

These would have been a situation frustratingly hard to recover
from.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * A test that saved away both postimage and preimage has been
   updated to only save postimage, as we do not look at the other
   one.

 rerere.c          | 42 +++++++++++++++++++++++++-----------------
 t/t4200-rerere.sh | 17 ++++++++++++++++-
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/rerere.c b/rerere.c
index fbdade8..a1e2963 100644
--- a/rerere.c
+++ b/rerere.c
@@ -117,7 +117,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 
 static int has_rerere_resolution(const struct rerere_id *id)
 {
-	return (id->collection->status & RR_HAS_POSTIMAGE);
+	const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+
+	return ((id->collection->status & both) == both);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -806,24 +808,30 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		string_list_insert(rr, path)->util = id;
 
 		/*
-		 * If the directory does not exist, create
-		 * it.  mkdir_in_gitdir() will fail with
-		 * EEXIST if there already is one.
-		 *
-		 * NEEDSWORK: make sure "gc" does not remove
-		 * preimage without removing the directory.
+		 * Ensure that the directory exists.
+		 * mkdir_in_gitdir() will fail with EEXIST if there
+		 * already is one.
 		 */
-		if (mkdir_in_gitdir(rerere_path(id, NULL)))
-			continue;
+		if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
+		    errno != EEXIST)
+			continue; /* NEEDSWORK: perhaps we should die? */
 
-		/*
-		 * We are the first to encounter this
-		 * conflict.  Ask handle_file() to write the
-		 * normalized contents to the "preimage" file.
-		 */
-		handle_file(path, NULL, rerere_path(id, "preimage"));
-		id->collection->status |= RR_HAS_PREIMAGE;
-		fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		if (id->collection->status & RR_HAS_PREIMAGE) {
+			;
+		} else {
+			/*
+			 * We are the first to encounter this
+			 * conflict.  Ask handle_file() to write the
+			 * normalized contents to the "preimage" file.
+			 *
+			 * NEEDSWORK: what should happen if we had a
+			 * leftover postimage that is totally
+			 * unrelated?  Perhaps we should unlink it?
+			 */
+			handle_file(path, NULL, rerere_path(id, "preimage"));
+			id->collection->status |= RR_HAS_PREIMAGE;
+			fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		}
 	}
 
 	for (i = 0; i < rr->nr; i++)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index ed9c91e..c428011 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -184,12 +184,27 @@ test_expect_success 'rerere updates postimage timestamp' '
 '
 
 test_expect_success 'rerere clear' '
-	rm $rr/postimage &&
+	mv $rr/postimage .git/post-saved &&
 	echo "$sha1	a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
 	git rerere clear &&
 	! test -d $rr
 '
 
+test_expect_success 'leftover directory' '
+	git reset --hard &&
+	mkdir -p $rr &&
+	test_must_fail git merge first &&
+	test -f $rr/preimage
+'
+
+test_expect_success 'missing preimage' '
+	git reset --hard &&
+	mkdir -p $rr &&
+	cp .git/post-saved $rr/postimage &&
+	test_must_fail git merge first &&
+	test -f $rr/preimage
+'
+
 test_expect_success 'set up for garbage collection tests' '
 	mkdir -p $rr &&
 	echo Hello >$rr/preimage &&
-- 
2.8.0-215-g046a488

[PATCH v2 04/11] rerere: delay the recording of preimage

[Junio C Hamano]

We record the preimage only when there is no directory to record the
conflict we encountered, i.e. when $GIT_DIR/rr-cache/$ID does not
exist.  As the plan is to allow multiple <preimage,postimage> pairs
as variants for the same conflict ID eventually, this logic needs to
go.

As the first step in that direction, stop the "did we create the
directory?  Then we record the preimage" logic.  Instead, we record
if a preimage does not exist when we saw a conflict in a path.  Also
make sure that we remove a stale postimage, which most likely is
totally unrelated to the resolution of this new conflict, when we
create a new preimage under $ID when $GIT_DIR/rr-cache/$ID already
exists.

In later patches, we will further update this logic to be "do we
have <preimage,postimage> pair that cleanly resolve the current
conflicts?  If not, record a new preimage as a new variant", but
that does not happen at this stage yet.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * The previous one had a handful of confusions between which of
   RR_HAS_{PRE,POST}IMAGE bit to clear, which was pointed out by
   J6t.  The corrections to them appear here and a few patches
   that follow this one in this round.

 rerere.c | 52 +++++++++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/rerere.c b/rerere.c
index a1e2963..33b1348 100644
--- a/rerere.c
+++ b/rerere.c
@@ -122,6 +122,11 @@ static int has_rerere_resolution(const struct rerere_id *id)
 	return ((id->collection->status & both) == both);
 }
 
+static int has_rerere_preimage(const struct rerere_id *id)
+{
+	return (id->collection->status & RR_HAS_PREIMAGE);
+}
+
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
@@ -749,8 +754,24 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	const char *path = rr_item->string;
 	const struct rerere_id *id = rr_item->util;
 
-	/* Is there a recorded resolution we could attempt to apply? */
-	if (has_rerere_resolution(id)) {
+	if (!has_rerere_preimage(id)) {
+		/*
+		 * We are the first to encounter this conflict.  Ask
+		 * handle_file() to write the normalized contents to
+		 * the "preimage" file.
+		 */
+		handle_file(path, NULL, rerere_path(id, "preimage"));
+		if (id->collection->status & RR_HAS_POSTIMAGE) {
+			const char *path = rerere_path(id, "postimage");
+			if (unlink(path))
+				die_errno("cannot unlink stray '%s'", path);
+			id->collection->status &= ~RR_HAS_POSTIMAGE;
+		}
+		id->collection->status |= RR_HAS_PREIMAGE;
+		fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		return;
+	} else if (has_rerere_resolution(id)) {
+		/* Is there a recorded resolution we could attempt to apply? */
 		if (merge(id, path))
 			return; /* failed to replay */
 
@@ -807,31 +828,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		id = new_rerere_id(sha1);
 		string_list_insert(rr, path)->util = id;
 
-		/*
-		 * Ensure that the directory exists.
-		 * mkdir_in_gitdir() will fail with EEXIST if there
-		 * already is one.
-		 */
-		if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
-		    errno != EEXIST)
-			continue; /* NEEDSWORK: perhaps we should die? */
-
-		if (id->collection->status & RR_HAS_PREIMAGE) {
-			;
-		} else {
-			/*
-			 * We are the first to encounter this
-			 * conflict.  Ask handle_file() to write the
-			 * normalized contents to the "preimage" file.
-			 *
-			 * NEEDSWORK: what should happen if we had a
-			 * leftover postimage that is totally
-			 * unrelated?  Perhaps we should unlink it?
-			 */
-			handle_file(path, NULL, rerere_path(id, "preimage"));
-			id->collection->status |= RR_HAS_PREIMAGE;
-			fprintf(stderr, "Recorded preimage for '%s'\n", path);
-		}
+		/* Ensure that the directory exists. */
+		mkdir_in_gitdir(rerere_path(id, NULL));
 	}
 
 	for (i = 0; i < rr->nr; i++)
-- 
2.8.0-215-g046a488

[PATCH v2 05/11] rerere: allow multiple variants to exist

[Junio C Hamano]

The shape of the conflict in a path determines the conflict ID.  The
preimage and postimage pair that was recorded for the conflict ID
previously may or may not replay well for the conflict we just saw.

Currently, we punt when the previous resolution does not cleanly
replay, but ideally we should then be able to record the currently
conflicted path by assigning a new 'variant', and then record the
resolution the user is going to make.

Introduce a mechanism to have more than one variant for a given
conflict ID; we do not actually assign any variant other than 0th
variant yet at this step.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 rerere.h |   1 +
 2 files changed, 106 insertions(+), 22 deletions(-)

diff --git a/rerere.c b/rerere.c
index 33b1348..0cf857b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -30,14 +30,17 @@ static int rerere_dir_alloc;
 #define RR_HAS_PREIMAGE 2
 static struct rerere_dir {
 	unsigned char sha1[20];
-	unsigned char status;
+	int status_alloc, status_nr;
+	unsigned char *status;
 } **rerere_dir;
 
 static void free_rerere_dirs(void)
 {
 	int i;
-	for (i = 0; i < rerere_dir_nr; i++)
+	for (i = 0; i < rerere_dir_nr; i++) {
+		free(rerere_dir[i]->status);
 		free(rerere_dir[i]);
+	}
 	free(rerere_dir);
 	rerere_dir_nr = rerere_dir_alloc = 0;
 	rerere_dir = NULL;
@@ -53,17 +56,59 @@ static const char *rerere_id_hex(const struct rerere_id *id)
 	return sha1_to_hex(id->collection->sha1);
 }
 
+static void fit_variant(struct rerere_dir *rr_dir, int variant)
+{
+	variant++;
+	ALLOC_GROW(rr_dir->status, variant, rr_dir->status_alloc);
+	if (rr_dir->status_nr < variant) {
+		memset(rr_dir->status + rr_dir->status_nr,
+		       '\0', variant - rr_dir->status_nr);
+		rr_dir->status_nr = variant;
+	}
+}
+
+static void assign_variant(struct rerere_id *id)
+{
+	int variant;
+	struct rerere_dir *rr_dir = id->collection;
+
+	variant = id->variant;
+	if (variant < 0) {
+		variant = 0; /* for now */
+	}
+	fit_variant(rr_dir, variant);
+	id->variant = variant;
+}
+
 const char *rerere_path(const struct rerere_id *id, const char *file)
 {
 	if (!file)
 		return git_path("rr-cache/%s", rerere_id_hex(id));
 
-	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+	if (id->variant <= 0)
+		return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+
+	return git_path("rr-cache/%s/%s.%d",
+			rerere_id_hex(id), file, id->variant);
 }
 
-static int is_rr_file(const char *name, const char *filename)
+static int is_rr_file(const char *name, const char *filename, int *variant)
 {
-	return !strcmp(name, filename);
+	const char *suffix;
+	char *ep;
+
+	if (!strcmp(name, filename)) {
+		*variant = 0;
+		return 1;
+	}
+	if (!skip_prefix(name, filename, &suffix) || *suffix != '.')
+		return 0;
+
+	errno = 0;
+	*variant = strtol(suffix + 1, &ep, 10);
+	if (errno || *ep)
+		return 0;
+	return 1;
 }
 
 static void scan_rerere_dir(struct rerere_dir *rr_dir)
@@ -74,10 +119,15 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
 	if (!dir)
 		return;
 	while ((de = readdir(dir)) != NULL) {
-		if (is_rr_file(de->d_name, "postimage"))
-			rr_dir->status |= RR_HAS_POSTIMAGE;
-		else if (is_rr_file(de->d_name, "preimage"))
-			rr_dir->status |= RR_HAS_PREIMAGE;
+		int variant;
+
+		if (is_rr_file(de->d_name, "postimage", &variant)) {
+			fit_variant(rr_dir, variant);
+			rr_dir->status[variant] |= RR_HAS_POSTIMAGE;
+		} else if (is_rr_file(de->d_name, "preimage", &variant)) {
+			fit_variant(rr_dir, variant);
+			rr_dir->status[variant] |= RR_HAS_PREIMAGE;
+		}
 	}
 	closedir(dir);
 }
@@ -100,7 +150,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 	if (pos < 0) {
 		rr_dir = xmalloc(sizeof(*rr_dir));
 		hashcpy(rr_dir->sha1, sha1);
-		rr_dir->status = 0;
+		rr_dir->status = NULL;
+		rr_dir->status_nr = 0;
+		rr_dir->status_alloc = 0;
 		pos = -1 - pos;
 
 		/* Make sure the array is big enough ... */
@@ -118,19 +170,27 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 static int has_rerere_resolution(const struct rerere_id *id)
 {
 	const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+	int variant = id->variant;
 
-	return ((id->collection->status & both) == both);
+	if (variant < 0)
+		return 0;
+	return ((id->collection->status[variant] & both) == both);
 }
 
 static int has_rerere_preimage(const struct rerere_id *id)
 {
-	return (id->collection->status & RR_HAS_PREIMAGE);
+	int variant = id->variant;
+
+	if (variant < 0)
+		return 0;
+	return (id->collection->status[variant] & RR_HAS_PREIMAGE);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
 	id->collection = find_rerere_dir(hex);
+	id->variant = -1; /* not known yet */
 	return id;
 }
 
@@ -157,16 +217,26 @@ static void read_rr(struct string_list *rr)
 		char *path;
 		unsigned char sha1[20];
 		struct rerere_id *id;
+		int variant;
 
 		/* There has to be the hash, tab, path and then NUL */
 		if (buf.len < 42 || get_sha1_hex(buf.buf, sha1))
 			die("corrupt MERGE_RR");
 
-		if (buf.buf[40] != '\t')
+		if (buf.buf[40] != '.') {
+			variant = 0;
+			path = buf.buf + 40;
+		} else {
+			errno = 0;
+			variant = strtol(buf.buf + 41, &path, 10);
+			if (errno)
+				die("corrupt MERGE_RR");
+		}
+		if (*(path++) != '\t')
 			die("corrupt MERGE_RR");
 		buf.buf[40] = '\0';
-		path = buf.buf + 41;
 		id = new_rerere_id_hex(buf.buf);
+		id->variant = variant;
 		string_list_insert(rr, path)->util = id;
 	}
 	strbuf_release(&buf);
@@ -187,9 +257,16 @@ static int write_rr(struct string_list *rr, int out_fd)
 		id = rr->items[i].util;
 		if (!id)
 			continue;
-		strbuf_addf(&buf, "%s\t%s%c",
-			    rerere_id_hex(id),
-			    rr->items[i].string, 0);
+		assert(id->variant >= 0);
+		if (0 < id->variant)
+			strbuf_addf(&buf, "%s.%d\t%s%c",
+				    rerere_id_hex(id), id->variant,
+				    rr->items[i].string, 0);
+		else
+			strbuf_addf(&buf, "%s\t%s%c",
+				    rerere_id_hex(id),
+				    rr->items[i].string, 0);
+
 		if (write_in_full(out_fd, buf.buf, buf.len) != buf.len)
 			die("unable to write rerere record");
 
@@ -752,7 +829,12 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 			       struct string_list *update)
 {
 	const char *path = rr_item->string;
-	const struct rerere_id *id = rr_item->util;
+	struct rerere_id *id = rr_item->util;
+	int variant;
+
+	if (id->variant < 0)
+		assign_variant(id);
+	variant = id->variant;
 
 	if (!has_rerere_preimage(id)) {
 		/*
@@ -761,13 +843,13 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 		 * the "preimage" file.
 		 */
 		handle_file(path, NULL, rerere_path(id, "preimage"));
-		if (id->collection->status & RR_HAS_POSTIMAGE) {
+		if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
 			const char *path = rerere_path(id, "postimage");
 			if (unlink(path))
 				die_errno("cannot unlink stray '%s'", path);
-			id->collection->status &= ~RR_HAS_POSTIMAGE;
+			id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
 		}
-		id->collection->status |= RR_HAS_PREIMAGE;
+		id->collection->status[variant] |= RR_HAS_PREIMAGE;
 		fprintf(stderr, "Recorded preimage for '%s'\n", path);
 		return;
 	} else if (has_rerere_resolution(id)) {
@@ -784,7 +866,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	} else if (!handle_file(path, NULL, NULL)) {
 		/* The user has resolved it. */
 		copy_file(rerere_path(id, "postimage"), path, 0666);
-		id->collection->status |= RR_HAS_POSTIMAGE;
+		id->collection->status[variant] |= RR_HAS_POSTIMAGE;
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	} else {
 		return;
@@ -919,6 +1001,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 
 	/* Nuke the recorded resolution for the conflict */
 	id = new_rerere_id(sha1);
+	id->variant = 0; /* for now */
 	filename = rerere_path(id, "postimage");
 	if (unlink(filename))
 		return (errno == ENOENT
diff --git a/rerere.h b/rerere.h
index faf5a23..64d38d9 100644
--- a/rerere.h
+++ b/rerere.h
@@ -18,6 +18,7 @@ extern void *RERERE_RESOLVED;
 struct rerere_dir;
 struct rerere_id {
 	struct rerere_dir *collection;
+	int variant;
 };
 
 extern int setup_rerere(struct string_list *, int);
-- 
2.8.0-215-g046a488

[PATCH v2 06/11] t4200: rerere a merge with two identical conflicts

[Junio C Hamano]

When the context of multiple identical conflicts are different, two
seemingly the same conflict resolution cannot be safely applied.

In such a case, at least we should be able to record these two
resolutions separately in the rerere database, and reuse them when
we see the same conflict later.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Unchanged

 t/t4200-rerere.sh | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index c428011..6fcc6d4 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -406,4 +406,78 @@ test_expect_success 'rerere -h' '
 	test_i18ngrep [Uu]sage help
 '
 
+concat_insert () {
+	last=$1
+	shift
+	cat early && printf "%s\n" "$@" && cat late "$last"
+}
+
+test_expect_failure 'multiple identical conflicts' '
+	git reset --hard &&
+
+	test_seq 1 6 >early &&
+	>late &&
+	test_seq 11 15 >short &&
+	test_seq 111 120 >long &&
+	concat_insert short >file1 &&
+	concat_insert long >file2 &&
+	git add file1 file2 &&
+	git commit -m base &&
+	git tag base &&
+	git checkout -b six.1 &&
+	concat_insert short 6.1 >file1 &&
+	concat_insert long 6.1 >file2 &&
+	git add file1 file2 &&
+	git commit -m 6.1 &&
+	git checkout -b six.2 HEAD^ &&
+	concat_insert short 6.2 >file1 &&
+	concat_insert long 6.2 >file2 &&
+	git add file1 file2 &&
+	git commit -m 6.2 &&
+
+	# At this point, six.1 and six.2
+	# - derive from common ancestor that has two files
+	#   1...6 7 11..15 (file1) and 1...6 7 111..120 (file2)
+	# - six.1 replaces these 7s with 6.1
+	# - six.2 replaces these 7s with 6.2
+
+	test_must_fail git merge six.1 &&
+
+	# Check that rerere knows that file1 and file2 have conflicts
+
+	printf "%s\n" file1 file2 >expect &&
+	git ls-files -u | sed -e "s/^.*	//" | sort -u >actual &&
+	test_cmp expect actual &&
+
+	git rerere status | sort >actual &&
+	test_cmp expect actual &&
+
+	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
+	concat_insert short 6.1 6.2 >file1 &&
+	concat_insert long 6.1 6.2 >file2 &&
+
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	# We resolved file1 and file2
+	git rerere &&
+	>expect &&
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	# Now we should be able to resolve them both
+	git reset --hard &&
+	test_must_fail git merge six.1 &&
+	git rerere &&
+
+	>expect &&
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	concat_insert short 6.1 6.2 >file1.expect &&
+	concat_insert long 6.1 6.2 >file2.expect &&
+	test_cmp file1.expect file1 &&
+	test_cmp file2.expect file2
+'
+
 test_done
-- 
2.8.0-215-g046a488

[PATCH v2 07/11] rerere: do use multiple variants

[Junio C Hamano]

This enables the multiple-variant support for real.  Multiple
conflicts of the same shape can have differences in contexts where
they appear, interfering the replaying of recorded resolution of one
conflict to another, and in such a case, their resolutions are
recorded as different variants under the same conflict ID.

We still need to adjust garbage collection codepaths for this
change, but the basic "replay" functionality is functional with
this change.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Mostly unchanged except for fixing the same glitch as 04/11.

 rerere.c          | 98 ++++++++++++++++++++++++++++++++++---------------------
 t/t4200-rerere.sh |  2 +-
 2 files changed, 61 insertions(+), 39 deletions(-)

diff --git a/rerere.c b/rerere.c
index 0cf857b..353227c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -74,7 +74,9 @@ static void assign_variant(struct rerere_id *id)
 
 	variant = id->variant;
 	if (variant < 0) {
-		variant = 0; /* for now */
+		for (variant = 0; variant < rr_dir->status_nr; variant++)
+			if (!rr_dir->status[variant])
+				break;
 	}
 	fit_variant(rr_dir, variant);
 	id->variant = variant;
@@ -177,15 +179,6 @@ static int has_rerere_resolution(const struct rerere_id *id)
 	return ((id->collection->status[variant] & both) == both);
 }
 
-static int has_rerere_preimage(const struct rerere_id *id)
-{
-	int variant = id->variant;
-
-	if (variant < 0)
-		return 0;
-	return (id->collection->status[variant] & RR_HAS_PREIMAGE);
-}
-
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
@@ -818,6 +811,13 @@ static void update_paths(struct string_list *update)
 		rollback_lock_file(&index_lock);
 }
 
+static void remove_variant(struct rerere_id *id)
+{
+	unlink_or_warn(rerere_path(id, "postimage"));
+	unlink_or_warn(rerere_path(id, "preimage"));
+	id->collection->status[id->variant] = 0;
+}
+
 /*
  * The path indicated by rr_item may still have conflict for which we
  * have a recorded resolution, in which case replay it and optionally
@@ -830,32 +830,46 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 {
 	const char *path = rr_item->string;
 	struct rerere_id *id = rr_item->util;
+	struct rerere_dir *rr_dir = id->collection;
 	int variant;
 
-	if (id->variant < 0)
-		assign_variant(id);
 	variant = id->variant;
 
-	if (!has_rerere_preimage(id)) {
+	/* Has the user resolved it already? */
+	if (variant >= 0) {
+		if (!handle_file(path, NULL, NULL)) {
+			copy_file(rerere_path(id, "postimage"), path, 0666);
+			id->collection->status[variant] |= RR_HAS_POSTIMAGE;
+			fprintf(stderr, "Recorded resolution for '%s'.\n", path);
+			free_rerere_id(rr_item);
+			rr_item->util = NULL;
+			return;
+		}
 		/*
-		 * We are the first to encounter this conflict.  Ask
-		 * handle_file() to write the normalized contents to
-		 * the "preimage" file.
+		 * There may be other variants that can cleanly
+		 * replay.  Try them and update the variant number for
+		 * this one.
 		 */
-		handle_file(path, NULL, rerere_path(id, "preimage"));
-		if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
-			const char *path = rerere_path(id, "postimage");
-			if (unlink(path))
-				die_errno("cannot unlink stray '%s'", path);
-			id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
-		}
-		id->collection->status[variant] |= RR_HAS_PREIMAGE;
-		fprintf(stderr, "Recorded preimage for '%s'\n", path);
-		return;
-	} else if (has_rerere_resolution(id)) {
-		/* Is there a recorded resolution we could attempt to apply? */
-		if (merge(id, path))
-			return; /* failed to replay */
+	}
+
+	/* Does any existing resolution apply cleanly? */
+	for (variant = 0; variant < rr_dir->status_nr; variant++) {
+		const int both = RR_HAS_PREIMAGE | RR_HAS_POSTIMAGE;
+		struct rerere_id vid = *id;
+
+		if ((rr_dir->status[variant] & both) != both)
+			continue;
+
+		vid.variant = variant;
+		if (merge(&vid, path))
+			continue; /* failed to replay */
+
+		/*
+		 * If there already is a different variant that applies
+		 * cleanly, there is no point maintaining our own variant.
+		 */
+		if (0 <= id->variant && id->variant != variant)
+			remove_variant(id);
 
 		if (rerere_autoupdate)
 			string_list_insert(update, path);
@@ -863,16 +877,24 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 			fprintf(stderr,
 				"Resolved '%s' using previous resolution.\n",
 				path);
-	} else if (!handle_file(path, NULL, NULL)) {
-		/* The user has resolved it. */
-		copy_file(rerere_path(id, "postimage"), path, 0666);
-		id->collection->status[variant] |= RR_HAS_POSTIMAGE;
-		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-	} else {
+		free_rerere_id(rr_item);
+		rr_item->util = NULL;
 		return;
 	}
-	free_rerere_id(rr_item);
-	rr_item->util = NULL;
+
+	/* None of the existing one applies; we need a new variant */
+	assign_variant(id);
+
+	variant = id->variant;
+	handle_file(path, NULL, rerere_path(id, "preimage"));
+	if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
+		const char *path = rerere_path(id, "postimage");
+		if (unlink(path))
+			die_errno("cannot unlink stray '%s'", path);
+		id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
+	}
+	id->collection->status[variant] |= RR_HAS_PREIMAGE;
+	fprintf(stderr, "Recorded preimage for '%s'\n", path);
 }
 
 static int do_plain_rerere(struct string_list *rr, int fd)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 6fcc6d4..b1bda20 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -412,7 +412,7 @@ concat_insert () {
 	cat early && printf "%s\n" "$@" && cat late "$last"
 }
 
-test_expect_failure 'multiple identical conflicts' '
+test_expect_success 'multiple identical conflicts' '
 	git reset --hard &&
 
 	test_seq 1 6 >early &&
-- 
2.8.0-215-g046a488

[PATCH v2 08/11] rerere: gc and clear

[Junio C Hamano]

Adjust "git rerere gc" and "git rerere clear" to the new world order
with rerere database with multiple variants for the same shape of
conflicts.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * New in v2; a preview appeared on $gmane/288454

 rerere.c          | 87 ++++++++++++++++++++++++++++++-------------------------
 t/t4200-rerere.sh | 82 ++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 123 insertions(+), 46 deletions(-)

diff --git a/rerere.c b/rerere.c
index 353227c..3d4dd33 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1081,29 +1081,16 @@ int rerere_forget(struct pathspec *pathspec)
  * Garbage collection support
  */
 
-/*
- * Note that this is not reentrant but is used only one-at-a-time
- * so it does not matter right now.
- */
-static struct rerere_id *dirname_to_id(const char *name)
-{
-	static struct rerere_id id;
-	id.collection = find_rerere_dir(name);
-	return &id;
-}
-
-static time_t rerere_created_at(const char *dir_name)
+static time_t rerere_created_at(struct rerere_id *id)
 {
 	struct stat st;
-	struct rerere_id *id = dirname_to_id(dir_name);
 
 	return stat(rerere_path(id, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static time_t rerere_last_used_at(const char *dir_name)
+static time_t rerere_last_used_at(struct rerere_id *id)
 {
 	struct stat st;
-	struct rerere_id *id = dirname_to_id(dir_name);
 
 	return stat(rerere_path(id, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
@@ -1113,15 +1100,28 @@ static time_t rerere_last_used_at(const char *dir_name)
  */
 static void unlink_rr_item(struct rerere_id *id)
 {
-	unlink(rerere_path(id, "thisimage"));
-	unlink(rerere_path(id, "preimage"));
-	unlink(rerere_path(id, "postimage"));
-	/*
-	 * NEEDSWORK: what if this rmdir() fails?  Wouldn't we then
-	 * assume that we already have preimage recorded in
-	 * do_plain_rerere()?
-	 */
-	rmdir(rerere_path(id, NULL));
+	unlink_or_warn(rerere_path(id, "thisimage"));
+	remove_variant(id);
+	id->collection->status[id->variant] = 0;
+}
+
+static void prune_one(struct rerere_id *id, time_t now,
+		      int cutoff_resolve, int cutoff_noresolve)
+{
+	time_t then;
+	int cutoff;
+
+	then = rerere_last_used_at(id);
+	if (then)
+		cutoff = cutoff_resolve;
+	else {
+		then = rerere_created_at(id);
+		if (!then)
+			return;
+		cutoff = cutoff_noresolve;
+	}
+	if (then < now - cutoff * 86400)
+		unlink_rr_item(id);
 }
 
 void rerere_gc(struct string_list *rr)
@@ -1129,8 +1129,8 @@ void rerere_gc(struct string_list *rr)
 	struct string_list to_remove = STRING_LIST_INIT_DUP;
 	DIR *dir;
 	struct dirent *e;
-	int i, cutoff;
-	time_t now = time(NULL), then;
+	int i;
+	time_t now = time(NULL);
 	int cutoff_noresolve = 15;
 	int cutoff_resolve = 60;
 
@@ -1142,25 +1142,32 @@ void rerere_gc(struct string_list *rr)
 		die_errno("unable to open rr-cache directory");
 	/* Collect stale conflict IDs ... */
 	while ((e = readdir(dir))) {
+		struct rerere_dir *rr_dir;
+		struct rerere_id id;
+		int now_empty;
+
 		if (is_dot_or_dotdot(e->d_name))
 			continue;
-
-		then = rerere_last_used_at(e->d_name);
-		if (then) {
-			cutoff = cutoff_resolve;
-		} else {
-			then = rerere_created_at(e->d_name);
-			if (!then)
-				continue;
-			cutoff = cutoff_noresolve;
+		rr_dir = find_rerere_dir(e->d_name);
+		if (!rr_dir)
+			continue; /* or should we remove e->d_name? */
+
+		now_empty = 1;
+		for (id.variant = 0, id.collection = rr_dir;
+		     id.variant < id.collection->status_nr;
+		     id.variant++) {
+			prune_one(&id, now, cutoff_resolve, cutoff_noresolve);
+			if (id.collection->status[id.variant])
+				now_empty = 0;
 		}
-		if (then < now - cutoff * 86400)
+		if (now_empty)
 			string_list_append(&to_remove, e->d_name);
 	}
 	closedir(dir);
-	/* ... and then remove them one-by-one */
+
+	/* ... and then remove the empty directories */
 	for (i = 0; i < to_remove.nr; i++)
-		unlink_rr_item(dirname_to_id(to_remove.items[i].string));
+		rmdir(git_path("rr-cache/%s", to_remove.items[i].string));
 	string_list_clear(&to_remove, 0);
 }
 
@@ -1177,8 +1184,10 @@ void rerere_clear(struct string_list *merge_rr)
 
 	for (i = 0; i < merge_rr->nr; i++) {
 		struct rerere_id *id = merge_rr->items[i].util;
-		if (!has_rerere_resolution(id))
+		if (!has_rerere_resolution(id)) {
 			unlink_rr_item(id);
+			rmdir(rerere_path(id, NULL));
+		}
 	}
 	unlink_or_warn(git_path("MERGE_RR"));
 }
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index b1bda20..647ea6f 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -412,6 +412,39 @@ concat_insert () {
 	cat early && printf "%s\n" "$@" && cat late "$last"
 }
 
+count_pre_post () {
+	find .git/rr-cache/ -type f -name "preimage*" >actual &&
+	test_line_count = "$1" actual &&
+	find .git/rr-cache/ -type f -name "postimage*" >actual &&
+	test_line_count = "$2" actual
+}
+
+test_expect_success 'rerere gc' '
+	find .git/rr-cache -type f >original &&
+	xargs test-chmtime -172800 <original &&
+
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+	find .git/rr-cache -type f >actual &&
+	test_cmp original actual &&
+
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc &&
+	find .git/rr-cache -type f >actual &&
+	test_cmp original actual &&
+
+	git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc &&
+	find .git/rr-cache -type f >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+merge_conflict_resolve () {
+	git reset --hard &&
+	test_must_fail git merge six.1 &&
+	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
+	concat_insert short 6.1 6.2 >file1 &&
+	concat_insert long 6.1 6.2 >file2
+}
+
 test_expect_success 'multiple identical conflicts' '
 	git reset --hard &&
 
@@ -441,10 +474,9 @@ test_expect_success 'multiple identical conflicts' '
 	# - six.1 replaces these 7s with 6.1
 	# - six.2 replaces these 7s with 6.2
 
-	test_must_fail git merge six.1 &&
+	merge_conflict_resolve &&
 
 	# Check that rerere knows that file1 and file2 have conflicts
-
 	printf "%s\n" file1 file2 >expect &&
 	git ls-files -u | sed -e "s/^.*	//" | sort -u >actual &&
 	test_cmp expect actual &&
@@ -452,19 +484,43 @@ test_expect_success 'multiple identical conflicts' '
 	git rerere status | sort >actual &&
 	test_cmp expect actual &&
 
-	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
-	concat_insert short 6.1 6.2 >file1 &&
-	concat_insert long 6.1 6.2 >file2 &&
-
 	git rerere remaining >actual &&
 	test_cmp expect actual &&
 
+	count_pre_post 2 0 &&
+
+	# Pretend that the conflicts were made quite some time ago
+	find .git/rr-cache/ -type f | xargs test-chmtime -172800 &&
+
+	# Unresolved entries have not expired yet
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+	count_pre_post 2 0 &&
+
+	# Unresolved entries have expired
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=1 rerere gc &&
+	count_pre_post 0 0 &&
+
+	# Recreate the conflicted state
+	merge_conflict_resolve &&
+	count_pre_post 2 0 &&
+
+	# Clear it
+	git rerere clear &&
+	count_pre_post 0 0 &&
+
+	# Recreate the conflicted state
+	merge_conflict_resolve &&
+	count_pre_post 2 0 &&
+
 	# We resolved file1 and file2
 	git rerere &&
 	>expect &&
 	git rerere remaining >actual &&
 	test_cmp expect actual &&
 
+	# We must have recorded both of them
+	count_pre_post 2 2 &&
+
 	# Now we should be able to resolve them both
 	git reset --hard &&
 	test_must_fail git merge six.1 &&
@@ -477,7 +533,19 @@ test_expect_success 'multiple identical conflicts' '
 	concat_insert short 6.1 6.2 >file1.expect &&
 	concat_insert long 6.1 6.2 >file2.expect &&
 	test_cmp file1.expect file1 &&
-	test_cmp file2.expect file2
+	test_cmp file2.expect file2 &&
+
+	# Pretend again
+	find .git/rr-cache/ -type f | xargs test-chmtime -172800 &&
+
+	# Resolved entries have not expired yet
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+
+	count_pre_post 2 2 &&
+
+	# Resolved entries have expired
+	git -c gc.rerereresolved=1 -c gc.rerereunresolved=5 rerere gc &&
+	count_pre_post 0 0
 '
 
 test_done
-- 
2.8.0-215-g046a488

[PATCH v2 09/11] rerere: move code related to "forget" together

[Junio C Hamano]

"rerere forget" is the only user of handle_cache() helper, which in
turn is the only user of rerere_io that reads from an in-core buffer
whose getline method is implemented as rerere_mem_getline().  Gather
them together.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * New in v2.

 rerere.c | 194 +++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 97 insertions(+), 97 deletions(-)

diff --git a/rerere.c b/rerere.c
index 3d4dd33..2b870e0 100644
--- a/rerere.c
+++ b/rerere.c
@@ -517,103 +517,6 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output
 }
 
 /*
- * Subclass of rerere_io that reads from an in-core buffer that is a
- * strbuf
- */
-struct rerere_io_mem {
-	struct rerere_io io;
-	struct strbuf input;
-};
-
-/*
- * ... and its getline() method implementation
- */
-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 = memchr(io->input.buf, '\n', io->input.len);
-	if (!ep)
-		ep = io->input.buf + io->input.len;
-	else 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 int handle_cache(const char *path, unsigned char *sha1, const char *output)
-{
-	mmfile_t mmfile[3] = {{NULL}};
-	mmbuffer_t result = {NULL, 0};
-	const struct cache_entry *ce;
-	int pos, len, i, hunk_no;
-	struct rerere_io_mem io;
-	int marker_size = ll_merge_marker_size(path);
-
-	/*
-	 * Reproduce the conflicted merge in-core
-	 */
-	len = strlen(path);
-	pos = cache_name_pos(path, len);
-	if (0 <= pos)
-		return -1;
-	pos = -pos - 1;
-
-	while (pos < active_nr) {
-		enum object_type type;
-		unsigned long size;
-
-		ce = active_cache[pos++];
-		if (ce_namelen(ce) != len || memcmp(ce->name, path, len))
-			break;
-		i = ce_stage(ce) - 1;
-		if (!mmfile[i].ptr) {
-			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("");
-
-	/*
-	 * NEEDSWORK: handle conflicts from merges with
-	 * merge.renormalize set, too
-	 */
-	ll_merge(&result, path, &mmfile[0], NULL,
-		 &mmfile[1], "ours",
-		 &mmfile[2], "theirs", NULL);
-	for (i = 0; i < 3; i++)
-		free(mmfile[i].ptr);
-
-	memset(&io, 0, sizeof(io));
-	io.io.getline = rerere_mem_getline;
-	if (output)
-		io.io.output = fopen(output, "w");
-	else
-		io.io.output = NULL;
-	strbuf_init(&io.input, 0);
-	strbuf_attach(&io.input, result.ptr, result.size, result.size);
-
-	/*
-	 * Grab the conflict ID and optionally write the original
-	 * contents with conflict markers out.
-	 */
-	hunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);
-	strbuf_release(&io.input);
-	if (io.io.output)
-		fclose(io.io.output);
-	return hunk_no;
-}
-
-/*
  * Look at a cache entry at "i" and see if it is not conflicting,
  * conflicting and we are willing to handle, or conflicting and
  * we are unable to handle, and return the determination in *type.
@@ -1005,6 +908,103 @@ int rerere(int flags)
 	return status;
 }
 
+/*
+ * Subclass of rerere_io that reads from an in-core buffer that is a
+ * strbuf
+ */
+struct rerere_io_mem {
+	struct rerere_io io;
+	struct strbuf input;
+};
+
+/*
+ * ... and its getline() method implementation
+ */
+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 = memchr(io->input.buf, '\n', io->input.len);
+	if (!ep)
+		ep = io->input.buf + io->input.len;
+	else 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 int handle_cache(const char *path, unsigned char *sha1, const char *output)
+{
+	mmfile_t mmfile[3] = {{NULL}};
+	mmbuffer_t result = {NULL, 0};
+	const struct cache_entry *ce;
+	int pos, len, i, hunk_no;
+	struct rerere_io_mem io;
+	int marker_size = ll_merge_marker_size(path);
+
+	/*
+	 * Reproduce the conflicted merge in-core
+	 */
+	len = strlen(path);
+	pos = cache_name_pos(path, len);
+	if (0 <= pos)
+		return -1;
+	pos = -pos - 1;
+
+	while (pos < active_nr) {
+		enum object_type type;
+		unsigned long size;
+
+		ce = active_cache[pos++];
+		if (ce_namelen(ce) != len || memcmp(ce->name, path, len))
+			break;
+		i = ce_stage(ce) - 1;
+		if (!mmfile[i].ptr) {
+			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("");
+
+	/*
+	 * NEEDSWORK: handle conflicts from merges with
+	 * merge.renormalize set, too?
+	 */
+	ll_merge(&result, path, &mmfile[0], NULL,
+		 &mmfile[1], "ours",
+		 &mmfile[2], "theirs", NULL);
+	for (i = 0; i < 3; i++)
+		free(mmfile[i].ptr);
+
+	memset(&io, 0, sizeof(io));
+	io.io.getline = rerere_mem_getline;
+	if (output)
+		io.io.output = fopen(output, "w");
+	else
+		io.io.output = NULL;
+	strbuf_init(&io.input, 0);
+	strbuf_attach(&io.input, result.ptr, result.size, result.size);
+
+	/*
+	 * Grab the conflict ID and optionally write the original
+	 * contents with conflict markers out.
+	 */
+	hunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);
+	strbuf_release(&io.input);
+	if (io.io.output)
+		fclose(io.io.output);
+	return hunk_no;
+}
+
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
 {
 	const char *filename;
-- 
2.8.0-215-g046a488

[PATCH v2 10/11] rerere: split code to call ll_merge() further

[Junio C Hamano]

The merge() helper function is given an existing rerere ID (i.e. the
name of the .git/rr-cache/* subdirectory, and the variant number)
that identifies one <preimage, postimage> pair, try to see if the
conflicted state in the given path can be resolved by using the pair,
and if this succeeds, then update the conflicted path with the
result in the working tree.

To implement rerere_forget() in the multiple variant world, we'd
need a helper to do the "see if a <preimage, postimage> pair cleanly
resolves a conflicted state we have in-core" part, without actually
touching any file in the working tree, in order to identify which
variant(s) to remove.  Split the logic to do so into a separate
helper function try_merge() out of merge().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * New in v2.

 rerere.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/rerere.c b/rerere.c
index 2b870e0..e636d4b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -622,6 +622,33 @@ int rerere_remaining(struct string_list *merge_rr)
 }
 
 /*
+ * Try using the given conflict resolution "ID" to see
+ * if that recorded conflict resolves cleanly what we
+ * got in the "cur".
+ */
+static int try_merge(const struct rerere_id *id, const char *path,
+		     mmfile_t *cur, mmbuffer_t *result)
+{
+	int ret;
+	mmfile_t base = {NULL, 0}, other = {NULL, 0};
+
+	if (read_mmfile(&base, rerere_path(id, "preimage")) ||
+	    read_mmfile(&other, rerere_path(id, "postimage")))
+		ret = 1;
+	else
+		/*
+		 * A three-way merge. Note that this honors user-customizable
+		 * low-level merge driver settings.
+		 */
+		ret = ll_merge(result, path, &base, NULL, cur, "", &other, "", NULL);
+
+	free(base.ptr);
+	free(other.ptr);
+
+	return ret;
+}
+
+/*
  * Find the conflict identified by "id"; the change between its
  * "preimage" (i.e. a previous contents with conflict markers) and its
  * "postimage" (i.e. the corresponding contents with conflicts
@@ -635,30 +662,20 @@ static int merge(const struct rerere_id *id, const char *path)
 {
 	FILE *f;
 	int ret;
-	mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};
+	mmfile_t cur = {NULL, 0};
 	mmbuffer_t result = {NULL, 0};
 
 	/*
 	 * Normalize the conflicts in path and write it out to
 	 * "thisimage" temporary file.
 	 */
-	if (handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) {
-		ret = 1;
-		goto out;
-	}
-
-	if (read_mmfile(&cur, rerere_path(id, "thisimage")) ||
-	    read_mmfile(&base, rerere_path(id, "preimage")) ||
-	    read_mmfile(&other, rerere_path(id, "postimage"))) {
+	if ((handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) ||
+	    read_mmfile(&cur, rerere_path(id, "thisimage"))) {
 		ret = 1;
 		goto out;
 	}
 
-	/*
-	 * A three-way merge. Note that this honors user-customizable
-	 * low-level merge driver settings.
-	 */
-	ret = ll_merge(&result, path, &base, NULL, &cur, "", &other, "", NULL);
+	ret = try_merge(id, path, &cur, &result);
 	if (ret)
 		goto out;
 
@@ -684,8 +701,6 @@ static int merge(const struct rerere_id *id, const char *path)
 
 out:
 	free(cur.ptr);
-	free(base.ptr);
-	free(other.ptr);
 	free(result.ptr);
 
 	return ret;
-- 
2.8.0-215-g046a488

[PATCH v2 11/11] rerere: adjust 'forget' to multi-variant world order

[Junio C Hamano]

Because conflicts with the same contents inside conflict blocks
enclosed by "<<<<<<<" and ">>>>>>>" can now have multiple variants
to help three-way merge to adjust to the differences outside the
conflict blocks, "rerere forget $path" needs to be taught that there
may be multiple recorded resolutions that share the same conflict
hash (which groups the conflicts with "the same contents inside
conflict blocks"), among which there are some that would not be
relevant to the conflict we are looking at.  These "other variants"
that happen to share the same conflict hash should not be cleared,
and the variant that would apply to the current conflict may not be
the zero-th one (which is the only one that is cleared by the
current code).

After finding the conflict hash, iterate over the existing variants
and try to resolve the conflict using each of them to find the one
that "cleanly" resolves the current conflict.  That is the one we
want to forget and record the preimage for, so that the user can
record the corrected resolution.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * New in v2.  I _think_ we should refactor the code further in
   order to avoid rewriting "thisimage", "thisimage.1", etc. on the
   filesystem (instead, teach handle_path() to output to an in-core
   buffer and keep it around during this loop), and doing the same
   handle_cache() over and over again, but for now this should do.

 rerere.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/rerere.c b/rerere.c
index e636d4b..1693866 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1038,7 +1038,33 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 
 	/* Nuke the recorded resolution for the conflict */
 	id = new_rerere_id(sha1);
-	id->variant = 0; /* for now */
+
+	for (id->variant = 0;
+	     id->variant < id->collection->status_nr;
+	     id->variant++) {
+		mmfile_t cur = { NULL, 0 };
+		mmbuffer_t result = {NULL, 0};
+		int cleanly_resolved;
+
+		if (!has_rerere_resolution(id))
+			continue;
+
+		handle_cache(path, sha1, rerere_path(id, "thisimage"));
+		if (read_mmfile(&cur, rerere_path(id, "thisimage"))) {
+			free(cur.ptr);
+			return error("Failed to update conflicted state in '%s'",
+				     path);
+		}
+		cleanly_resolved = !try_merge(id, path, &cur, &result);
+		free(result.ptr);
+		free(cur.ptr);
+		if (cleanly_resolved)
+			break;
+	}
+
+	if (id->collection->status_nr <= id->variant)
+		return error("no remembered resolution for '%s'", path);
+
 	filename = rerere_path(id, "postimage");
 	if (unlink(filename))
 		return (errno == ENOENT
-- 
2.8.0-215-g046a488

[PATCH v3 00/11] saving and replaying multiple variants with rerere

[Junio C Hamano]

This adds only finishing touches compared to the previous one, which
starts at the message this is a response to and is found at
http://thread.gmane.org/gmane.comp.version-control.git/290068.

Namely, the last patch is different in that it not just adjusts
'forget', but also has a new test for 'rerere forget'.

Junio C Hamano (11):
  rerere: split conflict ID further
  rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id
  rerere: handle leftover rr-cache/$ID directory and postimage files
  rerere: delay the recording of preimage
  rerere: allow multiple variants to exist
  t4200: rerere a merge with two identical conflicts
  rerere: do use multiple variants
  rerere: gc and clear
  rerere: move code related to "forget" together
  rerere: split code to call ll_merge() further
  rerere: adjust 'forget' to multi-variant world order

 rerere.c          | 626 +++++++++++++++++++++++++++++++++++++-----------------
 rerere.h          |   4 +-
 t/t4200-rerere.sh | 170 ++++++++++++++-
 3 files changed, 604 insertions(+), 196 deletions(-)

-- 
2.8.1-273-ga2cd0f9


 t/t4200-rerere.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 647ea6f..1a080e7 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -477,6 +477,7 @@ test_expect_success 'multiple identical conflicts' '
 	merge_conflict_resolve &&
 
 	# Check that rerere knows that file1 and file2 have conflicts
+
 	printf "%s\n" file1 file2 >expect &&
 	git ls-files -u | sed -e "s/^.*	//" | sort -u >actual &&
 	test_cmp expect actual &&
@@ -535,7 +536,17 @@ test_expect_success 'multiple identical conflicts' '
 	test_cmp file1.expect file1 &&
 	test_cmp file2.expect file2 &&
 
-	# Pretend again
+	# Forget resolution for file2
+	git rerere forget file2 &&
+	echo file2 >expect &&
+	git rerere status >actual &&
+	test_cmp expect actual &&
+	count_pre_post 2 1 &&
+
+	# file2 already has correct resolution, so record it again
+	git rerere &&
+
+	# Pretend that the resolutions are old again
 	find .git/rr-cache/ -type f | xargs test-chmtime -172800 &&
 
 	# Resolved entries have not expired yet

[PATCH v3 01/11] rerere: split conflict ID further

[Junio C Hamano]

The plan is to keep assigning the backward compatible conflict ID
based on the hash of the (normalized) text of conflicts, keep using
that conflict ID as the directory name under $GIT_DIR/rr-cache/, but
allow each conflicted path to use a separate "variant" to record
resolutions, i.e. having more than one <preimage,postimage> pairs
under $GIT_DIR/rr-cache/$ID/ directory.  As the first step in that
direction, separate the shared "conflict ID" out of the rerere_id
structure.

The plan is to keep information per $ID in rerere_dir, that can be
shared among rerere_id that is per conflicted path.

When we are done with rerere(), which can be directly called from
other programs like "git apply", "git commit" and "git merge", the
shared rerere_dir structures can be freed entirely, so they are not
reference-counted and they are not freed when we release rerere_id's
that reference them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 rerere.h |  3 ++-
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/rerere.c b/rerere.c
index 3d0fa8f..a5d8a06 100644
--- a/rerere.c
+++ b/rerere.c
@@ -8,6 +8,7 @@
 #include "ll-merge.h"
 #include "attr.h"
 #include "pathspec.h"
+#include "sha1-lookup.h"
 
 #define RESOLVED 0
 #define PUNTED 1
@@ -22,6 +23,23 @@ static int rerere_autoupdate;
 
 static char *merge_rr_path;
 
+static int rerere_dir_nr;
+static int rerere_dir_alloc;
+
+static struct rerere_dir {
+	unsigned char sha1[20];
+} **rerere_dir;
+
+static void free_rerere_dirs(void)
+{
+	int i;
+	for (i = 0; i < rerere_dir_nr; i++)
+		free(rerere_dir[i]);
+	free(rerere_dir);
+	rerere_dir_nr = rerere_dir_alloc = 0;
+	rerere_dir = NULL;
+}
+
 static void free_rerere_id(struct string_list_item *item)
 {
 	free(item->util);
@@ -29,7 +47,7 @@ static void free_rerere_id(struct string_list_item *item)
 
 static const char *rerere_id_hex(const struct rerere_id *id)
 {
-	return id->hex;
+	return sha1_to_hex(id->collection->sha1);
 }
 
 const char *rerere_path(const struct rerere_id *id, const char *file)
@@ -40,6 +58,37 @@ const char *rerere_path(const struct rerere_id *id, const char *file)
 	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
 }
 
+static const unsigned char *rerere_dir_sha1(size_t i, void *table)
+{
+	struct rerere_dir **rr_dir = table;
+	return rr_dir[i]->sha1;
+}
+
+static struct rerere_dir *find_rerere_dir(const char *hex)
+{
+	unsigned char sha1[20];
+	struct rerere_dir *rr_dir;
+	int pos;
+
+	if (get_sha1_hex(hex, sha1))
+		return NULL; /* BUG */
+	pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr, rerere_dir_sha1);
+	if (pos < 0) {
+		rr_dir = xmalloc(sizeof(*rr_dir));
+		hashcpy(rr_dir->sha1, sha1);
+		pos = -1 - pos;
+
+		/* Make sure the array is big enough ... */
+		ALLOC_GROW(rerere_dir, rerere_dir_nr + 1, rerere_dir_alloc);
+		/* ... and add it in. */
+		rerere_dir_nr++;
+		memmove(rerere_dir + pos + 1, rerere_dir + pos,
+			(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
+		rerere_dir[pos] = rr_dir;
+	}
+	return rerere_dir[pos];
+}
+
 static int has_rerere_resolution(const struct rerere_id *id)
 {
 	struct stat st;
@@ -50,7 +99,7 @@ static int has_rerere_resolution(const struct rerere_id *id)
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
-	xsnprintf(id->hex, sizeof(id->hex), "%s", hex);
+	id->collection = find_rerere_dir(hex);
 	return id;
 }
 
@@ -810,12 +859,14 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 int rerere(int flags)
 {
 	struct string_list merge_rr = STRING_LIST_INIT_DUP;
-	int fd;
+	int fd, status;
 
 	fd = setup_rerere(&merge_rr, flags);
 	if (fd < 0)
 		return 0;
-	return do_plain_rerere(&merge_rr, fd);
+	status = do_plain_rerere(&merge_rr, fd);
+	free_rerere_dirs();
+	return status;
 }
 
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
@@ -900,7 +951,7 @@ int rerere_forget(struct pathspec *pathspec)
 static struct rerere_id *dirname_to_id(const char *name)
 {
 	static struct rerere_id id;
-	xsnprintf(id.hex, sizeof(id.hex), "%s", name);
+	id.collection = find_rerere_dir(name);
 	return &id;
 }
 
diff --git a/rerere.h b/rerere.h
index ce545d0..faf5a23 100644
--- a/rerere.h
+++ b/rerere.h
@@ -15,8 +15,9 @@ struct pathspec;
  */
 extern void *RERERE_RESOLVED;
 
+struct rerere_dir;
 struct rerere_id {
-	char hex[41];
+	struct rerere_dir *collection;
 };
 
 extern int setup_rerere(struct string_list *, int);
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 02/11] rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id

[Junio C Hamano]

This will help fixing bootstrap corner-case issues, e.g. having an
empty $GIT_DIR/rr-cache/$ID directory would fail to record a
preimage, in later changes in this series.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/rerere.c b/rerere.c
index a5d8a06..fbdade8 100644
--- a/rerere.c
+++ b/rerere.c
@@ -26,8 +26,11 @@ static char *merge_rr_path;
 static int rerere_dir_nr;
 static int rerere_dir_alloc;
 
+#define RR_HAS_POSTIMAGE 1
+#define RR_HAS_PREIMAGE 2
 static struct rerere_dir {
 	unsigned char sha1[20];
+	unsigned char status;
 } **rerere_dir;
 
 static void free_rerere_dirs(void)
@@ -58,6 +61,27 @@ const char *rerere_path(const struct rerere_id *id, const char *file)
 	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
 }
 
+static int is_rr_file(const char *name, const char *filename)
+{
+	return !strcmp(name, filename);
+}
+
+static void scan_rerere_dir(struct rerere_dir *rr_dir)
+{
+	struct dirent *de;
+	DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->sha1)));
+
+	if (!dir)
+		return;
+	while ((de = readdir(dir)) != NULL) {
+		if (is_rr_file(de->d_name, "postimage"))
+			rr_dir->status |= RR_HAS_POSTIMAGE;
+		else if (is_rr_file(de->d_name, "preimage"))
+			rr_dir->status |= RR_HAS_PREIMAGE;
+	}
+	closedir(dir);
+}
+
 static const unsigned char *rerere_dir_sha1(size_t i, void *table)
 {
 	struct rerere_dir **rr_dir = table;
@@ -76,6 +100,7 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 	if (pos < 0) {
 		rr_dir = xmalloc(sizeof(*rr_dir));
 		hashcpy(rr_dir->sha1, sha1);
+		rr_dir->status = 0;
 		pos = -1 - pos;
 
 		/* Make sure the array is big enough ... */
@@ -85,15 +110,14 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 		memmove(rerere_dir + pos + 1, rerere_dir + pos,
 			(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));
 		rerere_dir[pos] = rr_dir;
+		scan_rerere_dir(rr_dir);
 	}
 	return rerere_dir[pos];
 }
 
 static int has_rerere_resolution(const struct rerere_id *id)
 {
-	struct stat st;
-
-	return !stat(rerere_path(id, "postimage"), &st);
+	return (id->collection->status & RR_HAS_POSTIMAGE);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -737,6 +761,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	} else if (!handle_file(path, NULL, NULL)) {
 		/* The user has resolved it. */
 		copy_file(rerere_path(id, "postimage"), path, 0666);
+		id->collection->status |= RR_HAS_POSTIMAGE;
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	} else {
 		return;
@@ -797,6 +822,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		 * normalized contents to the "preimage" file.
 		 */
 		handle_file(path, NULL, rerere_path(id, "preimage"));
+		id->collection->status |= RR_HAS_PREIMAGE;
 		fprintf(stderr, "Recorded preimage for '%s'\n", path);
 	}
 
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 03/11] rerere: handle leftover rr-cache/$ID directory and postimage files

[Junio C Hamano]

If by some accident there is only $GIT_DIR/rr-cache/$ID directory
existed, we wouldn't have recorded a preimage for a conflict that
is newly encountered, which would mean after a manual resolution,
we wouldn't have recorded it by storing the postimage, because the
logic used to be "if there is no rr-cache/$ID directory, then we are
the first so record the preimage".  Instead, record preimage if we
do not have one.

In addition, if there is only $GIT_DIR/rr-cache/$ID/postimage
without corresponding preimage, we would have tried to call into
merge() and punted.

These would have been a situation frustratingly hard to recover
from.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c          | 42 +++++++++++++++++++++++++-----------------
 t/t4200-rerere.sh | 17 ++++++++++++++++-
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/rerere.c b/rerere.c
index fbdade8..a1e2963 100644
--- a/rerere.c
+++ b/rerere.c
@@ -117,7 +117,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 
 static int has_rerere_resolution(const struct rerere_id *id)
 {
-	return (id->collection->status & RR_HAS_POSTIMAGE);
+	const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+
+	return ((id->collection->status & both) == both);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
@@ -806,24 +808,30 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		string_list_insert(rr, path)->util = id;
 
 		/*
-		 * If the directory does not exist, create
-		 * it.  mkdir_in_gitdir() will fail with
-		 * EEXIST if there already is one.
-		 *
-		 * NEEDSWORK: make sure "gc" does not remove
-		 * preimage without removing the directory.
+		 * Ensure that the directory exists.
+		 * mkdir_in_gitdir() will fail with EEXIST if there
+		 * already is one.
 		 */
-		if (mkdir_in_gitdir(rerere_path(id, NULL)))
-			continue;
+		if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
+		    errno != EEXIST)
+			continue; /* NEEDSWORK: perhaps we should die? */
 
-		/*
-		 * We are the first to encounter this
-		 * conflict.  Ask handle_file() to write the
-		 * normalized contents to the "preimage" file.
-		 */
-		handle_file(path, NULL, rerere_path(id, "preimage"));
-		id->collection->status |= RR_HAS_PREIMAGE;
-		fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		if (id->collection->status & RR_HAS_PREIMAGE) {
+			;
+		} else {
+			/*
+			 * We are the first to encounter this
+			 * conflict.  Ask handle_file() to write the
+			 * normalized contents to the "preimage" file.
+			 *
+			 * NEEDSWORK: what should happen if we had a
+			 * leftover postimage that is totally
+			 * unrelated?  Perhaps we should unlink it?
+			 */
+			handle_file(path, NULL, rerere_path(id, "preimage"));
+			id->collection->status |= RR_HAS_PREIMAGE;
+			fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		}
 	}
 
 	for (i = 0; i < rr->nr; i++)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index ed9c91e..c428011 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -184,12 +184,27 @@ test_expect_success 'rerere updates postimage timestamp' '
 '
 
 test_expect_success 'rerere clear' '
-	rm $rr/postimage &&
+	mv $rr/postimage .git/post-saved &&
 	echo "$sha1	a1" | perl -pe "y/\012/\000/" >.git/MERGE_RR &&
 	git rerere clear &&
 	! test -d $rr
 '
 
+test_expect_success 'leftover directory' '
+	git reset --hard &&
+	mkdir -p $rr &&
+	test_must_fail git merge first &&
+	test -f $rr/preimage
+'
+
+test_expect_success 'missing preimage' '
+	git reset --hard &&
+	mkdir -p $rr &&
+	cp .git/post-saved $rr/postimage &&
+	test_must_fail git merge first &&
+	test -f $rr/preimage
+'
+
 test_expect_success 'set up for garbage collection tests' '
 	mkdir -p $rr &&
 	echo Hello >$rr/preimage &&
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 04/11] rerere: delay the recording of preimage

[Junio C Hamano]

We record the preimage only when there is no directory to record the
conflict we encountered, i.e. when $GIT_DIR/rr-cache/$ID does not
exist.  As the plan is to allow multiple <preimage,postimage> pairs
as variants for the same conflict ID eventually, this logic needs to
go.

As the first step in that direction, stop the "did we create the
directory?  Then we record the preimage" logic.  Instead, we record
if a preimage does not exist when we saw a conflict in a path.  Also
make sure that we remove a stale postimage, which most likely is
totally unrelated to the resolution of this new conflict, when we
create a new preimage under $ID when $GIT_DIR/rr-cache/$ID already
exists.

In later patches, we will further update this logic to be "do we
have <preimage,postimage> pair that cleanly resolve the current
conflicts?  If not, record a new preimage as a new variant", but
that does not happen at this stage yet.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 52 +++++++++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/rerere.c b/rerere.c
index a1e2963..33b1348 100644
--- a/rerere.c
+++ b/rerere.c
@@ -122,6 +122,11 @@ static int has_rerere_resolution(const struct rerere_id *id)
 	return ((id->collection->status & both) == both);
 }
 
+static int has_rerere_preimage(const struct rerere_id *id)
+{
+	return (id->collection->status & RR_HAS_PREIMAGE);
+}
+
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
@@ -749,8 +754,24 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	const char *path = rr_item->string;
 	const struct rerere_id *id = rr_item->util;
 
-	/* Is there a recorded resolution we could attempt to apply? */
-	if (has_rerere_resolution(id)) {
+	if (!has_rerere_preimage(id)) {
+		/*
+		 * We are the first to encounter this conflict.  Ask
+		 * handle_file() to write the normalized contents to
+		 * the "preimage" file.
+		 */
+		handle_file(path, NULL, rerere_path(id, "preimage"));
+		if (id->collection->status & RR_HAS_POSTIMAGE) {
+			const char *path = rerere_path(id, "postimage");
+			if (unlink(path))
+				die_errno("cannot unlink stray '%s'", path);
+			id->collection->status &= ~RR_HAS_POSTIMAGE;
+		}
+		id->collection->status |= RR_HAS_PREIMAGE;
+		fprintf(stderr, "Recorded preimage for '%s'\n", path);
+		return;
+	} else if (has_rerere_resolution(id)) {
+		/* Is there a recorded resolution we could attempt to apply? */
 		if (merge(id, path))
 			return; /* failed to replay */
 
@@ -807,31 +828,8 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		id = new_rerere_id(sha1);
 		string_list_insert(rr, path)->util = id;
 
-		/*
-		 * Ensure that the directory exists.
-		 * mkdir_in_gitdir() will fail with EEXIST if there
-		 * already is one.
-		 */
-		if (mkdir_in_gitdir(rerere_path(id, NULL)) &&
-		    errno != EEXIST)
-			continue; /* NEEDSWORK: perhaps we should die? */
-
-		if (id->collection->status & RR_HAS_PREIMAGE) {
-			;
-		} else {
-			/*
-			 * We are the first to encounter this
-			 * conflict.  Ask handle_file() to write the
-			 * normalized contents to the "preimage" file.
-			 *
-			 * NEEDSWORK: what should happen if we had a
-			 * leftover postimage that is totally
-			 * unrelated?  Perhaps we should unlink it?
-			 */
-			handle_file(path, NULL, rerere_path(id, "preimage"));
-			id->collection->status |= RR_HAS_PREIMAGE;
-			fprintf(stderr, "Recorded preimage for '%s'\n", path);
-		}
+		/* Ensure that the directory exists. */
+		mkdir_in_gitdir(rerere_path(id, NULL));
 	}
 
 	for (i = 0; i < rr->nr; i++)
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 05/11] rerere: allow multiple variants to exist

[Junio C Hamano]

The shape of the conflict in a path determines the conflict ID.  The
preimage and postimage pair that was recorded for the conflict ID
previously may or may not replay well for the conflict we just saw.

Currently, we punt when the previous resolution does not cleanly
replay, but ideally we should then be able to record the currently
conflicted path by assigning a new 'variant', and then record the
resolution the user is going to make.

Introduce a mechanism to have more than one variant for a given
conflict ID; we do not actually assign any variant other than 0th
variant yet at this step.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 rerere.h |   1 +
 2 files changed, 106 insertions(+), 22 deletions(-)

diff --git a/rerere.c b/rerere.c
index 33b1348..0cf857b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -30,14 +30,17 @@ static int rerere_dir_alloc;
 #define RR_HAS_PREIMAGE 2
 static struct rerere_dir {
 	unsigned char sha1[20];
-	unsigned char status;
+	int status_alloc, status_nr;
+	unsigned char *status;
 } **rerere_dir;
 
 static void free_rerere_dirs(void)
 {
 	int i;
-	for (i = 0; i < rerere_dir_nr; i++)
+	for (i = 0; i < rerere_dir_nr; i++) {
+		free(rerere_dir[i]->status);
 		free(rerere_dir[i]);
+	}
 	free(rerere_dir);
 	rerere_dir_nr = rerere_dir_alloc = 0;
 	rerere_dir = NULL;
@@ -53,17 +56,59 @@ static const char *rerere_id_hex(const struct rerere_id *id)
 	return sha1_to_hex(id->collection->sha1);
 }
 
+static void fit_variant(struct rerere_dir *rr_dir, int variant)
+{
+	variant++;
+	ALLOC_GROW(rr_dir->status, variant, rr_dir->status_alloc);
+	if (rr_dir->status_nr < variant) {
+		memset(rr_dir->status + rr_dir->status_nr,
+		       '\0', variant - rr_dir->status_nr);
+		rr_dir->status_nr = variant;
+	}
+}
+
+static void assign_variant(struct rerere_id *id)
+{
+	int variant;
+	struct rerere_dir *rr_dir = id->collection;
+
+	variant = id->variant;
+	if (variant < 0) {
+		variant = 0; /* for now */
+	}
+	fit_variant(rr_dir, variant);
+	id->variant = variant;
+}
+
 const char *rerere_path(const struct rerere_id *id, const char *file)
 {
 	if (!file)
 		return git_path("rr-cache/%s", rerere_id_hex(id));
 
-	return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+	if (id->variant <= 0)
+		return git_path("rr-cache/%s/%s", rerere_id_hex(id), file);
+
+	return git_path("rr-cache/%s/%s.%d",
+			rerere_id_hex(id), file, id->variant);
 }
 
-static int is_rr_file(const char *name, const char *filename)
+static int is_rr_file(const char *name, const char *filename, int *variant)
 {
-	return !strcmp(name, filename);
+	const char *suffix;
+	char *ep;
+
+	if (!strcmp(name, filename)) {
+		*variant = 0;
+		return 1;
+	}
+	if (!skip_prefix(name, filename, &suffix) || *suffix != '.')
+		return 0;
+
+	errno = 0;
+	*variant = strtol(suffix + 1, &ep, 10);
+	if (errno || *ep)
+		return 0;
+	return 1;
 }
 
 static void scan_rerere_dir(struct rerere_dir *rr_dir)
@@ -74,10 +119,15 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
 	if (!dir)
 		return;
 	while ((de = readdir(dir)) != NULL) {
-		if (is_rr_file(de->d_name, "postimage"))
-			rr_dir->status |= RR_HAS_POSTIMAGE;
-		else if (is_rr_file(de->d_name, "preimage"))
-			rr_dir->status |= RR_HAS_PREIMAGE;
+		int variant;
+
+		if (is_rr_file(de->d_name, "postimage", &variant)) {
+			fit_variant(rr_dir, variant);
+			rr_dir->status[variant] |= RR_HAS_POSTIMAGE;
+		} else if (is_rr_file(de->d_name, "preimage", &variant)) {
+			fit_variant(rr_dir, variant);
+			rr_dir->status[variant] |= RR_HAS_PREIMAGE;
+		}
 	}
 	closedir(dir);
 }
@@ -100,7 +150,9 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 	if (pos < 0) {
 		rr_dir = xmalloc(sizeof(*rr_dir));
 		hashcpy(rr_dir->sha1, sha1);
-		rr_dir->status = 0;
+		rr_dir->status = NULL;
+		rr_dir->status_nr = 0;
+		rr_dir->status_alloc = 0;
 		pos = -1 - pos;
 
 		/* Make sure the array is big enough ... */
@@ -118,19 +170,27 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
 static int has_rerere_resolution(const struct rerere_id *id)
 {
 	const int both = RR_HAS_POSTIMAGE|RR_HAS_PREIMAGE;
+	int variant = id->variant;
 
-	return ((id->collection->status & both) == both);
+	if (variant < 0)
+		return 0;
+	return ((id->collection->status[variant] & both) == both);
 }
 
 static int has_rerere_preimage(const struct rerere_id *id)
 {
-	return (id->collection->status & RR_HAS_PREIMAGE);
+	int variant = id->variant;
+
+	if (variant < 0)
+		return 0;
+	return (id->collection->status[variant] & RR_HAS_PREIMAGE);
 }
 
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
 	id->collection = find_rerere_dir(hex);
+	id->variant = -1; /* not known yet */
 	return id;
 }
 
@@ -157,16 +217,26 @@ static void read_rr(struct string_list *rr)
 		char *path;
 		unsigned char sha1[20];
 		struct rerere_id *id;
+		int variant;
 
 		/* There has to be the hash, tab, path and then NUL */
 		if (buf.len < 42 || get_sha1_hex(buf.buf, sha1))
 			die("corrupt MERGE_RR");
 
-		if (buf.buf[40] != '\t')
+		if (buf.buf[40] != '.') {
+			variant = 0;
+			path = buf.buf + 40;
+		} else {
+			errno = 0;
+			variant = strtol(buf.buf + 41, &path, 10);
+			if (errno)
+				die("corrupt MERGE_RR");
+		}
+		if (*(path++) != '\t')
 			die("corrupt MERGE_RR");
 		buf.buf[40] = '\0';
-		path = buf.buf + 41;
 		id = new_rerere_id_hex(buf.buf);
+		id->variant = variant;
 		string_list_insert(rr, path)->util = id;
 	}
 	strbuf_release(&buf);
@@ -187,9 +257,16 @@ static int write_rr(struct string_list *rr, int out_fd)
 		id = rr->items[i].util;
 		if (!id)
 			continue;
-		strbuf_addf(&buf, "%s\t%s%c",
-			    rerere_id_hex(id),
-			    rr->items[i].string, 0);
+		assert(id->variant >= 0);
+		if (0 < id->variant)
+			strbuf_addf(&buf, "%s.%d\t%s%c",
+				    rerere_id_hex(id), id->variant,
+				    rr->items[i].string, 0);
+		else
+			strbuf_addf(&buf, "%s\t%s%c",
+				    rerere_id_hex(id),
+				    rr->items[i].string, 0);
+
 		if (write_in_full(out_fd, buf.buf, buf.len) != buf.len)
 			die("unable to write rerere record");
 
@@ -752,7 +829,12 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 			       struct string_list *update)
 {
 	const char *path = rr_item->string;
-	const struct rerere_id *id = rr_item->util;
+	struct rerere_id *id = rr_item->util;
+	int variant;
+
+	if (id->variant < 0)
+		assign_variant(id);
+	variant = id->variant;
 
 	if (!has_rerere_preimage(id)) {
 		/*
@@ -761,13 +843,13 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 		 * the "preimage" file.
 		 */
 		handle_file(path, NULL, rerere_path(id, "preimage"));
-		if (id->collection->status & RR_HAS_POSTIMAGE) {
+		if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
 			const char *path = rerere_path(id, "postimage");
 			if (unlink(path))
 				die_errno("cannot unlink stray '%s'", path);
-			id->collection->status &= ~RR_HAS_POSTIMAGE;
+			id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
 		}
-		id->collection->status |= RR_HAS_PREIMAGE;
+		id->collection->status[variant] |= RR_HAS_PREIMAGE;
 		fprintf(stderr, "Recorded preimage for '%s'\n", path);
 		return;
 	} else if (has_rerere_resolution(id)) {
@@ -784,7 +866,7 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 	} else if (!handle_file(path, NULL, NULL)) {
 		/* The user has resolved it. */
 		copy_file(rerere_path(id, "postimage"), path, 0666);
-		id->collection->status |= RR_HAS_POSTIMAGE;
+		id->collection->status[variant] |= RR_HAS_POSTIMAGE;
 		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	} else {
 		return;
@@ -919,6 +1001,7 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 
 	/* Nuke the recorded resolution for the conflict */
 	id = new_rerere_id(sha1);
+	id->variant = 0; /* for now */
 	filename = rerere_path(id, "postimage");
 	if (unlink(filename))
 		return (errno == ENOENT
diff --git a/rerere.h b/rerere.h
index faf5a23..64d38d9 100644
--- a/rerere.h
+++ b/rerere.h
@@ -18,6 +18,7 @@ extern void *RERERE_RESOLVED;
 struct rerere_dir;
 struct rerere_id {
 	struct rerere_dir *collection;
+	int variant;
 };
 
 extern int setup_rerere(struct string_list *, int);
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 06/11] t4200: rerere a merge with two identical conflicts

[Junio C Hamano]

When the context of multiple identical conflicts are different, two
seemingly the same conflict resolution cannot be safely applied.

In such a case, at least we should be able to record these two
resolutions separately in the rerere database, and reuse them when
we see the same conflict later.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4200-rerere.sh | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index c428011..6fcc6d4 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -406,4 +406,78 @@ test_expect_success 'rerere -h' '
 	test_i18ngrep [Uu]sage help
 '
 
+concat_insert () {
+	last=$1
+	shift
+	cat early && printf "%s\n" "$@" && cat late "$last"
+}
+
+test_expect_failure 'multiple identical conflicts' '
+	git reset --hard &&
+
+	test_seq 1 6 >early &&
+	>late &&
+	test_seq 11 15 >short &&
+	test_seq 111 120 >long &&
+	concat_insert short >file1 &&
+	concat_insert long >file2 &&
+	git add file1 file2 &&
+	git commit -m base &&
+	git tag base &&
+	git checkout -b six.1 &&
+	concat_insert short 6.1 >file1 &&
+	concat_insert long 6.1 >file2 &&
+	git add file1 file2 &&
+	git commit -m 6.1 &&
+	git checkout -b six.2 HEAD^ &&
+	concat_insert short 6.2 >file1 &&
+	concat_insert long 6.2 >file2 &&
+	git add file1 file2 &&
+	git commit -m 6.2 &&
+
+	# At this point, six.1 and six.2
+	# - derive from common ancestor that has two files
+	#   1...6 7 11..15 (file1) and 1...6 7 111..120 (file2)
+	# - six.1 replaces these 7s with 6.1
+	# - six.2 replaces these 7s with 6.2
+
+	test_must_fail git merge six.1 &&
+
+	# Check that rerere knows that file1 and file2 have conflicts
+
+	printf "%s\n" file1 file2 >expect &&
+	git ls-files -u | sed -e "s/^.*	//" | sort -u >actual &&
+	test_cmp expect actual &&
+
+	git rerere status | sort >actual &&
+	test_cmp expect actual &&
+
+	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
+	concat_insert short 6.1 6.2 >file1 &&
+	concat_insert long 6.1 6.2 >file2 &&
+
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	# We resolved file1 and file2
+	git rerere &&
+	>expect &&
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	# Now we should be able to resolve them both
+	git reset --hard &&
+	test_must_fail git merge six.1 &&
+	git rerere &&
+
+	>expect &&
+	git rerere remaining >actual &&
+	test_cmp expect actual &&
+
+	concat_insert short 6.1 6.2 >file1.expect &&
+	concat_insert long 6.1 6.2 >file2.expect &&
+	test_cmp file1.expect file1 &&
+	test_cmp file2.expect file2
+'
+
 test_done
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 07/11] rerere: do use multiple variants

[Junio C Hamano]

This enables the multiple-variant support for real.  Multiple
conflicts of the same shape can have differences in contexts where
they appear, interfering the replaying of recorded resolution of one
conflict to another, and in such a case, their resolutions are
recorded as different variants under the same conflict ID.

We still need to adjust garbage collection codepaths for this
change, but the basic "replay" functionality is functional with
this change.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c          | 98 ++++++++++++++++++++++++++++++++++---------------------
 t/t4200-rerere.sh |  2 +-
 2 files changed, 61 insertions(+), 39 deletions(-)

diff --git a/rerere.c b/rerere.c
index 0cf857b..353227c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -74,7 +74,9 @@ static void assign_variant(struct rerere_id *id)
 
 	variant = id->variant;
 	if (variant < 0) {
-		variant = 0; /* for now */
+		for (variant = 0; variant < rr_dir->status_nr; variant++)
+			if (!rr_dir->status[variant])
+				break;
 	}
 	fit_variant(rr_dir, variant);
 	id->variant = variant;
@@ -177,15 +179,6 @@ static int has_rerere_resolution(const struct rerere_id *id)
 	return ((id->collection->status[variant] & both) == both);
 }
 
-static int has_rerere_preimage(const struct rerere_id *id)
-{
-	int variant = id->variant;
-
-	if (variant < 0)
-		return 0;
-	return (id->collection->status[variant] & RR_HAS_PREIMAGE);
-}
-
 static struct rerere_id *new_rerere_id_hex(char *hex)
 {
 	struct rerere_id *id = xmalloc(sizeof(*id));
@@ -818,6 +811,13 @@ static void update_paths(struct string_list *update)
 		rollback_lock_file(&index_lock);
 }
 
+static void remove_variant(struct rerere_id *id)
+{
+	unlink_or_warn(rerere_path(id, "postimage"));
+	unlink_or_warn(rerere_path(id, "preimage"));
+	id->collection->status[id->variant] = 0;
+}
+
 /*
  * The path indicated by rr_item may still have conflict for which we
  * have a recorded resolution, in which case replay it and optionally
@@ -830,32 +830,46 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 {
 	const char *path = rr_item->string;
 	struct rerere_id *id = rr_item->util;
+	struct rerere_dir *rr_dir = id->collection;
 	int variant;
 
-	if (id->variant < 0)
-		assign_variant(id);
 	variant = id->variant;
 
-	if (!has_rerere_preimage(id)) {
+	/* Has the user resolved it already? */
+	if (variant >= 0) {
+		if (!handle_file(path, NULL, NULL)) {
+			copy_file(rerere_path(id, "postimage"), path, 0666);
+			id->collection->status[variant] |= RR_HAS_POSTIMAGE;
+			fprintf(stderr, "Recorded resolution for '%s'.\n", path);
+			free_rerere_id(rr_item);
+			rr_item->util = NULL;
+			return;
+		}
 		/*
-		 * We are the first to encounter this conflict.  Ask
-		 * handle_file() to write the normalized contents to
-		 * the "preimage" file.
+		 * There may be other variants that can cleanly
+		 * replay.  Try them and update the variant number for
+		 * this one.
 		 */
-		handle_file(path, NULL, rerere_path(id, "preimage"));
-		if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
-			const char *path = rerere_path(id, "postimage");
-			if (unlink(path))
-				die_errno("cannot unlink stray '%s'", path);
-			id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
-		}
-		id->collection->status[variant] |= RR_HAS_PREIMAGE;
-		fprintf(stderr, "Recorded preimage for '%s'\n", path);
-		return;
-	} else if (has_rerere_resolution(id)) {
-		/* Is there a recorded resolution we could attempt to apply? */
-		if (merge(id, path))
-			return; /* failed to replay */
+	}
+
+	/* Does any existing resolution apply cleanly? */
+	for (variant = 0; variant < rr_dir->status_nr; variant++) {
+		const int both = RR_HAS_PREIMAGE | RR_HAS_POSTIMAGE;
+		struct rerere_id vid = *id;
+
+		if ((rr_dir->status[variant] & both) != both)
+			continue;
+
+		vid.variant = variant;
+		if (merge(&vid, path))
+			continue; /* failed to replay */
+
+		/*
+		 * If there already is a different variant that applies
+		 * cleanly, there is no point maintaining our own variant.
+		 */
+		if (0 <= id->variant && id->variant != variant)
+			remove_variant(id);
 
 		if (rerere_autoupdate)
 			string_list_insert(update, path);
@@ -863,16 +877,24 @@ static void do_rerere_one_path(struct string_list_item *rr_item,
 			fprintf(stderr,
 				"Resolved '%s' using previous resolution.\n",
 				path);
-	} else if (!handle_file(path, NULL, NULL)) {
-		/* The user has resolved it. */
-		copy_file(rerere_path(id, "postimage"), path, 0666);
-		id->collection->status[variant] |= RR_HAS_POSTIMAGE;
-		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
-	} else {
+		free_rerere_id(rr_item);
+		rr_item->util = NULL;
 		return;
 	}
-	free_rerere_id(rr_item);
-	rr_item->util = NULL;
+
+	/* None of the existing one applies; we need a new variant */
+	assign_variant(id);
+
+	variant = id->variant;
+	handle_file(path, NULL, rerere_path(id, "preimage"));
+	if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
+		const char *path = rerere_path(id, "postimage");
+		if (unlink(path))
+			die_errno("cannot unlink stray '%s'", path);
+		id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
+	}
+	id->collection->status[variant] |= RR_HAS_PREIMAGE;
+	fprintf(stderr, "Recorded preimage for '%s'\n", path);
 }
 
 static int do_plain_rerere(struct string_list *rr, int fd)
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 6fcc6d4..b1bda20 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -412,7 +412,7 @@ concat_insert () {
 	cat early && printf "%s\n" "$@" && cat late "$last"
 }
 
-test_expect_failure 'multiple identical conflicts' '
+test_expect_success 'multiple identical conflicts' '
 	git reset --hard &&
 
 	test_seq 1 6 >early &&
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 08/11] rerere: gc and clear

[Junio C Hamano]

Adjust "git rerere gc" and "git rerere clear" to the new world order
with rerere database with multiple variants for the same shape of
conflicts.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c          | 87 ++++++++++++++++++++++++++++++-------------------------
 t/t4200-rerere.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 123 insertions(+), 45 deletions(-)

diff --git a/rerere.c b/rerere.c
index 353227c..3d4dd33 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1081,29 +1081,16 @@ int rerere_forget(struct pathspec *pathspec)
  * Garbage collection support
  */
 
-/*
- * Note that this is not reentrant but is used only one-at-a-time
- * so it does not matter right now.
- */
-static struct rerere_id *dirname_to_id(const char *name)
-{
-	static struct rerere_id id;
-	id.collection = find_rerere_dir(name);
-	return &id;
-}
-
-static time_t rerere_created_at(const char *dir_name)
+static time_t rerere_created_at(struct rerere_id *id)
 {
 	struct stat st;
-	struct rerere_id *id = dirname_to_id(dir_name);
 
 	return stat(rerere_path(id, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
 
-static time_t rerere_last_used_at(const char *dir_name)
+static time_t rerere_last_used_at(struct rerere_id *id)
 {
 	struct stat st;
-	struct rerere_id *id = dirname_to_id(dir_name);
 
 	return stat(rerere_path(id, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
 }
@@ -1113,15 +1100,28 @@ static time_t rerere_last_used_at(const char *dir_name)
  */
 static void unlink_rr_item(struct rerere_id *id)
 {
-	unlink(rerere_path(id, "thisimage"));
-	unlink(rerere_path(id, "preimage"));
-	unlink(rerere_path(id, "postimage"));
-	/*
-	 * NEEDSWORK: what if this rmdir() fails?  Wouldn't we then
-	 * assume that we already have preimage recorded in
-	 * do_plain_rerere()?
-	 */
-	rmdir(rerere_path(id, NULL));
+	unlink_or_warn(rerere_path(id, "thisimage"));
+	remove_variant(id);
+	id->collection->status[id->variant] = 0;
+}
+
+static void prune_one(struct rerere_id *id, time_t now,
+		      int cutoff_resolve, int cutoff_noresolve)
+{
+	time_t then;
+	int cutoff;
+
+	then = rerere_last_used_at(id);
+	if (then)
+		cutoff = cutoff_resolve;
+	else {
+		then = rerere_created_at(id);
+		if (!then)
+			return;
+		cutoff = cutoff_noresolve;
+	}
+	if (then < now - cutoff * 86400)
+		unlink_rr_item(id);
 }
 
 void rerere_gc(struct string_list *rr)
@@ -1129,8 +1129,8 @@ void rerere_gc(struct string_list *rr)
 	struct string_list to_remove = STRING_LIST_INIT_DUP;
 	DIR *dir;
 	struct dirent *e;
-	int i, cutoff;
-	time_t now = time(NULL), then;
+	int i;
+	time_t now = time(NULL);
 	int cutoff_noresolve = 15;
 	int cutoff_resolve = 60;
 
@@ -1142,25 +1142,32 @@ void rerere_gc(struct string_list *rr)
 		die_errno("unable to open rr-cache directory");
 	/* Collect stale conflict IDs ... */
 	while ((e = readdir(dir))) {
+		struct rerere_dir *rr_dir;
+		struct rerere_id id;
+		int now_empty;
+
 		if (is_dot_or_dotdot(e->d_name))
 			continue;
-
-		then = rerere_last_used_at(e->d_name);
-		if (then) {
-			cutoff = cutoff_resolve;
-		} else {
-			then = rerere_created_at(e->d_name);
-			if (!then)
-				continue;
-			cutoff = cutoff_noresolve;
+		rr_dir = find_rerere_dir(e->d_name);
+		if (!rr_dir)
+			continue; /* or should we remove e->d_name? */
+
+		now_empty = 1;
+		for (id.variant = 0, id.collection = rr_dir;
+		     id.variant < id.collection->status_nr;
+		     id.variant++) {
+			prune_one(&id, now, cutoff_resolve, cutoff_noresolve);
+			if (id.collection->status[id.variant])
+				now_empty = 0;
 		}
-		if (then < now - cutoff * 86400)
+		if (now_empty)
 			string_list_append(&to_remove, e->d_name);
 	}
 	closedir(dir);
-	/* ... and then remove them one-by-one */
+
+	/* ... and then remove the empty directories */
 	for (i = 0; i < to_remove.nr; i++)
-		unlink_rr_item(dirname_to_id(to_remove.items[i].string));
+		rmdir(git_path("rr-cache/%s", to_remove.items[i].string));
 	string_list_clear(&to_remove, 0);
 }
 
@@ -1177,8 +1184,10 @@ void rerere_clear(struct string_list *merge_rr)
 
 	for (i = 0; i < merge_rr->nr; i++) {
 		struct rerere_id *id = merge_rr->items[i].util;
-		if (!has_rerere_resolution(id))
+		if (!has_rerere_resolution(id)) {
 			unlink_rr_item(id);
+			rmdir(rerere_path(id, NULL));
+		}
 	}
 	unlink_or_warn(git_path("MERGE_RR"));
 }
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index b1bda20..a85fc7d 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -412,6 +412,39 @@ concat_insert () {
 	cat early && printf "%s\n" "$@" && cat late "$last"
 }
 
+count_pre_post () {
+	find .git/rr-cache/ -type f -name "preimage*" >actual &&
+	test_line_count = "$1" actual &&
+	find .git/rr-cache/ -type f -name "postimage*" >actual &&
+	test_line_count = "$2" actual
+}
+
+test_expect_success 'rerere gc' '
+	find .git/rr-cache -type f >original &&
+	xargs test-chmtime -172800 <original &&
+
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+	find .git/rr-cache -type f >actual &&
+	test_cmp original actual &&
+
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=0 rerere gc &&
+	find .git/rr-cache -type f >actual &&
+	test_cmp original actual &&
+
+	git -c gc.rerereresolved=0 -c gc.rerereunresolved=0 rerere gc &&
+	find .git/rr-cache -type f >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
+merge_conflict_resolve () {
+	git reset --hard &&
+	test_must_fail git merge six.1 &&
+	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
+	concat_insert short 6.1 6.2 >file1 &&
+	concat_insert long 6.1 6.2 >file2
+}
+
 test_expect_success 'multiple identical conflicts' '
 	git reset --hard &&
 
@@ -441,7 +474,7 @@ test_expect_success 'multiple identical conflicts' '
 	# - six.1 replaces these 7s with 6.1
 	# - six.2 replaces these 7s with 6.2
 
-	test_must_fail git merge six.1 &&
+	merge_conflict_resolve &&
 
 	# Check that rerere knows that file1 and file2 have conflicts
 
@@ -452,19 +485,43 @@ test_expect_success 'multiple identical conflicts' '
 	git rerere status | sort >actual &&
 	test_cmp expect actual &&
 
-	# Resolution is to replace 7 with 6.1 and 6.2 (i.e. take both)
-	concat_insert short 6.1 6.2 >file1 &&
-	concat_insert long 6.1 6.2 >file2 &&
-
 	git rerere remaining >actual &&
 	test_cmp expect actual &&
 
+	count_pre_post 2 0 &&
+
+	# Pretend that the conflicts were made quite some time ago
+	find .git/rr-cache/ -type f | xargs test-chmtime -172800 &&
+
+	# Unresolved entries have not expired yet
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+	count_pre_post 2 0 &&
+
+	# Unresolved entries have expired
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=1 rerere gc &&
+	count_pre_post 0 0 &&
+
+	# Recreate the conflicted state
+	merge_conflict_resolve &&
+	count_pre_post 2 0 &&
+
+	# Clear it
+	git rerere clear &&
+	count_pre_post 0 0 &&
+
+	# Recreate the conflicted state
+	merge_conflict_resolve &&
+	count_pre_post 2 0 &&
+
 	# We resolved file1 and file2
 	git rerere &&
 	>expect &&
 	git rerere remaining >actual &&
 	test_cmp expect actual &&
 
+	# We must have recorded both of them
+	count_pre_post 2 2 &&
+
 	# Now we should be able to resolve them both
 	git reset --hard &&
 	test_must_fail git merge six.1 &&
@@ -477,7 +534,19 @@ test_expect_success 'multiple identical conflicts' '
 	concat_insert short 6.1 6.2 >file1.expect &&
 	concat_insert long 6.1 6.2 >file2.expect &&
 	test_cmp file1.expect file1 &&
-	test_cmp file2.expect file2
+	test_cmp file2.expect file2 &&
+
+	# Pretend that the resolutions are old again
+	find .git/rr-cache/ -type f | xargs test-chmtime -172800 &&
+
+	# Resolved entries have not expired yet
+	git -c gc.rerereresolved=5 -c gc.rerereunresolved=5 rerere gc &&
+
+	count_pre_post 2 2 &&
+
+	# Resolved entries have expired
+	git -c gc.rerereresolved=1 -c gc.rerereunresolved=5 rerere gc &&
+	count_pre_post 0 0
 '
 
 test_done
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 09/11] rerere: move code related to "forget" together

[Junio C Hamano]

"rerere forget" is the only user of handle_cache() helper, which in
turn is the only user of rerere_io that reads from an in-core buffer
whose getline method is implemented as rerere_mem_getline().  Gather
them together.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 194 +++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 97 insertions(+), 97 deletions(-)

diff --git a/rerere.c b/rerere.c
index 3d4dd33..2b870e0 100644
--- a/rerere.c
+++ b/rerere.c
@@ -517,103 +517,6 @@ static int handle_file(const char *path, unsigned char *sha1, const char *output
 }
 
 /*
- * Subclass of rerere_io that reads from an in-core buffer that is a
- * strbuf
- */
-struct rerere_io_mem {
-	struct rerere_io io;
-	struct strbuf input;
-};
-
-/*
- * ... and its getline() method implementation
- */
-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 = memchr(io->input.buf, '\n', io->input.len);
-	if (!ep)
-		ep = io->input.buf + io->input.len;
-	else 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 int handle_cache(const char *path, unsigned char *sha1, const char *output)
-{
-	mmfile_t mmfile[3] = {{NULL}};
-	mmbuffer_t result = {NULL, 0};
-	const struct cache_entry *ce;
-	int pos, len, i, hunk_no;
-	struct rerere_io_mem io;
-	int marker_size = ll_merge_marker_size(path);
-
-	/*
-	 * Reproduce the conflicted merge in-core
-	 */
-	len = strlen(path);
-	pos = cache_name_pos(path, len);
-	if (0 <= pos)
-		return -1;
-	pos = -pos - 1;
-
-	while (pos < active_nr) {
-		enum object_type type;
-		unsigned long size;
-
-		ce = active_cache[pos++];
-		if (ce_namelen(ce) != len || memcmp(ce->name, path, len))
-			break;
-		i = ce_stage(ce) - 1;
-		if (!mmfile[i].ptr) {
-			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("");
-
-	/*
-	 * NEEDSWORK: handle conflicts from merges with
-	 * merge.renormalize set, too
-	 */
-	ll_merge(&result, path, &mmfile[0], NULL,
-		 &mmfile[1], "ours",
-		 &mmfile[2], "theirs", NULL);
-	for (i = 0; i < 3; i++)
-		free(mmfile[i].ptr);
-
-	memset(&io, 0, sizeof(io));
-	io.io.getline = rerere_mem_getline;
-	if (output)
-		io.io.output = fopen(output, "w");
-	else
-		io.io.output = NULL;
-	strbuf_init(&io.input, 0);
-	strbuf_attach(&io.input, result.ptr, result.size, result.size);
-
-	/*
-	 * Grab the conflict ID and optionally write the original
-	 * contents with conflict markers out.
-	 */
-	hunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);
-	strbuf_release(&io.input);
-	if (io.io.output)
-		fclose(io.io.output);
-	return hunk_no;
-}
-
-/*
  * Look at a cache entry at "i" and see if it is not conflicting,
  * conflicting and we are willing to handle, or conflicting and
  * we are unable to handle, and return the determination in *type.
@@ -1005,6 +908,103 @@ int rerere(int flags)
 	return status;
 }
 
+/*
+ * Subclass of rerere_io that reads from an in-core buffer that is a
+ * strbuf
+ */
+struct rerere_io_mem {
+	struct rerere_io io;
+	struct strbuf input;
+};
+
+/*
+ * ... and its getline() method implementation
+ */
+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 = memchr(io->input.buf, '\n', io->input.len);
+	if (!ep)
+		ep = io->input.buf + io->input.len;
+	else 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 int handle_cache(const char *path, unsigned char *sha1, const char *output)
+{
+	mmfile_t mmfile[3] = {{NULL}};
+	mmbuffer_t result = {NULL, 0};
+	const struct cache_entry *ce;
+	int pos, len, i, hunk_no;
+	struct rerere_io_mem io;
+	int marker_size = ll_merge_marker_size(path);
+
+	/*
+	 * Reproduce the conflicted merge in-core
+	 */
+	len = strlen(path);
+	pos = cache_name_pos(path, len);
+	if (0 <= pos)
+		return -1;
+	pos = -pos - 1;
+
+	while (pos < active_nr) {
+		enum object_type type;
+		unsigned long size;
+
+		ce = active_cache[pos++];
+		if (ce_namelen(ce) != len || memcmp(ce->name, path, len))
+			break;
+		i = ce_stage(ce) - 1;
+		if (!mmfile[i].ptr) {
+			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("");
+
+	/*
+	 * NEEDSWORK: handle conflicts from merges with
+	 * merge.renormalize set, too?
+	 */
+	ll_merge(&result, path, &mmfile[0], NULL,
+		 &mmfile[1], "ours",
+		 &mmfile[2], "theirs", NULL);
+	for (i = 0; i < 3; i++)
+		free(mmfile[i].ptr);
+
+	memset(&io, 0, sizeof(io));
+	io.io.getline = rerere_mem_getline;
+	if (output)
+		io.io.output = fopen(output, "w");
+	else
+		io.io.output = NULL;
+	strbuf_init(&io.input, 0);
+	strbuf_attach(&io.input, result.ptr, result.size, result.size);
+
+	/*
+	 * Grab the conflict ID and optionally write the original
+	 * contents with conflict markers out.
+	 */
+	hunk_no = handle_path(sha1, (struct rerere_io *)&io, marker_size);
+	strbuf_release(&io.input);
+	if (io.io.output)
+		fclose(io.io.output);
+	return hunk_no;
+}
+
 static int rerere_forget_one_path(const char *path, struct string_list *rr)
 {
 	const char *filename;
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 10/11] rerere: split code to call ll_merge() further

[Junio C Hamano]

The merge() helper function is given an existing rerere ID (i.e. the
name of the .git/rr-cache/* subdirectory, and the variant number)
that identifies one <preimage, postimage> pair, try to see if the
conflicted state in the given path can be resolved by using the pair,
and if this succeeds, then update the conflicted path with the
result in the working tree.

To implement rerere_forget() in the multiple variant world, we'd
need a helper to do the "see if a <preimage, postimage> pair cleanly
resolves a conflicted state we have in-core" part, without actually
touching any file in the working tree, in order to identify which
variant(s) to remove.  Split the logic to do so into a separate
helper function try_merge() out of merge().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/rerere.c b/rerere.c
index 2b870e0..e636d4b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -622,6 +622,33 @@ int rerere_remaining(struct string_list *merge_rr)
 }
 
 /*
+ * Try using the given conflict resolution "ID" to see
+ * if that recorded conflict resolves cleanly what we
+ * got in the "cur".
+ */
+static int try_merge(const struct rerere_id *id, const char *path,
+		     mmfile_t *cur, mmbuffer_t *result)
+{
+	int ret;
+	mmfile_t base = {NULL, 0}, other = {NULL, 0};
+
+	if (read_mmfile(&base, rerere_path(id, "preimage")) ||
+	    read_mmfile(&other, rerere_path(id, "postimage")))
+		ret = 1;
+	else
+		/*
+		 * A three-way merge. Note that this honors user-customizable
+		 * low-level merge driver settings.
+		 */
+		ret = ll_merge(result, path, &base, NULL, cur, "", &other, "", NULL);
+
+	free(base.ptr);
+	free(other.ptr);
+
+	return ret;
+}
+
+/*
  * Find the conflict identified by "id"; the change between its
  * "preimage" (i.e. a previous contents with conflict markers) and its
  * "postimage" (i.e. the corresponding contents with conflicts
@@ -635,30 +662,20 @@ static int merge(const struct rerere_id *id, const char *path)
 {
 	FILE *f;
 	int ret;
-	mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};
+	mmfile_t cur = {NULL, 0};
 	mmbuffer_t result = {NULL, 0};
 
 	/*
 	 * Normalize the conflicts in path and write it out to
 	 * "thisimage" temporary file.
 	 */
-	if (handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) {
-		ret = 1;
-		goto out;
-	}
-
-	if (read_mmfile(&cur, rerere_path(id, "thisimage")) ||
-	    read_mmfile(&base, rerere_path(id, "preimage")) ||
-	    read_mmfile(&other, rerere_path(id, "postimage"))) {
+	if ((handle_file(path, NULL, rerere_path(id, "thisimage")) < 0) ||
+	    read_mmfile(&cur, rerere_path(id, "thisimage"))) {
 		ret = 1;
 		goto out;
 	}
 
-	/*
-	 * A three-way merge. Note that this honors user-customizable
-	 * low-level merge driver settings.
-	 */
-	ret = ll_merge(&result, path, &base, NULL, &cur, "", &other, "", NULL);
+	ret = try_merge(id, path, &cur, &result);
 	if (ret)
 		goto out;
 
@@ -684,8 +701,6 @@ static int merge(const struct rerere_id *id, const char *path)
 
 out:
 	free(cur.ptr);
-	free(base.ptr);
-	free(other.ptr);
 	free(result.ptr);
 
 	return ret;
-- 
2.8.1-273-ga2cd0f9

[PATCH v3 11/11] rerere: adjust 'forget' to multi-variant world order

[Junio C Hamano]

Because conflicts with the same contents inside conflict blocks
enclosed by "<<<<<<<" and ">>>>>>>" can now have multiple variants
to help three-way merge to adjust to the differences outside the
conflict blocks, "rerere forget $path" needs to be taught that there
may be multiple recorded resolutions that share the same conflict
hash (which groups the conflicts with "the same contents inside
conflict blocks"), among which there are some that would not be
relevant to the conflict we are looking at.  These "other variants"
that happen to share the same conflict hash should not be cleared,
and the variant that would apply to the current conflict may not be
the zero-th one (which is the only one that is cleared by the
current code).

After finding the conflict hash, iterate over the existing variants
and try to resolve the conflict using each of them to find the one
that "cleanly" resolves the current conflict.  That is the one we
want to forget and record the preimage for, so that the user can
record the corrected resolution.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c          | 28 +++++++++++++++++++++++++++-
 t/t4200-rerere.sh | 10 ++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/rerere.c b/rerere.c
index e636d4b..1693866 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1038,7 +1038,33 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 
 	/* Nuke the recorded resolution for the conflict */
 	id = new_rerere_id(sha1);
-	id->variant = 0; /* for now */
+
+	for (id->variant = 0;
+	     id->variant < id->collection->status_nr;
+	     id->variant++) {
+		mmfile_t cur = { NULL, 0 };
+		mmbuffer_t result = {NULL, 0};
+		int cleanly_resolved;
+
+		if (!has_rerere_resolution(id))
+			continue;
+
+		handle_cache(path, sha1, rerere_path(id, "thisimage"));
+		if (read_mmfile(&cur, rerere_path(id, "thisimage"))) {
+			free(cur.ptr);
+			return error("Failed to update conflicted state in '%s'",
+				     path);
+		}
+		cleanly_resolved = !try_merge(id, path, &cur, &result);
+		free(result.ptr);
+		free(cur.ptr);
+		if (cleanly_resolved)
+			break;
+	}
+
+	if (id->collection->status_nr <= id->variant)
+		return error("no remembered resolution for '%s'", path);
+
 	filename = rerere_path(id, "postimage");
 	if (unlink(filename))
 		return (errno == ENOENT
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index a85fc7d..1a080e7 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -536,6 +536,16 @@ test_expect_success 'multiple identical conflicts' '
 	test_cmp file1.expect file1 &&
 	test_cmp file2.expect file2 &&
 
+	# Forget resolution for file2
+	git rerere forget file2 &&
+	echo file2 >expect &&
+	git rerere status >actual &&
+	test_cmp expect actual &&
+	count_pre_post 2 1 &&
+
+	# file2 already has correct resolution, so record it again
+	git rerere &&
+
 	# Pretend that the resolutions are old again
 	find .git/rr-cache/ -type f | xargs test-chmtime -172800 &&
 
-- 
2.8.1-273-ga2cd0f9