[PATCH 1/4] Allow more than true/false to attributes.

[PATCH 1/4] Allow more than true/false to attributes.

[Junio C Hamano]

This allows you to define three values (and possibly more) to
each attribute: true, false, and unset.

Typically the handlers that notice and act on attribute values
treat "unset" attribute to mean "do your default thing"
(e.g. crlf that is unset would trigger "guess from contents"),
so being able to override a setting to an unset state is
actually useful.

 - If you want to set the attribute value to true, have an entry
   in .gitattributes file that mentions the attribute name; e.g.

	*.o	binary

 - If you want to set the attribute value explicitly to false,
   use '-'; e.g.

	*.a	-diff

 - If you want to make the attribute value _unset_, perhaps to
   override an earlier entry, use '!'; e.g.

	*.a	-diff
	c.i.a	!diff

This also allows string values to attributes, with the natural
syntax:

	attrname=attrvalue

but you cannot use it, as nobody takes notice and acts on
it yet.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 attr.c               |  194 +++++++++++++++++++++++++++++++------------------
 attr.h               |   12 +++-
 builtin-check-attr.c |   14 +++-
 convert.c            |   16 ++++-
 diff.c               |   15 +++-
 5 files changed, 169 insertions(+), 82 deletions(-)

diff --git a/attr.c b/attr.c
index 60fe48f..b3496a6 100644
--- a/attr.c
+++ b/attr.c
@@ -1,6 +1,8 @@
 #include "cache.h"
 #include "attr.h"
 
+#define ATTR__UNKNOWN	((void *) -2)
+
 /*
  * The basic design decision here is that we are not going to have
  * insanely large number of attributes.
@@ -83,6 +85,7 @@ struct git_attr *git_attr(const char *name, int len)
 	check_all_attr = xrealloc(check_all_attr,
 				  sizeof(*check_all_attr) * attr_nr);
 	check_all_attr[a->attr_nr].attr = a;
+	check_all_attr[a->attr_nr].value = ATTR__UNKNOWN;
 	return a;
 }
 
@@ -92,12 +95,14 @@ struct git_attr *git_attr(const char *name, int len)
  * (1) glob pattern.
  * (2) whitespace
  * (3) whitespace separated list of attribute names, each of which
- *     could be prefixed with '-' to mean "not set".
+ *     could be prefixed with '-' to mean "set to false", '!' to mean
+ *     "unset".
  */
 
+/* What does a matched pattern decide? */
 struct attr_state {
-	int unset;
 	struct git_attr *attr;
+	void *setto;
 };
 
 struct match_attr {
@@ -112,13 +117,63 @@ struct match_attr {
 
 static const char blank[] = " \t\r\n";
 
+static const char *parse_attr(const char *src, int lineno, const char *cp,
+			      int *num_attr, struct match_attr *res)
+{
+	const char *ep, *equals;
+	int len;
+
+	ep = cp + strcspn(cp, blank);
+	equals = strchr(cp, '=');
+	if (equals && ep < equals)
+		equals = NULL;
+	if (equals)
+		len = equals - cp;
+	else
+		len = ep - cp;
+	if (!res) {
+		if (*cp == '-' || *cp == '!') {
+			cp++;
+			len--;
+		}
+		if (invalid_attr_name(cp, len)) {
+			fprintf(stderr,
+				"%.*s is not a valid attribute name: %s:%d\n",
+				len, cp, src, lineno);
+			return NULL;
+		}
+	} else {
+		struct attr_state *e;
+
+		e = &(res->state[*num_attr]);
+		if (*cp == '-' || *cp == '!') {
+			e->setto = (*cp == '-') ? ATTR__FALSE : ATTR__UNSET;
+			cp++;
+			len--;
+		}
+		else if (!equals)
+			e->setto = ATTR__TRUE;
+		else {
+			char *value;
+			int vallen = ep - equals;
+			value = xmalloc(vallen);
+			memcpy(value, equals+1, vallen-1);
+			value[vallen-1] = 0;
+			e->setto = value;
+		}
+		e->attr = git_attr(cp, len);
+	}
+	(*num_attr)++;
+	return ep + strspn(ep, blank);
+}
+
 static struct match_attr *parse_attr_line(const char *line, const char *src,
 					  int lineno, int macro_ok)
 {
 	int namelen;
 	int num_attr;
 	const char *cp, *name;
-	struct match_attr *res = res;
+	struct match_attr *res = NULL;
 	int pass;
 	int is_macro;
 
@@ -153,42 +208,16 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
 		num_attr = 0;
 		cp = name + namelen;
 		cp = cp + strspn(cp, blank);
-		while (*cp) {
-			const char *ep;
-			ep = cp + strcspn(cp, blank);
-			if (!pass) {
-				if (*cp == '-')
-					cp++;
-				if (invalid_attr_name(cp, ep - cp)) {
-					fprintf(stderr,
-						"%.*s is not a valid attribute name: %s:%d\n",
-						(int)(ep - cp), cp,
-						src, lineno);
-					return NULL;
-				}
-			} else {
-				struct attr_state *e;
-
-				e = &(res->state[num_attr]);
-				if (*cp == '-') {
-					e->unset = 1;
-					cp++;
-				}
-				e->attr = git_attr(cp, ep - cp);
-			}
-			num_attr++;
-			cp = ep + strspn(ep, blank);
-		}
+		while (*cp)
+			cp = parse_attr(src, lineno, cp, &num_attr, res);
 		if (pass)
 			break;
-
 		res = xcalloc(1,
 			      sizeof(*res) +
 			      sizeof(struct attr_state) * num_attr +
 			      (is_macro ? 0 : namelen + 1));
-		if (is_macro) {
+		if (is_macro)
 			res->u.attr = git_attr(name, namelen);
-		}
 		else {
 			res->u.pattern = (char*)&(res->state[num_attr]);
 			memcpy(res->u.pattern, name, namelen);
@@ -205,9 +234,9 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
  * come from many places.
  *
  * (1) .gitattribute file of the same directory;
- * (2) .gitattribute file of the parent directory if (1) does not have any match;
- *     this goes recursively upwards, just like .gitignore
- * (3) perhaps $GIT_DIR/info/attributes, as the final fallback.
+ * (2) .gitattribute file of the parent directory if (1) does not have
+ *      any match; this goes recursively upwards, just like .gitignore.
+ * (3) $GIT_DIR/info/attributes, which overrides both of the above.
  *
  * In the same file, later entries override the earlier match, so in the
  * global list, we would have entries from info/attributes the earliest
@@ -229,8 +258,21 @@ static void free_attr_elem(struct attr_stack *e)
 {
 	int i;
 	free(e->origin);
-	for (i = 0; i < e->num_matches; i++)
-		free(e->attrs[i]);
+	for (i = 0; i < e->num_matches; i++) {
+		struct match_attr *a = e->attrs[i];
+		int j;
+		for (j = 0; j < a->num_attr; j++) {
+			void *setto = a->state[j].setto;
+			if (setto == ATTR__TRUE ||
+			    setto == ATTR__FALSE ||
+			    setto == ATTR__UNSET ||
+			    setto == ATTR__UNKNOWN)
+				;
+			else
+				free(setto);
+		}
+		free(a);
+	}
 	free(e);
 }
 
@@ -288,10 +330,19 @@ static void debug_info(const char *what, struct attr_stack *elem)
 {
 	fprintf(stderr, "%s: %s\n", what, elem->origin ? elem->origin : "()");
 }
-static void debug_set(const char *what, const char *match, struct git_attr *attr, int set)
+static void debug_set(const char *what, const char *match, struct git_attr *attr, void *v)
 {
-	fprintf(stderr, "%s: %s => %d (%s)\n",
-		what, attr->name, set, match);
+	const char *value = v;
+
+	if (ATTR_TRUE(value))
+		value = "set";
+	else if (ATTR_FALSE(value))
+		value = "unset";
+	else if (ATTR_UNSET(value))
+		value = "unspecified";
+
+	fprintf(stderr, "%s: %s => %s (%s)\n",
+		what, attr->name, (char *) value, match);
 }
 #define debug_push(a) debug_info("push", (a))
 #define debug_pop(a) debug_info("pop", (a))
@@ -420,56 +471,53 @@ static int path_matches(const char *pathname, int pathlen,
 	return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
 }
 
+static int fill_one(const char *what, struct match_attr *a, int rem)
+{
+	struct git_attr_check *check = check_all_attr;
+	int i;
+
+	for (i = 0; 0 < rem && i < a->num_attr; i++) {
+		struct git_attr *attr = a->state[i].attr;
+		void **n = &(check[attr->attr_nr].value);
+		void *v = a->state[i].setto;
+
+		if (*n == ATTR__UNKNOWN) {
+			debug_set(what, a->u.pattern, attr, v);
+			*n = v;
+			rem--;
+		}
+	}
+	return rem;
+}
+
 static int fill(const char *path, int pathlen, struct attr_stack *stk, int rem)
 {
+	int i;
 	const char *base = stk->origin ? stk->origin : "";
-	int i, j;
-	struct git_attr_check *check = check_all_attr;
 
 	for (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {
 		struct match_attr *a = stk->attrs[i];
 		if (a->is_macro)
 			continue;
 		if (path_matches(path, pathlen,
-				 a->u.pattern, base, strlen(base))) {
-			for (j = 0; 0 < rem && j < a->num_attr; j++) {
-				struct git_attr *attr = a->state[j].attr;
-				int set = !a->state[j].unset;
-				int *n = &(check[attr->attr_nr].isset);
-
-				if (*n < 0) {
-					debug_set("fill", a->u.pattern, attr, set);
-					*n = set;
-					rem--;
-				}
-			}
-		}
+				 a->u.pattern, base, strlen(base)))
+			rem = fill_one("fill", a, rem);
 	}
 	return rem;
 }
 
 static int macroexpand(struct attr_stack *stk, int rem)
 {
-	int i, j;
+	int i;
 	struct git_attr_check *check = check_all_attr;
 
 	for (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {
 		struct match_attr *a = stk->attrs[i];
 		if (!a->is_macro)
 			continue;
-		if (check[a->u.attr->attr_nr].isset < 0)
+		if (check[a->u.attr->attr_nr].value != ATTR__TRUE)
 			continue;
-		for (j = 0; 0 < rem && j < a->num_attr; j++) {
-			struct git_attr *attr = a->state[j].attr;
-			int set = !a->state[j].unset;
-			int *n = &(check[attr->attr_nr].isset);
-
-			if (*n < 0) {
-				debug_set("expand", a->u.attr->name, attr, set);
-				*n = set;
-				rem--;
-			}
-		}
+		rem = fill_one("expand", a, rem);
 	}
 	return rem;
 }
@@ -482,7 +530,7 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 
 	bootstrap_attr_stack();
 	for (i = 0; i < attr_nr; i++)
-		check_all_attr[i].isset = -1;
+		check_all_attr[i].value = ATTR__UNKNOWN;
 
 	pathlen = strlen(path);
 	cp = strrchr(path, '/');
@@ -498,8 +546,12 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)
 	for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
 		rem = macroexpand(stk, rem);
 
-	for (i = 0; i < num; i++)
-		check[i].isset = check_all_attr[check[i].attr->attr_nr].isset;
+	for (i = 0; i < num; i++) {
+		void *value = check_all_attr[check[i].attr->attr_nr].value;
+		if (value == ATTR__UNKNOWN)
+			value = ATTR__UNSET;
+		check[i].value = value;
+	}
 
 	return 0;
 }
diff --git a/attr.h b/attr.h
index 1e5ab40..8ec2d3d 100644
--- a/attr.h
+++ b/attr.h
@@ -6,9 +6,19 @@ struct git_attr;
 
 struct git_attr *git_attr(const char *, int);
 
+/* Internal use */
+#define ATTR__TRUE	((void *) 1)
+#define ATTR__FALSE	((void *) 0)
+#define ATTR__UNSET	((void *) -1)
+
+/* For public to check git_attr_check results */
+#define ATTR_TRUE(v) ((v) == ATTR__TRUE)
+#define ATTR_FALSE(v) ((v) == ATTR__FALSE)
+#define ATTR_UNSET(v) ((v) == ATTR__UNSET)
+
 struct git_attr_check {
 	struct git_attr *attr;
-	int isset;
+	void *value;
 };
 
 int git_checkattr(const char *path, int, struct git_attr_check *);
diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index 634be9e..6983a73 100644
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -42,11 +42,17 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
 		if (git_checkattr(argv[i], cnt, check))
 			die("git_checkattr died");
 		for (j = 0; j < cnt; j++) {
+			void *value = check[j].value;
+
+			if (ATTR_TRUE(value))
+				value = "set";
+			else if (ATTR_FALSE(value))
+				value = "unset";
+			else if (ATTR_UNSET(value))
+				value = "unspecified";
+
 			write_name_quoted("", 0, argv[i], 1, stdout);
-			printf(": %s: %s\n", argv[j+1],
-			       (check[j].isset < 0) ? "unspecified" :
-			       (check[j].isset == 0) ? "unset" :
-			       "set");
+			printf(": %s: %s\n", argv[j+1], (char *) value);
 		}
 	}
 	return 0;
diff --git a/convert.c b/convert.c
index d0d4b81..68bb70f 100644
--- a/convert.c
+++ b/convert.c
@@ -225,9 +225,19 @@ static int git_path_check_crlf(const char *path)
 
 	setup_crlf_check(&attr_crlf_check);
 
-	if (git_checkattr(path, 1, &attr_crlf_check))
-		return -1;
-	return attr_crlf_check.isset;
+	if (!git_checkattr(path, 1, &attr_crlf_check)) {
+		void *value = attr_crlf_check.value;
+		if (ATTR_TRUE(value))
+			return 1;
+		else if (ATTR_FALSE(value))
+			return 0;
+		else if (ATTR_UNSET(value))
+			;
+		else
+			die("unknown value %s given to 'crlf' attribute",
+			    (char *)value);
+	}
+	return -1;
 }
 
 int convert_to_git(const char *path, char **bufp, unsigned long *sizep)
diff --git a/diff.c b/diff.c
index dcea405..a32078e 100644
--- a/diff.c
+++ b/diff.c
@@ -1068,9 +1068,18 @@ static int file_is_binary(struct diff_filespec *one)
 	struct git_attr_check attr_diff_check;
 
 	setup_diff_attr_check(&attr_diff_check);
-	if (!git_checkattr(one->path, 1, &attr_diff_check) &&
-	    (0 <= attr_diff_check.isset))
-		return !attr_diff_check.isset;
+	if (!git_checkattr(one->path, 1, &attr_diff_check)) {
+		void *value = attr_diff_check.value;
+		if (ATTR_TRUE(value))
+			return 0;
+		else if (ATTR_FALSE(value))
+			return 1;
+		else if (ATTR_UNSET(value))
+			;
+		else
+			die("unknown value %s given to 'diff' attribute",
+			    (char *)value);
+	}
 
 	if (!one->data) {
 		if (!DIFF_FILE_VALID(one))
-- 
1.5.1.1.821.g88bdb

[PATCH 2/4] merge-recursive: separate out xdl_merge() interface.

[Junio C Hamano]

This just moves code around to make the actual call to
xdl_merge() into a separate function.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 merge-recursive.c |   56 +++++++++++++++++++++++++++++++++-------------------
 1 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 3096594..4eb62cf 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -659,6 +659,39 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
 	mm->size = size;
 }
 
+static int ll_merge(mmbuffer_t *result_buf,
+		    struct diff_filespec *o,
+		    struct diff_filespec *a,
+		    struct diff_filespec *b,
+		    const char *branch1,
+		    const char *branch2)
+{
+	mmfile_t orig, src1, src2;
+	xpparam_t xpp;
+	char *name1, *name2;
+	int merge_status;
+
+	name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
+	name2 = xstrdup(mkpath("%s:%s", branch2, b->path));
+
+	fill_mm(o->sha1, &orig);
+	fill_mm(a->sha1, &src1);
+	fill_mm(b->sha1, &src2);
+
+	memset(&xpp, 0, sizeof(xpp));
+	merge_status = xdl_merge(&orig,
+				 &src1, name1,
+				 &src2, name2,
+				 &xpp, XDL_MERGE_ZEALOUS,
+				 result_buf);
+	free(name1);
+	free(name2);
+	free(orig.ptr);
+	free(src1.ptr);
+	free(src2.ptr);
+	return merge_status;
+}
+
 static struct merge_file_info merge_file(struct diff_filespec *o,
 		struct diff_filespec *a, struct diff_filespec *b,
 		const char *branch1, const char *branch2)
@@ -687,30 +720,11 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
 		else if (sha_eq(b->sha1, o->sha1))
 			hashcpy(result.sha, a->sha1);
 		else if (S_ISREG(a->mode)) {
-			mmfile_t orig, src1, src2;
 			mmbuffer_t result_buf;
-			xpparam_t xpp;
-			char *name1, *name2;
 			int merge_status;
 
-			name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
-			name2 = xstrdup(mkpath("%s:%s", branch2, b->path));
-
-			fill_mm(o->sha1, &orig);
-			fill_mm(a->sha1, &src1);
-			fill_mm(b->sha1, &src2);
-
-			memset(&xpp, 0, sizeof(xpp));
-			merge_status = xdl_merge(&orig,
-						 &src1, name1,
-						 &src2, name2,
-						 &xpp, XDL_MERGE_ZEALOUS,
-						 &result_buf);
-			free(name1);
-			free(name2);
-			free(orig.ptr);
-			free(src1.ptr);
-			free(src2.ptr);
+			merge_status = ll_merge(&result_buf, o, a, b,
+						branch1, branch2);
 
 			if ((merge_status < 0) || !result_buf.ptr)
 				die("Failed to execute internal merge");
-- 
1.5.1.1.821.g88bdb

[PATCH 3/4] Allow specifying specialized merge-backend per path.

[Junio C Hamano]

This allows 'merge' attribute to control how the file-level
three-way merge is done per path.

 - If you set 'merge' to true or leave unspecified, we use the
   built-in 3-way xdl-merge.

 - If you set 'merge' to false, the merge result is 'ours'.  But
   this still leaves the path conflicted, so that the mess can
   be sorted out by the user.  This is probably useful for
   binary files.

 - 'merge=union' (this is the first example of a string valued
   attribute, introduced in the previous one) uses the "union"
   merge.  The "union" merge takes lines in conflicted hunks
   from both sides, which is useful for line-oriented files such
   as .gitignore.

Currently there is no way to specify random programs but it
should be trivial for motivated contributors to add later.

There is one caveat, though.  ll_merge() is called for both
internal ancestor merge and the outer "final" merge.  I think an
interactive custom per-path merge backend should refrain from
going interactive when performing an internal merge (you can
tell it by checking call_depth) and instead just call either
ll_xdl_merge() if the content is text, or call ll_ours_merge()
otherwise.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 merge-recursive.c |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 126 insertions(+), 7 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 4eb62cf..a6c08a1 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -15,6 +15,7 @@
 #include "unpack-trees.h"
 #include "path-list.h"
 #include "xdiff-interface.h"
+#include "attr.h"
 
 static int subtree_merge;
 
@@ -659,6 +660,124 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
 	mm->size = size;
 }
 
+/* Low-level merge functions */
+typedef int (*ll_merge_fn)(mmfile_t *orig,
+			   mmfile_t *src1, const char *name1,
+			   mmfile_t *src2, const char *name2,
+			   mmbuffer_t *result);
+
+static int ll_xdl_merge(mmfile_t *orig,
+			mmfile_t *src1, const char *name1,
+			mmfile_t *src2, const char *name2,
+			mmbuffer_t *result)
+{
+	xpparam_t xpp;
+	memset(&xpp, 0, sizeof(xpp));
+	memset(&xpp, 0, sizeof(xpp));
+	return xdl_merge(orig,
+			 src1, name1,
+			 src2, name2,
+			 &xpp, XDL_MERGE_ZEALOUS,
+			 result);
+}
+
+static int ll_union_merge(mmfile_t *orig,
+			  mmfile_t *src1, const char *name1,
+			  mmfile_t *src2, const char *name2,
+			  mmbuffer_t *result)
+{
+	char *src, *dst;
+	long size;
+	const int marker_size = 7;
+
+	int status = ll_xdl_merge(orig, src1, NULL, src2, NULL, result);
+	if (status <= 0)
+		return status;
+	size = result->size;
+	src = dst = result->ptr;
+	while (size) {
+		char ch;
+		if ((marker_size < size) &&
+		    (*src == '<' || *src == '=' || *src == '>')) {
+			int i;
+			ch = *src;
+			for (i = 0; i < marker_size; i++)
+				if (src[i] != ch)
+					goto not_a_marker;
+			if (src[marker_size] != '\n')
+				goto not_a_marker;
+			src += marker_size + 1;
+			size -= marker_size + 1;
+			continue;
+		}
+	not_a_marker:
+		do {
+			ch = *src++;
+			*dst++ = ch;
+			size--;
+		} while (ch != '\n' && size);
+	}
+	result->size = dst - result->ptr;
+	return 0;
+}
+
+static int ll_ours_merge(mmfile_t *orig,
+			 mmfile_t *src1, const char *name1,
+			 mmfile_t *src2, const char *name2,
+			 mmbuffer_t *result)
+{
+	/*
+	 * Pretend the tentative merge result is "ours",
+	 * but still return "conflicted merge" status.
+	 */
+	result->ptr = src1->ptr;
+	result->size = src1->size;
+	src1->ptr = NULL;
+	return 1;
+}
+
+static struct {
+	const char *name;
+	ll_merge_fn fn;
+} ll_merge_fns[] = {
+	{ "3way", ll_xdl_merge },
+	{ "ours", ll_ours_merge },
+	{ "union", ll_union_merge },
+	{ NULL, NULL },
+};
+
+static ll_merge_fn find_ll_merge_fn(void *merge_attr)
+{
+	const char *name;
+	int i;
+
+	if (ATTR_TRUE(merge_attr) || ATTR_UNSET(merge_attr))
+		return ll_xdl_merge;
+	else if (ATTR_FALSE(merge_attr))
+		return ll_ours_merge;
+
+	/* Otherwise merge_attr may name the merge function */
+	name = merge_attr;
+	for (i = 0; ll_merge_fns[i].name; i++)
+		if (!strcmp(ll_merge_fns[i].name, name))
+			return ll_merge_fns[i].fn;
+
+	/* default to the 3-way */
+	return ll_xdl_merge;
+}
+
+static void *git_path_check_merge(const char *path)
+{
+	static struct git_attr_check attr_merge_check;
+
+	if (!attr_merge_check.attr)
+		attr_merge_check.attr = git_attr("merge", 5);
+
+	if (git_checkattr(path, 1, &attr_merge_check))
+		return ATTR__UNSET;
+	return attr_merge_check.value;
+}
+
 static int ll_merge(mmbuffer_t *result_buf,
 		    struct diff_filespec *o,
 		    struct diff_filespec *a,
@@ -667,9 +786,10 @@ static int ll_merge(mmbuffer_t *result_buf,
 		    const char *branch2)
 {
 	mmfile_t orig, src1, src2;
-	xpparam_t xpp;
 	char *name1, *name2;
 	int merge_status;
+	void *merge_attr;
+	ll_merge_fn fn;
 
 	name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
 	name2 = xstrdup(mkpath("%s:%s", branch2, b->path));
@@ -678,12 +798,11 @@ static int ll_merge(mmbuffer_t *result_buf,
 	fill_mm(a->sha1, &src1);
 	fill_mm(b->sha1, &src2);
 
-	memset(&xpp, 0, sizeof(xpp));
-	merge_status = xdl_merge(&orig,
-				 &src1, name1,
-				 &src2, name2,
-				 &xpp, XDL_MERGE_ZEALOUS,
-				 result_buf);
+	merge_attr = git_path_check_merge(a->path);
+	fn = find_ll_merge_fn(merge_attr);
+
+	merge_status = fn(&orig, &src1, name1, &src2, name2, result_buf);
+
 	free(name1);
 	free(name2);
 	free(orig.ptr);
-- 
1.5.1.1.821.g88bdb

[PATCH 4/4] Add a demonstration/test of customized merge.

[Junio C Hamano]

This demonstrates how the new low-level per-path merge backends,
union and ours, and shows how they are controlled by the
gitattribute mechanism.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 t/t6026-merge-attr.sh |   72 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100755 t/t6026-merge-attr.sh

diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
new file mode 100755
index 0000000..5daa223
--- /dev/null
+++ b/t/t6026-merge-attr.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Junio C Hamano
+#
+
+test_description='per path merge controlled by merge attribute'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	for f in text binary union
+	do
+		echo Initial >$f && git add $f || break
+	done &&
+	test_tick &&
+	git commit -m Initial &&
+
+	git branch side &&
+	for f in text binary union
+	do
+		echo Master >>$f && git add $f || break
+	done &&
+	test_tick &&
+	git commit -m Master &&
+
+	git checkout side &&
+	for f in text binary union
+	do
+		echo Side >>$f && git add $f || break
+	done &&
+	test_tick &&
+	git commit -m Side
+
+'
+
+test_expect_success merge '
+
+	{
+		echo "binary -merge"
+		echo "union merge=union"
+	} >.gitattributes &&
+
+	if git merge master
+	then
+		echo Gaah, should have conflicted
+		false
+	else
+		echo Ok, conflicted.
+	fi
+'
+
+test_expect_success 'check merge result in index' '
+
+	git ls-files -u | grep binary &&
+	git ls-files -u | grep text &&
+	! (git ls-files -u | grep union)
+
+'
+
+test_expect_success 'check merge result in working tree' '
+
+	git cat-file -p HEAD:binary >binary-orig &&
+	grep "<<<<<<<" text &&
+	cmp binary-orig binary &&
+	! grep "<<<<<<<" union &&
+	grep Master union &&
+	grep Side union
+
+'
+
+test_done
-- 
1.5.1.1.821.g88bdb

Re: [PATCH 3/4] Allow specifying specialized merge-backend per path.

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

I hate to nitpick my own patches, but...

> There is one caveat, though.  ll_merge() is called for both
> internal ancestor merge and the outer "final" merge.  I think an
> interactive custom per-path merge backend should refrain from
> going interactive when performing an internal merge (you can
> tell it by checking call_depth) and instead just call either
> ll_xdl_merge() if the content is text, or call ll_ours_merge()
> otherwise.

After having thought about this a bit more, I think a conflict
in *internal* merge for a binary file should probably take the
ancestor's, instead of ours.

A conflict during the internal merge happens when there are
multiple common ancestors, and the development lines continuing
from these ancestores may have disagreed on the result of this
conflicting merge earlier.

             o---A---C---o---Y
            /     \ /         \
          GP       .           M
            \     / \         /
             o---B---D---o---Z

When we are trying to do the merge M between Y and Z, we find
two closest common ancestors, A and B.  An internal merge to
merge them, using the grandparent ancestor, GP, is attempted,
and we will use the result as the virtual common ancestor to
create a merge between Y and Z.

The fact the internal merge between A and B conflicted means
that the development lines leading to Y and Z already saw that
same conflict before reaching Y and Z (in the above picture, at
C and D, respectively), and they might have resolved it
differently.

When it is a text file, Fredrik's clever "recursive merge"
leaves conflicted merge as the common ancestor, which is
different from either C or D's resolution (perhaps we can think
of it being somewhere areound sign X in the above picture), and
merge between Y and Z using that as the common ancestor cancels
the conflict out if C and D resolved it the same way.

However, when the file is a binary guck, the result of such a
half-merge with conflict markers is simply a corrupt binary
data, and is unusable even for inspection.  That was the reason
why I initially suggested to use 'ours'.  But that means, when
we are on the branch that leads to Y, we essentially favor C's
resolution, defeating the "accidental revert avoidance" the
recursive merge strategy offers.  If instead we take the version
from GP as the common ancestor, we have a usable binary guck as
an ancestor as well, and also we are being neutral between the
branches, giving the end user a chance to make an unbiased
decision on his own.

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 4eb62cf..a6c08a1 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -659,6 +660,124 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
>  	mm->size = size;
>  }
>  
> +/* Low-level merge functions */
> +typedef int (*ll_merge_fn)(mmfile_t *orig,
> +			   mmfile_t *src1, const char *name1,
> +			   mmfile_t *src2, const char *name2,
> +			   mmbuffer_t *result);
> +
> +static int ll_xdl_merge(mmfile_t *orig,
> +			mmfile_t *src1, const char *name1,
> +			mmfile_t *src2, const char *name2,
> +			mmbuffer_t *result)
> +{
> +	xpparam_t xpp;
> +	memset(&xpp, 0, sizeof(xpp));
> +	memset(&xpp, 0, sizeof(xpp));

I caught this after sending this message, and already fixed it
in the version I pushed out on 'pu'.

Re: [PATCH 3/4] Allow specifying specialized merge-backend per path.

[Linus Torvalds]

On Tue, 17 Apr 2007, Junio C Hamano wrote:
>
> This allows 'merge' attribute to control how the file-level
> three-way merge is done per path.

Cool. Good job. The whole attribute thing seems to be really working out.

		Linus