{"thread":{"id":"7711","subject":"[PATCH 2/4] merge-recursive: separate out xdl_merge() interface.","startedAt":"2007-04-17T08:08:35Z","lastAt":"2007-04-17T15:16:13Z","messageCount":6,"participants":["Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"39618","messageId":"11767973183627-git-send-email-junkio@cox.net","threadId":"7711","inReplyTo":null,"subject":"[PATCH 1/4] Allow more than true/false to attributes.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T08:08:35Z","receivedAt":"2007-04-17T08:08:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This allows you to define three values (and possibly more) to\neach attribute: true, false, and unset.\n\nTypically the handlers that notice and act on attribute values\ntreat \"unset\" attribute to mean \"do your default thing\"\n(e.g. crlf that is unset would trigger \"guess from contents\"),\nso being able to override a setting to an unset state is\nactually useful.\n\n - If you want to set the attribute value to true, have an entry\n   in .gitattributes file that mentions the attribute name; e.g.\n\n\t*.o\tbinary\n\n - If you want to set the attribute value explicitly to false,\n   use '-'; e.g.\n\n\t*.a\t-diff\n\n - If you want to make the attribute value _unset_, perhaps to\n   override an earlier entry, use '!'; e.g.\n\n\t*.a\t-diff\n\tc.i.a\t!diff\n\nThis also allows string values to attributes, with the natural\nsyntax:\n\n\tattrname=attrvalue\n\nbut you cannot use it, as nobody takes notice and acts on\nit yet.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n attr.c               |  194 +++++++++++++++++++++++++++++++------------------\n attr.h               |   12 +++-\n builtin-check-attr.c |   14 +++-\n convert.c            |   16 ++++-\n diff.c               |   15 +++-\n 5 files changed, 169 insertions(+), 82 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 60fe48f..b3496a6 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -1,6 +1,8 @@\n #include \"cache.h\"\n #include \"attr.h\"\n \n+#define ATTR__UNKNOWN\t((void *) -2)\n+\n /*\n  * The basic design decision here is that we are not going to have\n  * insanely large number of attributes.\n@@ -83,6 +85,7 @@ struct git_attr *git_attr(const char *name, int len)\n \tcheck_all_attr = xrealloc(check_all_attr,\n \t\t\t\t  sizeof(*check_all_attr) * attr_nr);\n \tcheck_all_attr[a->attr_nr].attr = a;\n+\tcheck_all_attr[a->attr_nr].value = ATTR__UNKNOWN;\n \treturn a;\n }\n \n@@ -92,12 +95,14 @@ struct git_attr *git_attr(const char *name, int len)\n  * (1) glob pattern.\n  * (2) whitespace\n  * (3) whitespace separated list of attribute names, each of which\n- *     could be prefixed with '-' to mean \"not set\".\n+ *     could be prefixed with '-' to mean \"set to false\", '!' to mean\n+ *     \"unset\".\n  */\n \n+/* What does a matched pattern decide? */\n struct attr_state {\n-\tint unset;\n \tstruct git_attr *attr;\n+\tvoid *setto;\n };\n \n struct match_attr {\n@@ -112,13 +117,63 @@ struct match_attr {\n \n static const char blank[] = \" \\t\\r\\n\";\n \n+static const char *parse_attr(const char *src, int lineno, const char *cp,\n+\t\t\t      int *num_attr, struct match_attr *res)\n+{\n+\tconst char *ep, *equals;\n+\tint len;\n+\n+\tep = cp + strcspn(cp, blank);\n+\tequals = strchr(cp, '=');\n+\tif (equals && ep < equals)\n+\t\tequals = NULL;\n+\tif (equals)\n+\t\tlen = equals - cp;\n+\telse\n+\t\tlen = ep - cp;\n+\tif (!res) {\n+\t\tif (*cp == '-' || *cp == '!') {\n+\t\t\tcp++;\n+\t\t\tlen--;\n+\t\t}\n+\t\tif (invalid_attr_name(cp, len)) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"%.*s is not a valid attribute name: %s:%d\\n\",\n+\t\t\t\tlen, cp, src, lineno);\n+\t\t\treturn NULL;\n+\t\t}\n+\t} else {\n+\t\tstruct attr_state *e;\n+\n+\t\te = &(res->state[*num_attr]);\n+\t\tif (*cp == '-' || *cp == '!') {\n+\t\t\te->setto = (*cp == '-') ? ATTR__FALSE : ATTR__UNSET;\n+\t\t\tcp++;\n+\t\t\tlen--;\n+\t\t}\n+\t\telse if (!equals)\n+\t\t\te->setto = ATTR__TRUE;\n+\t\telse {\n+\t\t\tchar *value;\n+\t\t\tint vallen = ep - equals;\n+\t\t\tvalue = xmalloc(vallen);\n+\t\t\tmemcpy(value, equals+1, vallen-1);\n+\t\t\tvalue[vallen-1] = 0;\n+\t\t\te->setto = value;\n+\t\t}\n+\t\te->attr = git_attr(cp, len);\n+\t}\n+\t(*num_attr)++;\n+\treturn ep + strspn(ep, blank);\n+}\n+\n static struct match_attr *parse_attr_line(const char *line, const char *src,\n \t\t\t\t\t  int lineno, int macro_ok)\n {\n \tint namelen;\n \tint num_attr;\n \tconst char *cp, *name;\n-\tstruct match_attr *res = res;\n+\tstruct match_attr *res = NULL;\n \tint pass;\n \tint is_macro;\n \n@@ -153,42 +208,16 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,\n \t\tnum_attr = 0;\n \t\tcp = name + namelen;\n \t\tcp = cp + strspn(cp, blank);\n-\t\twhile (*cp) {\n-\t\t\tconst char *ep;\n-\t\t\tep = cp + strcspn(cp, blank);\n-\t\t\tif (!pass) {\n-\t\t\t\tif (*cp == '-')\n-\t\t\t\t\tcp++;\n-\t\t\t\tif (invalid_attr_name(cp, ep - cp)) {\n-\t\t\t\t\tfprintf(stderr,\n-\t\t\t\t\t\t\"%.*s is not a valid attribute name: %s:%d\\n\",\n-\t\t\t\t\t\t(int)(ep - cp), cp,\n-\t\t\t\t\t\tsrc, lineno);\n-\t\t\t\t\treturn NULL;\n-\t\t\t\t}\n-\t\t\t} else {\n-\t\t\t\tstruct attr_state *e;\n-\n-\t\t\t\te = &(res->state[num_attr]);\n-\t\t\t\tif (*cp == '-') {\n-\t\t\t\t\te->unset = 1;\n-\t\t\t\t\tcp++;\n-\t\t\t\t}\n-\t\t\t\te->attr = git_attr(cp, ep - cp);\n-\t\t\t}\n-\t\t\tnum_attr++;\n-\t\t\tcp = ep + strspn(ep, blank);\n-\t\t}\n+\t\twhile (*cp)\n+\t\t\tcp = parse_attr(src, lineno, cp, &num_attr, res);\n \t\tif (pass)\n \t\t\tbreak;\n-\n \t\tres = xcalloc(1,\n \t\t\t      sizeof(*res) +\n \t\t\t      sizeof(struct attr_state) * num_attr +\n \t\t\t      (is_macro ? 0 : namelen + 1));\n-\t\tif (is_macro) {\n+\t\tif (is_macro)\n \t\t\tres->u.attr = git_attr(name, namelen);\n-\t\t}\n \t\telse {\n \t\t\tres->u.pattern = (char*)&(res->state[num_attr]);\n \t\t\tmemcpy(res->u.pattern, name, namelen);\n@@ -205,9 +234,9 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,\n  * come from many places.\n  *\n  * (1) .gitattribute file of the same directory;\n- * (2) .gitattribute file of the parent directory if (1) does not have any match;\n- *     this goes recursively upwards, just like .gitignore\n- * (3) perhaps $GIT_DIR/info/attributes, as the final fallback.\n+ * (2) .gitattribute file of the parent directory if (1) does not have\n+ *      any match; this goes recursively upwards, just like .gitignore.\n+ * (3) $GIT_DIR/info/attributes, which overrides both of the above.\n  *\n  * In the same file, later entries override the earlier match, so in the\n  * global list, we would have entries from info/attributes the earliest\n@@ -229,8 +258,21 @@ static void free_attr_elem(struct attr_stack *e)\n {\n \tint i;\n \tfree(e->origin);\n-\tfor (i = 0; i < e->num_matches; i++)\n-\t\tfree(e->attrs[i]);\n+\tfor (i = 0; i < e->num_matches; i++) {\n+\t\tstruct match_attr *a = e->attrs[i];\n+\t\tint j;\n+\t\tfor (j = 0; j < a->num_attr; j++) {\n+\t\t\tvoid *setto = a->state[j].setto;\n+\t\t\tif (setto == ATTR__TRUE ||\n+\t\t\t    setto == ATTR__FALSE ||\n+\t\t\t    setto == ATTR__UNSET ||\n+\t\t\t    setto == ATTR__UNKNOWN)\n+\t\t\t\t;\n+\t\t\telse\n+\t\t\t\tfree(setto);\n+\t\t}\n+\t\tfree(a);\n+\t}\n \tfree(e);\n }\n \n@@ -288,10 +330,19 @@ static void debug_info(const char *what, struct attr_stack *elem)\n {\n \tfprintf(stderr, \"%s: %s\\n\", what, elem->origin ? elem->origin : \"()\");\n }\n-static void debug_set(const char *what, const char *match, struct git_attr *attr, int set)\n+static void debug_set(const char *what, const char *match, struct git_attr *attr, void *v)\n {\n-\tfprintf(stderr, \"%s: %s => %d (%s)\\n\",\n-\t\twhat, attr->name, set, match);\n+\tconst char *value = v;\n+\n+\tif (ATTR_TRUE(value))\n+\t\tvalue = \"set\";\n+\telse if (ATTR_FALSE(value))\n+\t\tvalue = \"unset\";\n+\telse if (ATTR_UNSET(value))\n+\t\tvalue = \"unspecified\";\n+\n+\tfprintf(stderr, \"%s: %s => %s (%s)\\n\",\n+\t\twhat, attr->name, (char *) value, match);\n }\n #define debug_push(a) debug_info(\"push\", (a))\n #define debug_pop(a) debug_info(\"pop\", (a))\n@@ -420,56 +471,53 @@ static int path_matches(const char *pathname, int pathlen,\n \treturn fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;\n }\n \n+static int fill_one(const char *what, struct match_attr *a, int rem)\n+{\n+\tstruct git_attr_check *check = check_all_attr;\n+\tint i;\n+\n+\tfor (i = 0; 0 < rem && i < a->num_attr; i++) {\n+\t\tstruct git_attr *attr = a->state[i].attr;\n+\t\tvoid **n = &(check[attr->attr_nr].value);\n+\t\tvoid *v = a->state[i].setto;\n+\n+\t\tif (*n == ATTR__UNKNOWN) {\n+\t\t\tdebug_set(what, a->u.pattern, attr, v);\n+\t\t\t*n = v;\n+\t\t\trem--;\n+\t\t}\n+\t}\n+\treturn rem;\n+}\n+\n static int fill(const char *path, int pathlen, struct attr_stack *stk, int rem)\n {\n+\tint i;\n \tconst char *base = stk->origin ? stk->origin : \"\";\n-\tint i, j;\n-\tstruct git_attr_check *check = check_all_attr;\n \n \tfor (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {\n \t\tstruct match_attr *a = stk->attrs[i];\n \t\tif (a->is_macro)\n \t\t\tcontinue;\n \t\tif (path_matches(path, pathlen,\n-\t\t\t\t a->u.pattern, base, strlen(base))) {\n-\t\t\tfor (j = 0; 0 < rem && j < a->num_attr; j++) {\n-\t\t\t\tstruct git_attr *attr = a->state[j].attr;\n-\t\t\t\tint set = !a->state[j].unset;\n-\t\t\t\tint *n = &(check[attr->attr_nr].isset);\n-\n-\t\t\t\tif (*n < 0) {\n-\t\t\t\t\tdebug_set(\"fill\", a->u.pattern, attr, set);\n-\t\t\t\t\t*n = set;\n-\t\t\t\t\trem--;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t}\n+\t\t\t\t a->u.pattern, base, strlen(base)))\n+\t\t\trem = fill_one(\"fill\", a, rem);\n \t}\n \treturn rem;\n }\n \n static int macroexpand(struct attr_stack *stk, int rem)\n {\n-\tint i, j;\n+\tint i;\n \tstruct git_attr_check *check = check_all_attr;\n \n \tfor (i = stk->num_matches - 1; 0 < rem && 0 <= i; i--) {\n \t\tstruct match_attr *a = stk->attrs[i];\n \t\tif (!a->is_macro)\n \t\t\tcontinue;\n-\t\tif (check[a->u.attr->attr_nr].isset < 0)\n+\t\tif (check[a->u.attr->attr_nr].value != ATTR__TRUE)\n \t\t\tcontinue;\n-\t\tfor (j = 0; 0 < rem && j < a->num_attr; j++) {\n-\t\t\tstruct git_attr *attr = a->state[j].attr;\n-\t\t\tint set = !a->state[j].unset;\n-\t\t\tint *n = &(check[attr->attr_nr].isset);\n-\n-\t\t\tif (*n < 0) {\n-\t\t\t\tdebug_set(\"expand\", a->u.attr->name, attr, set);\n-\t\t\t\t*n = set;\n-\t\t\t\trem--;\n-\t\t\t}\n-\t\t}\n+\t\trem = fill_one(\"expand\", a, rem);\n \t}\n \treturn rem;\n }\n@@ -482,7 +530,7 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)\n \n \tbootstrap_attr_stack();\n \tfor (i = 0; i < attr_nr; i++)\n-\t\tcheck_all_attr[i].isset = -1;\n+\t\tcheck_all_attr[i].value = ATTR__UNKNOWN;\n \n \tpathlen = strlen(path);\n \tcp = strrchr(path, '/');\n@@ -498,8 +546,12 @@ int git_checkattr(const char *path, int num, struct git_attr_check *check)\n \tfor (stk = attr_stack; 0 < rem && stk; stk = stk->prev)\n \t\trem = macroexpand(stk, rem);\n \n-\tfor (i = 0; i < num; i++)\n-\t\tcheck[i].isset = check_all_attr[check[i].attr->attr_nr].isset;\n+\tfor (i = 0; i < num; i++) {\n+\t\tvoid *value = check_all_attr[check[i].attr->attr_nr].value;\n+\t\tif (value == ATTR__UNKNOWN)\n+\t\t\tvalue = ATTR__UNSET;\n+\t\tcheck[i].value = value;\n+\t}\n \n \treturn 0;\n }\ndiff --git a/attr.h b/attr.h\nindex 1e5ab40..8ec2d3d 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -6,9 +6,19 @@ struct git_attr;\n \n struct git_attr *git_attr(const char *, int);\n \n+/* Internal use */\n+#define ATTR__TRUE\t((void *) 1)\n+#define ATTR__FALSE\t((void *) 0)\n+#define ATTR__UNSET\t((void *) -1)\n+\n+/* For public to check git_attr_check results */\n+#define ATTR_TRUE(v) ((v) == ATTR__TRUE)\n+#define ATTR_FALSE(v) ((v) == ATTR__FALSE)\n+#define ATTR_UNSET(v) ((v) == ATTR__UNSET)\n+\n struct git_attr_check {\n \tstruct git_attr *attr;\n-\tint isset;\n+\tvoid *value;\n };\n \n int git_checkattr(const char *path, int, struct git_attr_check *);\ndiff --git a/builtin-check-attr.c b/builtin-check-attr.c\nindex 634be9e..6983a73 100644\n--- a/builtin-check-attr.c\n+++ b/builtin-check-attr.c\n@@ -42,11 +42,17 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \t\tif (git_checkattr(argv[i], cnt, check))\n \t\t\tdie(\"git_checkattr died\");\n \t\tfor (j = 0; j < cnt; j++) {\n+\t\t\tvoid *value = check[j].value;\n+\n+\t\t\tif (ATTR_TRUE(value))\n+\t\t\t\tvalue = \"set\";\n+\t\t\telse if (ATTR_FALSE(value))\n+\t\t\t\tvalue = \"unset\";\n+\t\t\telse if (ATTR_UNSET(value))\n+\t\t\t\tvalue = \"unspecified\";\n+\n \t\t\twrite_name_quoted(\"\", 0, argv[i], 1, stdout);\n-\t\t\tprintf(\": %s: %s\\n\", argv[j+1],\n-\t\t\t       (check[j].isset < 0) ? \"unspecified\" :\n-\t\t\t       (check[j].isset == 0) ? \"unset\" :\n-\t\t\t       \"set\");\n+\t\t\tprintf(\": %s: %s\\n\", argv[j+1], (char *) value);\n \t\t}\n \t}\n \treturn 0;\ndiff --git a/convert.c b/convert.c\nindex d0d4b81..68bb70f 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -225,9 +225,19 @@ static int git_path_check_crlf(const char *path)\n \n \tsetup_crlf_check(&attr_crlf_check);\n \n-\tif (git_checkattr(path, 1, &attr_crlf_check))\n-\t\treturn -1;\n-\treturn attr_crlf_check.isset;\n+\tif (!git_checkattr(path, 1, &attr_crlf_check)) {\n+\t\tvoid *value = attr_crlf_check.value;\n+\t\tif (ATTR_TRUE(value))\n+\t\t\treturn 1;\n+\t\telse if (ATTR_FALSE(value))\n+\t\t\treturn 0;\n+\t\telse if (ATTR_UNSET(value))\n+\t\t\t;\n+\t\telse\n+\t\t\tdie(\"unknown value %s given to 'crlf' attribute\",\n+\t\t\t    (char *)value);\n+\t}\n+\treturn -1;\n }\n \n int convert_to_git(const char *path, char **bufp, unsigned long *sizep)\ndiff --git a/diff.c b/diff.c\nindex dcea405..a32078e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1068,9 +1068,18 @@ static int file_is_binary(struct diff_filespec *one)\n \tstruct git_attr_check attr_diff_check;\n \n \tsetup_diff_attr_check(&attr_diff_check);\n-\tif (!git_checkattr(one->path, 1, &attr_diff_check) &&\n-\t    (0 <= attr_diff_check.isset))\n-\t\treturn !attr_diff_check.isset;\n+\tif (!git_checkattr(one->path, 1, &attr_diff_check)) {\n+\t\tvoid *value = attr_diff_check.value;\n+\t\tif (ATTR_TRUE(value))\n+\t\t\treturn 0;\n+\t\telse if (ATTR_FALSE(value))\n+\t\t\treturn 1;\n+\t\telse if (ATTR_UNSET(value))\n+\t\t\t;\n+\t\telse\n+\t\t\tdie(\"unknown value %s given to 'diff' attribute\",\n+\t\t\t    (char *)value);\n+\t}\n \n \tif (!one->data) {\n \t\tif (!DIFF_FILE_VALID(one))\n-- \n1.5.1.1.821.g88bdb\n"},{"id":"39616","messageId":"11767973192276-git-send-email-junkio@cox.net","threadId":"7711","inReplyTo":"11767973183627-git-send-email-junkio@cox.net","subject":"[PATCH 2/4] merge-recursive: separate out xdl_merge() interface.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T08:08:36Z","receivedAt":"2007-04-17T08:08:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This just moves code around to make the actual call to\nxdl_merge() into a separate function.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n merge-recursive.c |   56 +++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 35 insertions(+), 21 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 3096594..4eb62cf 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -659,6 +659,39 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n \tmm->size = size;\n }\n \n+static int ll_merge(mmbuffer_t *result_buf,\n+\t\t    struct diff_filespec *o,\n+\t\t    struct diff_filespec *a,\n+\t\t    struct diff_filespec *b,\n+\t\t    const char *branch1,\n+\t\t    const char *branch2)\n+{\n+\tmmfile_t orig, src1, src2;\n+\txpparam_t xpp;\n+\tchar *name1, *name2;\n+\tint merge_status;\n+\n+\tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n+\tname2 = xstrdup(mkpath(\"%s:%s\", branch2, b->path));\n+\n+\tfill_mm(o->sha1, &orig);\n+\tfill_mm(a->sha1, &src1);\n+\tfill_mm(b->sha1, &src2);\n+\n+\tmemset(&xpp, 0, sizeof(xpp));\n+\tmerge_status = xdl_merge(&orig,\n+\t\t\t\t &src1, name1,\n+\t\t\t\t &src2, name2,\n+\t\t\t\t &xpp, XDL_MERGE_ZEALOUS,\n+\t\t\t\t result_buf);\n+\tfree(name1);\n+\tfree(name2);\n+\tfree(orig.ptr);\n+\tfree(src1.ptr);\n+\tfree(src2.ptr);\n+\treturn merge_status;\n+}\n+\n static struct merge_file_info merge_file(struct diff_filespec *o,\n \t\tstruct diff_filespec *a, struct diff_filespec *b,\n \t\tconst char *branch1, const char *branch2)\n@@ -687,30 +720,11 @@ static struct merge_file_info merge_file(struct diff_filespec *o,\n \t\telse if (sha_eq(b->sha1, o->sha1))\n \t\t\thashcpy(result.sha, a->sha1);\n \t\telse if (S_ISREG(a->mode)) {\n-\t\t\tmmfile_t orig, src1, src2;\n \t\t\tmmbuffer_t result_buf;\n-\t\t\txpparam_t xpp;\n-\t\t\tchar *name1, *name2;\n \t\t\tint merge_status;\n \n-\t\t\tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n-\t\t\tname2 = xstrdup(mkpath(\"%s:%s\", branch2, b->path));\n-\n-\t\t\tfill_mm(o->sha1, &orig);\n-\t\t\tfill_mm(a->sha1, &src1);\n-\t\t\tfill_mm(b->sha1, &src2);\n-\n-\t\t\tmemset(&xpp, 0, sizeof(xpp));\n-\t\t\tmerge_status = xdl_merge(&orig,\n-\t\t\t\t\t\t &src1, name1,\n-\t\t\t\t\t\t &src2, name2,\n-\t\t\t\t\t\t &xpp, XDL_MERGE_ZEALOUS,\n-\t\t\t\t\t\t &result_buf);\n-\t\t\tfree(name1);\n-\t\t\tfree(name2);\n-\t\t\tfree(orig.ptr);\n-\t\t\tfree(src1.ptr);\n-\t\t\tfree(src2.ptr);\n+\t\t\tmerge_status = ll_merge(&result_buf, o, a, b,\n+\t\t\t\t\t\tbranch1, branch2);\n \n \t\t\tif ((merge_status < 0) || !result_buf.ptr)\n \t\t\t\tdie(\"Failed to execute internal merge\");\n-- \n1.5.1.1.821.g88bdb\n"},{"id":"39617","messageId":"11767973191314-git-send-email-junkio@cox.net","threadId":"7711","inReplyTo":"11767973183627-git-send-email-junkio@cox.net","subject":"[PATCH 3/4] Allow specifying specialized merge-backend per path.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T08:08:37Z","receivedAt":"2007-04-17T08:08:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This allows 'merge' attribute to control how the file-level\nthree-way merge is done per path.\n\n - If you set 'merge' to true or leave unspecified, we use the\n   built-in 3-way xdl-merge.\n\n - If you set 'merge' to false, the merge result is 'ours'.  But\n   this still leaves the path conflicted, so that the mess can\n   be sorted out by the user.  This is probably useful for\n   binary files.\n\n - 'merge=union' (this is the first example of a string valued\n   attribute, introduced in the previous one) uses the \"union\"\n   merge.  The \"union\" merge takes lines in conflicted hunks\n   from both sides, which is useful for line-oriented files such\n   as .gitignore.\n\nCurrently there is no way to specify random programs but it\nshould be trivial for motivated contributors to add later.\n\nThere is one caveat, though.  ll_merge() is called for both\ninternal ancestor merge and the outer \"final\" merge.  I think an\ninteractive custom per-path merge backend should refrain from\ngoing interactive when performing an internal merge (you can\ntell it by checking call_depth) and instead just call either\nll_xdl_merge() if the content is text, or call ll_ours_merge()\notherwise.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n merge-recursive.c |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 126 insertions(+), 7 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4eb62cf..a6c08a1 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -15,6 +15,7 @@\n #include \"unpack-trees.h\"\n #include \"path-list.h\"\n #include \"xdiff-interface.h\"\n+#include \"attr.h\"\n \n static int subtree_merge;\n \n@@ -659,6 +660,124 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n \tmm->size = size;\n }\n \n+/* Low-level merge functions */\n+typedef int (*ll_merge_fn)(mmfile_t *orig,\n+\t\t\t   mmfile_t *src1, const char *name1,\n+\t\t\t   mmfile_t *src2, const char *name2,\n+\t\t\t   mmbuffer_t *result);\n+\n+static int ll_xdl_merge(mmfile_t *orig,\n+\t\t\tmmfile_t *src1, const char *name1,\n+\t\t\tmmfile_t *src2, const char *name2,\n+\t\t\tmmbuffer_t *result)\n+{\n+\txpparam_t xpp;\n+\tmemset(&xpp, 0, sizeof(xpp));\n+\tmemset(&xpp, 0, sizeof(xpp));\n+\treturn xdl_merge(orig,\n+\t\t\t src1, name1,\n+\t\t\t src2, name2,\n+\t\t\t &xpp, XDL_MERGE_ZEALOUS,\n+\t\t\t result);\n+}\n+\n+static int ll_union_merge(mmfile_t *orig,\n+\t\t\t  mmfile_t *src1, const char *name1,\n+\t\t\t  mmfile_t *src2, const char *name2,\n+\t\t\t  mmbuffer_t *result)\n+{\n+\tchar *src, *dst;\n+\tlong size;\n+\tconst int marker_size = 7;\n+\n+\tint status = ll_xdl_merge(orig, src1, NULL, src2, NULL, result);\n+\tif (status <= 0)\n+\t\treturn status;\n+\tsize = result->size;\n+\tsrc = dst = result->ptr;\n+\twhile (size) {\n+\t\tchar ch;\n+\t\tif ((marker_size < size) &&\n+\t\t    (*src == '<' || *src == '=' || *src == '>')) {\n+\t\t\tint i;\n+\t\t\tch = *src;\n+\t\t\tfor (i = 0; i < marker_size; i++)\n+\t\t\t\tif (src[i] != ch)\n+\t\t\t\t\tgoto not_a_marker;\n+\t\t\tif (src[marker_size] != '\\n')\n+\t\t\t\tgoto not_a_marker;\n+\t\t\tsrc += marker_size + 1;\n+\t\t\tsize -= marker_size + 1;\n+\t\t\tcontinue;\n+\t\t}\n+\tnot_a_marker:\n+\t\tdo {\n+\t\t\tch = *src++;\n+\t\t\t*dst++ = ch;\n+\t\t\tsize--;\n+\t\t} while (ch != '\\n' && size);\n+\t}\n+\tresult->size = dst - result->ptr;\n+\treturn 0;\n+}\n+\n+static int ll_ours_merge(mmfile_t *orig,\n+\t\t\t mmfile_t *src1, const char *name1,\n+\t\t\t mmfile_t *src2, const char *name2,\n+\t\t\t mmbuffer_t *result)\n+{\n+\t/*\n+\t * Pretend the tentative merge result is \"ours\",\n+\t * but still return \"conflicted merge\" status.\n+\t */\n+\tresult->ptr = src1->ptr;\n+\tresult->size = src1->size;\n+\tsrc1->ptr = NULL;\n+\treturn 1;\n+}\n+\n+static struct {\n+\tconst char *name;\n+\tll_merge_fn fn;\n+} ll_merge_fns[] = {\n+\t{ \"3way\", ll_xdl_merge },\n+\t{ \"ours\", ll_ours_merge },\n+\t{ \"union\", ll_union_merge },\n+\t{ NULL, NULL },\n+};\n+\n+static ll_merge_fn find_ll_merge_fn(void *merge_attr)\n+{\n+\tconst char *name;\n+\tint i;\n+\n+\tif (ATTR_TRUE(merge_attr) || ATTR_UNSET(merge_attr))\n+\t\treturn ll_xdl_merge;\n+\telse if (ATTR_FALSE(merge_attr))\n+\t\treturn ll_ours_merge;\n+\n+\t/* Otherwise merge_attr may name the merge function */\n+\tname = merge_attr;\n+\tfor (i = 0; ll_merge_fns[i].name; i++)\n+\t\tif (!strcmp(ll_merge_fns[i].name, name))\n+\t\t\treturn ll_merge_fns[i].fn;\n+\n+\t/* default to the 3-way */\n+\treturn ll_xdl_merge;\n+}\n+\n+static void *git_path_check_merge(const char *path)\n+{\n+\tstatic struct git_attr_check attr_merge_check;\n+\n+\tif (!attr_merge_check.attr)\n+\t\tattr_merge_check.attr = git_attr(\"merge\", 5);\n+\n+\tif (git_checkattr(path, 1, &attr_merge_check))\n+\t\treturn ATTR__UNSET;\n+\treturn attr_merge_check.value;\n+}\n+\n static int ll_merge(mmbuffer_t *result_buf,\n \t\t    struct diff_filespec *o,\n \t\t    struct diff_filespec *a,\n@@ -667,9 +786,10 @@ static int ll_merge(mmbuffer_t *result_buf,\n \t\t    const char *branch2)\n {\n \tmmfile_t orig, src1, src2;\n-\txpparam_t xpp;\n \tchar *name1, *name2;\n \tint merge_status;\n+\tvoid *merge_attr;\n+\tll_merge_fn fn;\n \n \tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n \tname2 = xstrdup(mkpath(\"%s:%s\", branch2, b->path));\n@@ -678,12 +798,11 @@ static int ll_merge(mmbuffer_t *result_buf,\n \tfill_mm(a->sha1, &src1);\n \tfill_mm(b->sha1, &src2);\n \n-\tmemset(&xpp, 0, sizeof(xpp));\n-\tmerge_status = xdl_merge(&orig,\n-\t\t\t\t &src1, name1,\n-\t\t\t\t &src2, name2,\n-\t\t\t\t &xpp, XDL_MERGE_ZEALOUS,\n-\t\t\t\t result_buf);\n+\tmerge_attr = git_path_check_merge(a->path);\n+\tfn = find_ll_merge_fn(merge_attr);\n+\n+\tmerge_status = fn(&orig, &src1, name1, &src2, name2, result_buf);\n+\n \tfree(name1);\n \tfree(name2);\n \tfree(orig.ptr);\n-- \n1.5.1.1.821.g88bdb\n"},{"id":"39619","messageId":"11767973192181-git-send-email-junkio@cox.net","threadId":"7711","inReplyTo":"11767973183627-git-send-email-junkio@cox.net","subject":"[PATCH 4/4] Add a demonstration/test of customized merge.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T08:08:38Z","receivedAt":"2007-04-17T08:08:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This demonstrates how the new low-level per-path merge backends,\nunion and ours, and shows how they are controlled by the\ngitattribute mechanism.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n t/t6026-merge-attr.sh |   72 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 72 insertions(+), 0 deletions(-)\n create mode 100755 t/t6026-merge-attr.sh\n\ndiff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh\nnew file mode 100755\nindex 0000000..5daa223\n--- /dev/null\n+++ b/t/t6026-merge-attr.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Junio C Hamano\n+#\n+\n+test_description='per path merge controlled by merge attribute'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\n+\tfor f in text binary union\n+\tdo\n+\t\techo Initial >$f && git add $f || break\n+\tdone &&\n+\ttest_tick &&\n+\tgit commit -m Initial &&\n+\n+\tgit branch side &&\n+\tfor f in text binary union\n+\tdo\n+\t\techo Master >>$f && git add $f || break\n+\tdone &&\n+\ttest_tick &&\n+\tgit commit -m Master &&\n+\n+\tgit checkout side &&\n+\tfor f in text binary union\n+\tdo\n+\t\techo Side >>$f && git add $f || break\n+\tdone &&\n+\ttest_tick &&\n+\tgit commit -m Side\n+\n+'\n+\n+test_expect_success merge '\n+\n+\t{\n+\t\techo \"binary -merge\"\n+\t\techo \"union merge=union\"\n+\t} >.gitattributes &&\n+\n+\tif git merge master\n+\tthen\n+\t\techo Gaah, should have conflicted\n+\t\tfalse\n+\telse\n+\t\techo Ok, conflicted.\n+\tfi\n+'\n+\n+test_expect_success 'check merge result in index' '\n+\n+\tgit ls-files -u | grep binary &&\n+\tgit ls-files -u | grep text &&\n+\t! (git ls-files -u | grep union)\n+\n+'\n+\n+test_expect_success 'check merge result in working tree' '\n+\n+\tgit cat-file -p HEAD:binary >binary-orig &&\n+\tgrep \"<<<<<<<\" text &&\n+\tcmp binary-orig binary &&\n+\t! grep \"<<<<<<<\" union &&\n+\tgrep Master union &&\n+\tgrep Side union\n+\n+'\n+\n+test_done\n-- \n1.5.1.1.821.g88bdb\n"},{"id":"39623","messageId":"7vabx7qrvl.fsf@assigned-by-dhcp.cox.net","threadId":"7711","inReplyTo":"11767973191314-git-send-email-junkio@cox.net","subject":"Re: [PATCH 3/4] Allow specifying specialized merge-backend per path.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T09:20:30Z","receivedAt":"2007-04-17T09:20:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\nI hate to nitpick my own patches, but...\n\n> There is one caveat, though.  ll_merge() is called for both\n> internal ancestor merge and the outer \"final\" merge.  I think an\n> interactive custom per-path merge backend should refrain from\n> going interactive when performing an internal merge (you can\n> tell it by checking call_depth) and instead just call either\n> ll_xdl_merge() if the content is text, or call ll_ours_merge()\n> otherwise.\n\nAfter having thought about this a bit more, I think a conflict\nin *internal* merge for a binary file should probably take the\nancestor's, instead of ours.\n\nA conflict during the internal merge happens when there are\nmultiple common ancestors, and the development lines continuing\nfrom these ancestores may have disagreed on the result of this\nconflicting merge earlier.\n\n             o---A---C---o---Y\n            /     \\ /         \\\n          GP       .           M\n            \\     / \\         /\n             o---B---D---o---Z\n\nWhen we are trying to do the merge M between Y and Z, we find\ntwo closest common ancestors, A and B.  An internal merge to\nmerge them, using the grandparent ancestor, GP, is attempted,\nand we will use the result as the virtual common ancestor to\ncreate a merge between Y and Z.\n\nThe fact the internal merge between A and B conflicted means\nthat the development lines leading to Y and Z already saw that\nsame conflict before reaching Y and Z (in the above picture, at\nC and D, respectively), and they might have resolved it\ndifferently.\n\nWhen it is a text file, Fredrik's clever \"recursive merge\"\nleaves conflicted merge as the common ancestor, which is\ndifferent from either C or D's resolution (perhaps we can think\nof it being somewhere areound sign X in the above picture), and\nmerge between Y and Z using that as the common ancestor cancels\nthe conflict out if C and D resolved it the same way.\n\nHowever, when the file is a binary guck, the result of such a\nhalf-merge with conflict markers is simply a corrupt binary\ndata, and is unusable even for inspection.  That was the reason\nwhy I initially suggested to use 'ours'.  But that means, when\nwe are on the branch that leads to Y, we essentially favor C's\nresolution, defeating the \"accidental revert avoidance\" the\nrecursive merge strategy offers.  If instead we take the version\nfrom GP as the common ancestor, we have a usable binary guck as\nan ancestor as well, and also we are being neutral between the\nbranches, giving the end user a chance to make an unbiased\ndecision on his own.\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 4eb62cf..a6c08a1 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -659,6 +660,124 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n>  \tmm->size = size;\n>  }\n>  \n> +/* Low-level merge functions */\n> +typedef int (*ll_merge_fn)(mmfile_t *orig,\n> +\t\t\t   mmfile_t *src1, const char *name1,\n> +\t\t\t   mmfile_t *src2, const char *name2,\n> +\t\t\t   mmbuffer_t *result);\n> +\n> +static int ll_xdl_merge(mmfile_t *orig,\n> +\t\t\tmmfile_t *src1, const char *name1,\n> +\t\t\tmmfile_t *src2, const char *name2,\n> +\t\t\tmmbuffer_t *result)\n> +{\n> +\txpparam_t xpp;\n> +\tmemset(&xpp, 0, sizeof(xpp));\n> +\tmemset(&xpp, 0, sizeof(xpp));\n\nI caught this after sending this message, and already fixed it\nin the version I pushed out on 'pu'.\n"},{"id":"39656","messageId":"Pine.LNX.4.64.0704170815500.5473@woody.linux-foundation.org","threadId":"7711","inReplyTo":"11767973191314-git-send-email-junkio@cox.net","subject":"Re: [PATCH 3/4] Allow specifying specialized merge-backend per path.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T15:16:13Z","receivedAt":"2007-04-17T15:16:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Junio C Hamano wrote:\n>\n> This allows 'merge' attribute to control how the file-level\n> three-way merge is done per path.\n\nCool. Good job. The whole attribute thing seems to be really working out.\n\n\t\tLinus\n"}]}