threads / patch / 22054

patch, 9 partsrerere: remove silly 1024-byte line limit

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

## tl;dr

21 messages between Dec 29, 2009 and Jan 11, 2010. Diffs are folded; open one to read it.

replies: 20people: 2as markdown or json

Junio C Hamano· Dec 29, 2009, 21:42 UTC · lore

[PATCH 0/9] Undoing conflict resolution

This series resurrects the topic we discussed later November.

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

 - ls-files has a new option primarily for debugging;
 - update-index has a new option --clear-resolve-undo;
 - rerere has a new subcommand forget.
Junio C Hamano (9):
  builtin-merge.c: use standard active_cache macros
  resolve-undo: record resolved conflicts in a new index extension
    section
  resolve-undo: basic tests
  resolve-undo: allow plumbing to clear the information
  resolve-undo: "checkout -m path" uses resolve-undo information
  resolve-undo: teach "update-index --unresolve" to use resolve-undo
    info
  rerere: remove silly 1024-byte line limit
  rerere: refactor rerere logic to make it independent from I/O
  rerere forget path: forget recorded resolution
 Makefile                  |    2 +
 builtin-checkout.c        |    6 +
 builtin-ls-files.c        |   43 +++++++-
 builtin-merge.c           |    9 +-
 builtin-read-tree.c       |    2 +
 builtin-rerere.c          |    2 +
 builtin-update-index.c    |   18 +++-
 cache.h                   |    4 +
 read-cache.c              |   18 +++
 rerere.c                  |  259 +++++++++++++++++++++++++++++++++++++-------
 rerere.h                  |    1 +
 resolve-undo.c            |  176 ++++++++++++++++++++++++++++++
 resolve-undo.h            |   16 +++
 t/t2030-unresolve-info.sh |  140 ++++++++++++++++++++++++
 14 files changed, 648 insertions(+), 48 deletions(-)
 create mode 100644 resolve-undo.c
 create mode 100644 resolve-undo.h
 create mode 100755 t/t2030-unresolve-info.sh
Junio C Hamano· Dec 29, 2009, 21:42 UTC · re: Junio C Hamano · lore

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

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

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
Show changes to builtin-merge.c +2 −3
diff --git a/builtin-merge.c b/builtin-merge.c
index f1c84d7..6cb804b 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -618,11 +618,10 @@ static void count_diff_files(struct diff_queue_struct *q,
 
 static int count_unmerged_entries(void)
 {
-	const struct index_state *state = &the_index;
 	int i, ret = 0;
 
-	for (i = 0; i < state->cache_nr; i++)
-		if (ce_stage(state->cache[i]))
+	for (i = 0; i < active_nr; i++)
+		if (ce_stage(active_cache[i]))
 			ret++;
 
 	return ret;
-- 
1.6.6
Junio C Hamano· Dec 29, 2009, 21:42 UTC · re: Junio C Hamano · lore

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

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

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

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

Makefile, builtin-checkout.c, builtin-merge.c, builtin-read-tree.c, cache.h, read-cache.c, resolve-undo.c, resolve-undo.h

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

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

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

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

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

builtin-ls-files.c, t/t2030-unresolve-info.sh

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

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

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

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

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

builtin-update-index.c, t/t2030-unresolve-info.sh

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

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

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

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

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

builtin-checkout.c, cache.h, resolve-undo.c, resolve-undo.h, t/t2030-unresolve-info.sh

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

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

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

There were several issues with this approach:
 - These days, merge is not the only command, and conflicts coming from
   commands like cherry-pick, "am -3", etc. cannot be recreated by looking
   at MERGE_HEAD;
 - For a conflict that came from a merge that had renames, picking up the
   same path from MERGE_HEAD and HEAD wouldn't help recreating it, either;
 - It may have been Ok not to recreate stage #1 back when it was written,
   because "diff --ours/--theirs" were the only availble ways to review
   conflicts and they don't need stage #1 information.  "diff --cc" that
   was invented much later is a lot more useful way but it needs stage #1.

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

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

builtin-update-index.c, cache.h, t/t2030-unresolve-info.sh

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

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

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)
Show changes to rerere.c +10 −9
diff --git a/rerere.c b/rerere.c
index 29f95f6..88bb4f1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -87,12 +87,12 @@ static int handle_file(const char *path,
 	 unsigned char *sha1, const char *output)
 {
 	git_SHA_CTX ctx;
-	char buf[1024];
 	int hunk_no = 0;
 	enum {
 		RR_CONTEXT = 0, RR_SIDE_1, RR_SIDE_2, RR_ORIGINAL,
 	} hunk = RR_CONTEXT;
 	struct strbuf one = STRBUF_INIT, two = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
 	FILE *f = fopen(path, "r");
 	FILE *out = NULL;
 	int wrerror = 0;
@@ -111,20 +111,20 @@ static int handle_file(const char *path,
 	if (sha1)
 		git_SHA1_Init(&ctx);
 
-	while (fgets(buf, sizeof(buf), f)) {
-		if (!prefixcmp(buf, "<<<<<<< ")) {
+	while (!strbuf_getwholeline(&buf, f, '\n')) {
+		if (!prefixcmp(buf.buf, "<<<<<<< ")) {
 			if (hunk != RR_CONTEXT)
 				goto bad;
 			hunk = RR_SIDE_1;
-		} else if (!prefixcmp(buf, "|||||||") && isspace(buf[7])) {
+		} else if (!prefixcmp(buf.buf, "|||||||") && isspace(buf.buf[7])) {
 			if (hunk != RR_SIDE_1)
 				goto bad;
 			hunk = RR_ORIGINAL;
-		} else if (!prefixcmp(buf, "=======") && isspace(buf[7])) {
+		} else if (!prefixcmp(buf.buf, "=======") && isspace(buf.buf[7])) {
 			if (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)
 				goto bad;
 			hunk = RR_SIDE_2;
-		} else if (!prefixcmp(buf, ">>>>>>> ")) {
+		} else if (!prefixcmp(buf.buf, ">>>>>>> ")) {
 			if (hunk != RR_SIDE_2)
 				goto bad;
 			if (strbuf_cmp(&one, &two) > 0)
@@ -147,13 +147,13 @@ static int handle_file(const char *path,
 			strbuf_reset(&one);
 			strbuf_reset(&two);
 		} else if (hunk == RR_SIDE_1)
-			strbuf_addstr(&one, buf);
+			strbuf_addstr(&one, buf.buf);
 		else if (hunk == RR_ORIGINAL)
 			; /* discard */
 		else if (hunk == RR_SIDE_2)
-			strbuf_addstr(&two, buf);
+			strbuf_addstr(&two, buf.buf);
 		else if (out)
-			ferr_puts(buf, out, &wrerror);
+			ferr_puts(buf.buf, out, &wrerror);
 		continue;
 	bad:
 		hunk = 99; /* force error exit */
@@ -161,6 +161,7 @@ static int handle_file(const char *path,
 	}
 	strbuf_release(&one);
 	strbuf_release(&two);
+	strbuf_release(&buf);
 
 	fclose(f);
 	if (wrerror)
-- 
1.6.6
Junio C Hamano· Dec 29, 2009, 21:42 UTC · re: Junio C Hamano · lore

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

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

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

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

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

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

builtin-rerere.c, rerere.c, rerere.h, t/t2030-unresolve-info.sh

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

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

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

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

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

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

-- Hannes
Junio C Hamano· Jan 6, 2010, 01:06 UTC · re: Johannes Sixt · lore

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

Johannes Sixt <j6t@kdbg.org> writes:
Show 25 quoted lines
> Junio C Hamano schrieb:
>> After you find out an earlier resolution you told rerere to use was a
>> mismerge, there is no easy way to clear it.  A new subcommand "forget" can
>> be used to tell git to forget a recorded resolution, so that you can redo
>> the merge from scratch.
>> ...
>> diff --git a/rerere.c b/rerere.c
>> index f013ae7..c1da6f6 100644
>> --- a/rerere.c
>> +++ b/rerere.c
>> ...
>> +static int handle_cache(const char *path, unsigned char *sha1)
>> +{
>> +...
>> +	ll_merge(&result, path, &mmfile[0],
>> +		 &mmfile[1], "ours",
>> +		 &mmfile[2], "theirs", 0);
>
> When you simply call ll_merge(), will it obey any merge drivers that
> are defined in .gitattributes? Do we care about them?
>
> I already had an implementation of "rerere forget" before you
> presented this solution, but it relies on that the user calls
> "checkout --conflict=merge" first. One reason (besides its simplicity)
> was that it does not have to care how the merge is computed.
Doesn't "checkout --conflict=merge" use the same ll_merge() machinery?
Show 5 quoted lines
> [I haven't submitted my solution, yet, because I haven't had the time
> to do this large merge where I expect to make use "rerere forget",
> testing its usefulness.]
>
> -- Hannes
Johannes Sixt· Jan 6, 2010, 08:55 UTC · re: Junio C Hamano · lore

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

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

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

-- Hannes
Junio C Hamano· Jan 6, 2010, 16:58 UTC · re: Johannes Sixt · lore

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

Johannes Sixt <j6t@kdbg.org> writes:
>> Doesn't "checkout --conflict=merge" use the same ll_merge() machinery?
>
> It does, without setting up .gitattributes, either (IIUC), which is a bug IMO.

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

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

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

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

The ll_merge() call jc/cache-unmerge topic adds for "rerere forget"
doesn't share this problem.
Junio C Hamano· Jan 6, 2010, 21:59 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
> However, I think you need this fix for "checkout -m" to work as intended,

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

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

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

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

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

builtin-checkout.c, t/t7201-co.sh

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

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

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

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

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

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

Johannes Sixt <j6t@kdbg.org> writes:
Show 5 quoted lines
> Your implementation forgets to re-insert the forgotten resolutions into
> MERGE_RR, therefore, the next 'git rerere' does not record the new
> resolution.
>
> In my implementation of 'rerere forget', I had the following tests.

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

Junio C Hamano· Jan 11, 2010, 01:58 UTC · re: Junio C Hamano · lore

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

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

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

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

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

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

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

Johannes Sixt· Jan 11, 2010, 19:22 UTC · re: Junio C Hamano · lore

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

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

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

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

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

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

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

-- Hannes
Junio C Hamano· Jan 11, 2010, 20:05 UTC · re: Johannes Sixt · lore

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

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

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

Try this in an empty directory.
-- >8 --
#!/bin/sh
git init
create_numbers () {
	for n in 0 1 2 3 4 "$1" 5 6 7 8 9
	do
		echo $n
	done >numbers.txt
}
create_letters () {
	for l in a b c d e "$1" f g h i j
	do
		echo $l
	done >letters.txt
}
create_files () {
	create_numbers "$1"
	create_letters "$1"
}

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

create_files "+" git commit -a -m master

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

mkdir -p .git/rr-cache

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

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

create_numbers "=" git rerere

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

-- 8< --
Now, immediately after this sequence, rerere will give you an disaster.
Johannes Sixt· Jan 11, 2010, 21:05 UTC · re: Junio C Hamano · lore

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

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

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

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

-- Hannes

← back to recent threads