{"thread":{"id":"7724","subject":"[PATCH 0/2] Custom low-level merge driver support.","startedAt":"2007-04-18T09:21:00Z","lastAt":"2007-04-19T08:08:28Z","messageCount":23,"participants":["Junio C Hamano","Johannes Sixt","Johannes Schindelin","Martin Waitz","Linus Torvalds","David Lang"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"39753","messageId":"11768880622402-git-send-email-junkio@cox.net","threadId":"7724","inReplyTo":null,"subject":"[PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T09:21:00Z","receivedAt":"2007-04-18T09:21:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This two-patch series concludes my initial attempt for\ngitattributes for now.\n\nIt hooks into the merge-recursive backend to allow custom low-level\n3-way merge drivers to be specified path-by-path basis.\n\nTo define a custom low-level merge driver, the usual\nconfiguration mechanism is used.\n\n\t[merge]\n\t\tdriver = ancient merge %A %O %B\n\nThe new configuration item, 'merge.driver', is multi-valued.\nEach of its values begin with the name of the low-level driver\n('ancient' in the above example), followed by its command line.\nThe command line can contain the following patterns to be\nprocessed with the usual interpolation mechanism:\n\n\t%O\tname of the temporary file that has the\n\t\tancestor's version.\n\n\t%A\tname of the temporary file that has the\n\t\tversion from the current branch.\n\n\t%B\tname of the temporary file that has the\n\t\tversion from the other branch.\n\nThe low-level driver is expected to update the file named with\n%A with the result of the merge, and exit with zero status upon\na clean merge. It can exit with non-zero status to signal that\nthe result still has conflicts.\n\nOnce you set up a low-level merge driver, you can specify that\ndriver to be used for specific paths, using the attributes\nmechanism, by using the driver's name as the value of 'merge'\nattribute. For example, taken together with the above 'ancient'\ndriver configuration and this line in your .git/info/attributes:\n\n\t*.txt\tmerge=ancient\n\nwill use the old \"RCS merge\" external program to merge a file\nwhose extension is \".txt\", just like we used to do before\nv1.5.0.\n"},{"id":"39754","messageId":"1176888062865-git-send-email-junkio@cox.net","threadId":"7724","inReplyTo":"11768880622402-git-send-email-junkio@cox.net","subject":"[PATCH 1/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T09:21:01Z","receivedAt":"2007-04-18T09:21:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This allows users to specify custom low-level merge driver per\npath, using the attributes mechanism. Just like you can specify\none of built-in \"text\", \"binary\", \"union\" low-level merge\ndrivers by saying:\n\n\t*\t\tmerge=text\n\t.gitignore\tmerge=union\n\t*.jpg\t\tmerge=binary\n\npick a name of your favorite merge driver, and assign it as the\nvalue of the 'merge' attribute.\n\nA custom low-level merge driver is defined via the config\nmechanism. This patch introduces 'merge.driver', a multi-valued\nconfiguration. Its value is the name (i.e. the one you use as\nthe value of 'merge' attribute) followed by a command line\nspecification. The command line can contain %O, %A, and %B to\nbe interpolated with the names of temporary files that hold the\ncommon ancestor version, the version from your branch, and the\nversion from the other branch, and the resulting command is\nspawned.\n\nThe low-level merge driver is expected to update the temporary\nfile for your branch (i.e. %A) with the result and exit with\nstatus 0 for a clean merge, and non-zero status for a conflicted\nmerge.\n\nA new test in t6026 demonstrates a sample usage.\n\nSigned-off-by: Junio C Hamano \n---\n merge-recursive.c | 177 +++++++++++++++++++++++++++++++++++++++++++++----\n t/t6026-merge-attr.sh | 71 +++++++++++++++++++-\n 2 files changed, 235 insertions(+), 13 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 3b34401..8ec18ad 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 \"interpolate.h\"\n #include \"attr.h\"\n \n static int subtree_merge;\n@@ -661,12 +662,14 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n }\n \n /* Low-level merge functions */\n-typedef int (*ll_merge_fn)(mmfile_t *orig,\n+typedef int (*ll_merge_fn)(const char *cmd,\n+\t\t\t 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+static int ll_xdl_merge(const char *cmd__unused,\n+\t\t\tmmfile_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@@ -681,7 +684,8 @@ static int ll_xdl_merge(mmfile_t *orig,\n \t\t\t result);\n }\n \n-static int ll_union_merge(mmfile_t *orig,\n+static int ll_union_merge(const char *cmd__unused,\n+\t\t\t 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@@ -690,7 +694,8 @@ static int ll_union_merge(mmfile_t *orig,\n \tlong size;\n \tconst int marker_size = 7;\n \n-\tint status = ll_xdl_merge(orig, src1, NULL, src2, NULL, result);\n+\tint status = ll_xdl_merge(cmd__unused, orig,\n+\t\t\t\t src1, NULL, src2, NULL, result);\n \tif (status <= 0)\n \t\treturn status;\n \tsize = result->size;\n@@ -721,7 +726,8 @@ static int ll_union_merge(mmfile_t *orig,\n \treturn 0;\n }\n \n-static int ll_binary_merge(mmfile_t *orig,\n+static int ll_binary_merge(const char *cmd__unused,\n+\t\t\t 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@@ -743,24 +749,169 @@ static struct {\n \tconst char *name;\n \tll_merge_fn fn;\n } ll_merge_fns[] = {\n-\t{ \"text\", ll_xdl_merge },\n \t{ \"binary\", ll_binary_merge },\n+\t{ \"text\", ll_xdl_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+static void create_temp(mmfile_t *src, char *path)\n {\n+\tint fd;\n+\n+\tstrcpy(path, \".merge_file_XXXXXX\");\n+\tfd = mkstemp(path);\n+\tif (fd < 0)\n+\t\tdie(\"unable to create temp-file\");\n+\tif (write_in_full(fd, src->ptr, src->size) != src->size)\n+\t\tdie(\"unable to write temp-file\");\n+\tclose(fd);\n+}\n+\n+static int ll_ext_merge(const char *cmd,\n+\t\t\tmmfile_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+\tchar temp[3][50];\n+\tchar cmdbuf[2048];\n+\tstruct interp table[] = {\n+\t\t{ \"%O\" },\n+\t\t{ \"%A\" },\n+\t\t{ \"%B\" },\n+\t};\n+\tstruct child_process child;\n+\tconst char *args[20];\n+\tint status, fd, i;\n+\tstruct stat st;\n+\n+\tresult->ptr = NULL;\n+\tresult->size = 0;\n+\tcreate_temp(orig, temp[0]);\n+\tcreate_temp(src1, temp[1]);\n+\tcreate_temp(src2, temp[2]);\n+\n+\tinterp_set_entry(table, 0, temp[0]);\n+\tinterp_set_entry(table, 1, temp[1]);\n+\tinterp_set_entry(table, 2, temp[2]);\n+\n+\tinterpolate(cmdbuf, sizeof(cmdbuf), cmd, table, 3);\n+\n+\tmemset(&child, 0, sizeof(child));\n+\tchild.argv = args;\n+\targs[0] = \"sh\";\n+\targs[1] = \"-c\";\n+\targs[2] = cmdbuf;\n+\targs[3] = NULL;\n+\n+\tstatus = run_command(&child);\n+\tif (status < -ERR_RUN_COMMAND_FORK)\n+\t\t; /* failure in run-command */\n+\telse\n+\t\tstatus = -status;\n+\tfd = open(temp[1], O_RDONLY);\n+\tif (fd < 0)\n+\t\tgoto bad;\n+\tif (fstat(fd, &st))\n+\t\tgoto close_bad;\n+\tresult->size = st.st_size;\n+\tresult->ptr = xmalloc(result->size + 1);\n+\tif (read_in_full(fd, result->ptr, result->size) != result->size) {\n+\t\tfree(result->ptr);\n+\t\tresult->ptr = NULL;\n+\t\tresult->size = 0;\n+\t}\n+ close_bad:\n+\tclose(fd);\n+ bad:\n+\tfor (i = 0; i < 3; i++)\n+\t\tunlink(temp[i]);\n+\treturn status;\n+}\n+\n+/*\n+ * merge.default and merge.driver configuration items\n+ */\n+static struct user_merge_fn {\n+\tstruct user_merge_fn *next;\n+\tconst char *name;\n+\tchar *cmdline;\n+\tchar b_[1];\n+} *ll_user_merge_fns, **ll_user_merge_fns_tail;\n+\n+static int read_merge_config(const char *var, const char *value)\n+{\n+\tstruct user_merge_fn *fn;\n+\tint blen, nlen;\n+\n+\tif (strcmp(var, \"merge.driver\"))\n+\t\treturn 0;\n+\tif (!value)\n+\t\treturn error(\"%s: lacks value\", var);\n+\t/*\n+\t * merge.driver is a multi-valued configuration, whose value is\n+\t * of form:\n+\t *\n+\t *\tname command-line\n+\t *\n+\t * The command-line will be interpolated with the following\n+\t * tokens and is given to the shell:\n+\t *\n+\t * %O - temporary file name for the merge base.\n+\t * %A - temporary file name for our version.\n+\t * %B - temporary file name for the other branches' version.\n+\t *\n+\t * The external merge driver should write the results in the file\n+\t * named by %A, and signal that it has done with exit status 0.\n+\t */\n+\tfor (nlen = -1, blen = 0; value[blen]; blen++)\n+\t\tif (nlen < 0 && isspace(value[blen]))\n+\t\t\tnlen = blen;\n+\tif (nlen < 0)\n+\t\treturn error(\"%s '%s': lacks command line\", var, value);\n+\tfn = xcalloc(1, sizeof(struct user_merge_fn) + blen + 1);\n+\tmemcpy(fn->b_, value, blen + 1);\n+\tfn->name = fn->b_;\n+\tfn->b_[nlen] = 0;\n+\tfn->cmdline = fn->b_ + nlen + 1;\n+\tfn->next = *ll_user_merge_fns_tail;\n+\t*ll_user_merge_fns_tail = fn;\n+\treturn 0;\n+}\n+\n+static void initialize_ll_merge(void)\n+{\n+\tif (ll_user_merge_fns_tail)\n+\t\treturn;\n+\tll_user_merge_fns_tail = &ll_user_merge_fns;\n+\tgit_config(read_merge_config);\n+}\n+\n+static ll_merge_fn find_ll_merge_fn(void *merge_attr, const char **cmdline)\n+{\n+\tstruct user_merge_fn *fn;\n \tconst char *name;\n \tint i;\n \n-\tif (ATTR_TRUE(merge_attr) || ATTR_UNSET(merge_attr))\n+\tinitialize_ll_merge();\n+\n+\tif (ATTR_TRUE(merge_attr))\n \t\treturn ll_xdl_merge;\n \telse if (ATTR_FALSE(merge_attr))\n \t\treturn ll_binary_merge;\n+\telse if (ATTR_UNSET(merge_attr))\n+\t\treturn ll_xdl_merge;\n+\telse\n+\t\tname = merge_attr;\n+\n+\tfor (fn = ll_user_merge_fns; fn; fn = fn->next) {\n+\t\tif (!strcmp(fn->name, name)) {\n+\t\t\t*cmdline = fn->cmdline;\n+\t\t\treturn ll_ext_merge;\n+\t\t}\n+\t}\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@@ -793,6 +944,7 @@ static int ll_merge(mmbuffer_t *result_buf,\n \tint merge_status;\n \tvoid *merge_attr;\n \tll_merge_fn fn;\n+\tconst char *driver = NULL;\n \n \tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n \tname2 = xstrdup(mkpath(\"%s:%s\", branch2, b->path));\n@@ -802,9 +954,10 @@ static int ll_merge(mmbuffer_t *result_buf,\n \tfill_mm(b->sha1, &src2);\n \n \tmerge_attr = git_path_check_merge(a->path);\n-\tfn = find_ll_merge_fn(merge_attr);\n+\tfn = find_ll_merge_fn(merge_attr, &driver);\n \n-\tmerge_status = fn(&orig, &src1, name1, &src2, name2, result_buf);\n+\tmerge_status = fn(driver, &orig,\n+\t\t\t &src1, name1, &src2, name2, result_buf);\n \n \tfree(name1);\n \tfree(name2);\ndiff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh\nindex 5daa223..1732b60 100755\n--- a/t/t6026-merge-attr.sh\n+++ b/t/t6026-merge-attr.sh\n@@ -30,8 +30,9 @@ test_expect_success setup '\n \t\techo Side >>$f && git add $f || break\n \tdone &&\n \ttest_tick &&\n-\tgit commit -m Side\n+\tgit commit -m Side &&\n \n+\tgit tag anchor\n '\n \n test_expect_success merge '\n@@ -69,4 +70,72 @@ test_expect_success 'check merge result in working tree' '\n \n '\n \n+cat >./custom-merge <<\\EOF\n+#!/bin/sh\n+\n+orig=\"$1\" ours=\"$2\" theirs=\"$3\" exit=\"$4\"\n+(\n+\techo \"orig is $orig\"\n+\techo \"ours is $ours\"\n+\techo \"theirs is $theirs\"\n+\techo \"=== orig ===\"\n+\tcat \"$orig\"\n+\techo \"=== ours ===\"\n+\tcat \"$ours\"\n+\techo \"=== theirs ===\"\n+\tcat \"$theirs\"\n+) >\"$ours+\"\n+cat \"$ours+\" >\"$ours\"\n+rm -f \"$ours+\"\n+exit \"$exit\"\n+EOF\n+chmod +x ./custom-merge\n+\n+test_expect_success 'custom merge backend' '\n+\n+\techo \"* merge=union\" >.gitattributes &&\n+\techo \"text merge=custom\" >>.gitattributes &&\n+\n+\tgit reset --hard anchor &&\n+\tgit config --replace-all \\\n+\tmerge.driver \"custom ./custom-merge %O %A %B 0\" &&\n+\n+\tgit merge master &&\n+\n+\tcmp binary union &&\n+\tsed -e 1,3d text >check-1 &&\n+\to=$(git-unpack-file master^:text) &&\n+\ta=$(git-unpack-file side^:text) &&\n+\tb=$(git-unpack-file master:text) &&\n+\tsh -c \"./custom-merge $o $a $b 0\" &&\n+\tsed -e 1,3d $a >check-2 &&\n+\tcmp check-1 check-2 &&\n+\trm -f $o $a $b\n+'\n+\n+test_expect_success 'custom merge backend' '\n+\n+\tgit reset --hard anchor &&\n+\tgit config --replace-all \\\n+\tmerge.driver \"custom ./custom-merge %O %A %B 1\" &&\n+\n+\tif git merge master\n+\tthen\n+\t\techo \"Eh? should have conflicted\"\n+\t\tfalse\n+\telse\n+\t\techo \"Ok, conflicted\"\n+\tfi &&\n+\n+\tcmp binary union &&\n+\tsed -e 1,3d text >check-1 &&\n+\to=$(git-unpack-file master^:text) &&\n+\ta=$(git-unpack-file anchor:text) &&\n+\tb=$(git-unpack-file master:text) &&\n+\tsh -c \"./custom-merge $o $a $b 0\" &&\n+\tsed -e 1,3d $a >check-2 &&\n+\tcmp check-1 check-2 &&\n+\trm -f $o $a $b\n+'\n+\n test_done\n-- \n1.5.1.1.901.g25ba\n"},{"id":"39755","messageId":"1176888062910-git-send-email-junkio@cox.net","threadId":"7724","inReplyTo":"11768880622402-git-send-email-junkio@cox.net","subject":"[PATCH 2/2] Allow the default low-level merge driver to be configured.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T09:21:02Z","receivedAt":"2007-04-18T09:21:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When no 'merge' attribute is given to a path, merge-recursive\nuses the built-in xdl-merge as the low-level merge driver.\n\nA new configuration item 'merge.default' can name a low-level\nmerge driver of user's choice to be used instead.\n\nSigned-off-by: Junio C Hamano \n---\n merge-recursive.c | 14 ++++++++++++--\n 1 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 8ec18ad..5983000 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -839,12 +839,18 @@ static struct user_merge_fn {\n \tchar *cmdline;\n \tchar b_[1];\n } *ll_user_merge_fns, **ll_user_merge_fns_tail;\n+static const char *default_ll_merge;\n \n static int read_merge_config(const char *var, const char *value)\n {\n \tstruct user_merge_fn *fn;\n \tint blen, nlen;\n \n+\tif (!strcmp(var, \"merge.default\")) {\n+\t\tdefault_ll_merge = strdup(value);\n+\t\treturn 0;\n+\t}\n+\n \tif (strcmp(var, \"merge.driver\"))\n \t\treturn 0;\n \tif (!value)\n@@ -900,8 +906,12 @@ static ll_merge_fn find_ll_merge_fn(void *merge_attr, const char **cmdline)\n \t\treturn ll_xdl_merge;\n \telse if (ATTR_FALSE(merge_attr))\n \t\treturn ll_binary_merge;\n-\telse if (ATTR_UNSET(merge_attr))\n-\t\treturn ll_xdl_merge;\n+\telse if (ATTR_UNSET(merge_attr)) {\n+\t\tif (!default_ll_merge)\n+\t\t\treturn ll_xdl_merge;\n+\t\telse\n+\t\t\tname = default_ll_merge;\n+\t}\n \telse\n \t\tname = merge_attr;\n \n-- \n1.5.1.1.901.g25ba\n"},{"id":"39761","messageId":"4625F4AD.5CDDC502@eudaptics.com","threadId":"7724","inReplyTo":"1176888062865-git-send-email-junkio@cox.net","subject":"Re: [PATCH 1/2] Custom low-level merge driver support.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-04-18T10:36:29Z","receivedAt":"2007-04-18T10:36:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano wrote:\n> + interpolate(cmdbuf, sizeof(cmdbuf), cmd, table, 3);\n> +\n> + memset(&child, 0, sizeof(child));\n> + child.argv = args;\n> + args[0] = \"sh\";\n> + args[1] = \"-c\";\n> + args[2] = cmdbuf;\n> + args[3] = NULL;\n\nIf I read the code correctly, there does not happen any shell quoting\nanywhere; hence, this shell invocation is dangerous.\n\n-- Hannes\n"},{"id":"39762","messageId":"Pine.LNX.4.64.0704181247410.12094@racer.site","threadId":"7724","inReplyTo":"11768880622402-git-send-email-junkio@cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-18T10:48:48Z","receivedAt":"2007-04-18T10:48:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Apr 2007, Junio C Hamano wrote:\n\n> \t[merge]\n> \t\tdriver = ancient merge %A %O %B\n\nWhy not do something like\n\n\t[merge.driver]\n\t\tancient = merge %A %O %B\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"39763","messageId":"Pine.LNX.4.64.0704181253350.12094@racer.site","threadId":"7724","inReplyTo":"4625F4AD.5CDDC502@eudaptics.com","subject":"Re: [PATCH 1/2] Custom low-level merge driver support.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-18T10:55:12Z","receivedAt":"2007-04-18T10:55:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Apr 2007, Johannes Sixt wrote:\n\n> Junio C Hamano wrote:\n> > + interpolate(cmdbuf, sizeof(cmdbuf), cmd, table, 3);\n> > +\n> > + memset(&child, 0, sizeof(child));\n> > + child.argv = args;\n> > + args[0] = \"sh\";\n> > + args[1] = \"-c\";\n> > + args[2] = cmdbuf;\n> > + args[3] = NULL;\n> \n> If I read the code correctly, there does not happen any shell quoting\n> anywhere; hence, this shell invocation is dangerous.\n\nAFAICT the files used are all temporary files named \".merge_file_xxxx\" in \nthe current directory, so there should not be a chance to have spaces or \nother weird characters in the files.\n\nCiao,\nDscho\n"},{"id":"39777","messageId":"20070418153445.GC12888@admingilde.org","threadId":"7724","inReplyTo":"Pine.LNX.4.64.0704181247410.12094@racer.site","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-04-18T15:34:45Z","receivedAt":"2007-04-18T15:34:45Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Wed, Apr 18, 2007 at 12:48:48PM +0200, Johannes Schindelin wrote:\n> On Wed, 18 Apr 2007, Junio C Hamano wrote:\n> \n> > \t[merge]\n> > \t\tdriver = ancient merge %A %O %B\n> \n> Why not do something like\n> \n> \t[merge.driver]\n> \t\tancient = merge %A %O %B\n\nor\n\n[merge \"ancient\"]\n\tdriver = merge %A %O %B\n\n-- \nMartin Waitz\n"},{"id":"39780","messageId":"7vps61isly.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"20070418153445.GC12888@admingilde.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T15:56:25Z","receivedAt":"2007-04-18T15:56:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz writes:\n\n> On Wed, Apr 18, 2007 at 12:48:48PM +0200, Johannes Schindelin wrote:\n>> On Wed, 18 Apr 2007, Junio C Hamano wrote:\n>> \n>> > \t[merge]\n>> > \t\tdriver = ancient merge %A %O %B\n>> \n>> Why not do something like\n>> \n>> \t[merge.driver]\n>> \t\tancient = merge %A %O %B\n>\n> or\n>\n> [merge \"ancient\"]\n> \tdriver = merge %A %O %B\n\nActually, I've considered the latter.\n\nJohannes's goes against the usual three-level configuration\nvariable naming rules. If you have 'user definable' part in the\nname, that should be at the second level.\n\nWhich yours does.\n\nThe only reason I did not do it your way was because we would\nneed to have three lines per driver (one [merge \"foo\"] section\nper driver, a \"driver\" line, and a blank line for readability\nafter that), which at the time I wrote it felt a bit wasteful,\nand it was late. But I think I like yours much better.\n\nIt probably is trivial to change the rule before it hits 'next'.\n"},{"id":"39782","messageId":"20070418160811.GD12888@admingilde.org","threadId":"7724","inReplyTo":"7vps61isly.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-04-18T16:08:11Z","receivedAt":"2007-04-18T16:08:11Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Wed, Apr 18, 2007 at 08:56:25AM -0700, Junio C Hamano wrote:\n> The only reason I did not do it your way was because we would\n> need to have three lines per driver (one [merge \"foo\"] section\n> per driver, a \"driver\" line, and a blank line for readability\n> after that), which at the time I wrote it felt a bit wasteful,\n> and it was late.\n\nperhaps we can extend the config syntax to also support an entry\non the same line as the category:\n\n\t[merge \"ancient\"] driver=merge...\n\nThis would allow an one-line merge driver definition and would play\nnice with your [attr] macro definition.\n\nPerhaps we should also add the = to macro definitions?\n\n\t[attr] binary=nocrlf nodiff merge=binary\n\nHmm, but then we would get multiple meanings for \"=\" on the same line,\nso nevermind.\n\nOr use some whitespace:\n\n\t[attr] binary = nocrlf nodiff merge=binary\n\nDoes this look better?\n\n-- \nMartin Waitz\n"},{"id":"39787","messageId":"alpine.LFD.0.98.0704180910120.2828@woody.linux-foundation.org","threadId":"7724","inReplyTo":"20070418153445.GC12888@admingilde.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T16:16:31Z","receivedAt":"2007-04-18T16:16:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Martin Waitz wrote:\n>\n> > \t[merge.driver]\n> > \t\tancient = merge %A %O %B\n> \n> or\n> \n> [merge \"ancient\"]\n> \tdriver = merge %A %O %B\n\nMuch better. That format also allows you to add extra flags if you want. \n\nFor example, it might be useful to add something like\n\n\t[merge \"ancient\"]\n\t\tname = external three-way merge\n\t\tdriver = merge %A %O %B\n\nso that you could make the merge process actually say what it is doing \nwhen it's merging things. Wouldn't it be nice to see\n\n\tmerging file xyzzy using external three-way merge\n\tmerging file fax.doc using word file merge\n\t..\n\nwhen these things trigger?\n\nSo even if we do *not* care about the name, this kind of config structure \nis just more flexible. \n\n\t\tLinus\n"},{"id":"39794","messageId":"7vslaxhamd.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"alpine.LFD.0.98.0704180910120.2828@woody.linux-foundation.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T17:10:18Z","receivedAt":"2007-04-18T17:10:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Wed, 18 Apr 2007, Martin Waitz wrote:\n>>\n>> > \t[merge.driver]\n>> > \t\tancient = merge %A %O %B\n>> \n>> or\n>> \n>> [merge \"ancient\"]\n>> \tdriver = merge %A %O %B\n>\n> Much better. That format also allows you to add extra flags if you want. \n>\n> For example, it might be useful to add something like\n>\n> \t[merge \"ancient\"]\n> \t\tname = external three-way merge\n> \t\tdriver = merge %A %O %B\n>\n> so that you could make the merge process actually say what it is doing \n> when it's merging things. Wouldn't it be nice to see\n>\n> \tmerging file xyzzy using external three-way merge\n> \tmerging file fax.doc using word file merge\n> \t..\n>\n> when these things trigger?\n>\n> So even if we do *not* care about the name, this kind of config structure \n> is just more flexible. \n\nYes.\n\nWhile I agree with all of the above, I am currently fighting\nwith a back-to-drawing-board design problem.\n\nThe series made the low-level 3-way merge machinery\ncustomizable, which is a progress, but I think merge-recursive\nneeds a hook to affect middle-level merge decision, similar to\nwhat git-merge-one-file does.\n\nI wanted to use 'ours' merge for RelNotes, which typically\npoints at the release notes being prepared for the next release\nfrom the branch (Documentation/RelNotes-1.5.1.1.txt for 'maint',\nand Documentation/RelNotes-1.5.2.txt for 'master'). As I never\nmerge 'master' into 'maint', using 'ours' merge for that path is\nthe right thing to do in this case.\n\nBut merge-recursive has a built-in middle-level decision that\nmakes a conflicting symlink modification never go through the\nlow-level 3-way merge codepath.\n\nThis was originally not a problem, as the low-level 3-way merge\nused to be always \"textual merge\" that would never make sense\nfor symbolic links, but once we allow low-level merge driver\nthat could be \"pick one of branches\", it becomes needless\nconstraints.\n\nFor now, I'd stop at pointing this issue in this message, and\nfinish up the configurability of low-level merge driver first\nand merge that to 'next'.\n\nBut I *think* we would end up revising the driver definition to\ntake more than %A %O %B and allow it to do more than the\nlow-level decision.\n\nPerhaps...\n\n\t(in .gitattributes)\n\n\tRelNotes\tmerge=ours\n\n\t(in .git/config)\n [merge \"ours\"]\n \tname = middle level 'ours' merge driver\n driver = exit 0\n middlelevel\n\nwhen \"merge.$name.middlelevel\" boolean is set, it would forbid\nprocess_entry() to make the 'git-merge-one-file' policy decision\nand always call the driver with these in addition to the %O %A\n%B:\n\n\t%OSHA1\tobject name for ancestor version (or 0{40} for missing)\n %OMODE\tmode bits for ancestor version (or 0{6} for missing)\n\nThe driver should be able to express the merge cleanliness via\nexit status, and the resulting (potentially partial) merge\nresult blob via %A as the low-level driver, but in addition to\nthem it needs to be able to say \"the merge result is to remove\nthat path\". I haven't figured out what that interface should\nbe; we could designate one special exit code to signal that,\nperhaps \"exit 42\", but that feels hacky.\n\nIn any case, making [merge \"driver-name\"] section with 'driver'\nvariable leaves the door open for such extension in a backward\ncompatible way by leaving room to add 'middlelevel' boolean like\nthe above outline, so I'd stop worrying about this for now,\nwithout doing anything further.\n"},{"id":"39803","messageId":"7vbqhlh6zv.fsf_-_@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"alpine.LFD.0.98.0704180910120.2828@woody.linux-foundation.org","subject":"[PATCH] Custom low-level merge driver: change the configuration scheme.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T18:28:36Z","receivedAt":"2007-04-18T18:28:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This changes the configuration syntax for defining a low-level\nmerge driver to be:\n\n\t[merge \"<>\"]\n\t\tdriver = \"<>\"\n\t\tname = \"<>\"\n\nwhich is much nicer to read and is extensible. Credit goes to\nMartin Waitz and Linus.\n\nIn addition, when we use an external low-level merge driver, it\nis reported as an extra output from merge-recursive, using the\nvalue of merge.<.name variable.\n\nThe demonstration in t6026 has also been updated.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is on top of what I pushed out on 'pu' last night. This\n being my git day, I am hoping I can push this out in 'next'\n today.\n\n merge-recursive.c | 202 ++++++++++++++++++++++++++++++-------------------\n t/t6026-merge-attr.sh | 8 ++-\n 2 files changed, 131 insertions(+), 79 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 5983000..4af69d7 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -661,14 +661,31 @@ 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)(const char *cmd,\n+/*\n+ * Customizable low-level merge drivers support.\n+ */\n+\n+struct ll_merge_driver;\n+typedef int (*ll_merge_fn)(const struct ll_merge_driver *,\n+\t\t\t const char *path,\n \t\t\t 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(const char *cmd__unused,\n+struct ll_merge_driver {\n+\tconst char *name;\n+\tconst char *description;\n+\tll_merge_fn fn;\n+\tstruct ll_merge_driver *next;\n+\tchar *cmdline;\n+};\n+\n+/*\n+ * Built-in low-levels\n+ */\n+static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n+\t\t\tconst char *path_unused,\n \t\t\tmmfile_t *orig,\n \t\t\tmmfile_t *src1, const char *name1,\n \t\t\tmmfile_t *src2, const char *name2,\n@@ -684,7 +701,8 @@ static int ll_xdl_merge(const char *cmd__unused,\n \t\t\t result);\n }\n \n-static int ll_union_merge(const char *cmd__unused,\n+static int ll_union_merge(const struct ll_merge_driver *drv_unused,\n+\t\t\t const char *path_unused,\n \t\t\t mmfile_t *orig,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n@@ -694,8 +712,8 @@ static int ll_union_merge(const char *cmd__unused,\n \tlong size;\n \tconst int marker_size = 7;\n \n-\tint status = ll_xdl_merge(cmd__unused, orig,\n-\t\t\t\t src1, NULL, src2, NULL, result);\n+\tint status = ll_xdl_merge(drv_unused, path_unused,\n+\t\t\t\t orig, src1, NULL, src2, NULL, result);\n \tif (status <= 0)\n \t\treturn status;\n \tsize = result->size;\n@@ -726,7 +744,8 @@ static int ll_union_merge(const char *cmd__unused,\n \treturn 0;\n }\n \n-static int ll_binary_merge(const char *cmd__unused,\n+static int ll_binary_merge(const struct ll_merge_driver *drv_unused,\n+\t\t\t const char *path_unused,\n \t\t\t mmfile_t *orig,\n \t\t\t mmfile_t *src1, const char *name1,\n \t\t\t mmfile_t *src2, const char *name2,\n@@ -745,14 +764,13 @@ static int ll_binary_merge(const char *cmd__unused,\n \treturn 1;\n }\n \n-static struct {\n-\tconst char *name;\n-\tll_merge_fn fn;\n-} ll_merge_fns[] = {\n-\t{ \"binary\", ll_binary_merge },\n-\t{ \"text\", ll_xdl_merge },\n-\t{ \"union\", ll_union_merge },\n-\t{ NULL, NULL },\n+#define LL_BINARY_MERGE 0\n+#define LL_TEXT_MERGE 1\n+#define LL_UNION_MERGE 2\n+static struct ll_merge_driver ll_merge_drv[] = {\n+\t{ \"binary\", \"built-in binary merge\", ll_binary_merge },\n+\t{ \"text\", \"built-in 3-way text merge\", ll_xdl_merge },\n+\t{ \"union\", \"built-in union merge\", ll_union_merge },\n };\n \n static void create_temp(mmfile_t *src, char *path)\n@@ -768,7 +786,11 @@ static void create_temp(mmfile_t *src, char *path)\n \tclose(fd);\n }\n \n-static int ll_ext_merge(const char *cmd,\n+/*\n+ * User defined low-level merge driver support.\n+ */\n+static int ll_ext_merge(const struct ll_merge_driver *fn,\n+\t\t\tconst char *path,\n \t\t\tmmfile_t *orig,\n \t\t\tmmfile_t *src1, const char *name1,\n \t\t\tmmfile_t *src2, const char *name2,\n@@ -796,7 +818,10 @@ static int ll_ext_merge(const char *cmd,\n \tinterp_set_entry(table, 1, temp[1]);\n \tinterp_set_entry(table, 2, temp[2]);\n \n-\tinterpolate(cmdbuf, sizeof(cmdbuf), cmd, table, 3);\n+\toutput(1, \"merging %s using %s\", path,\n+\t fn->description ? fn->description : fn->name);\n+\n+\tinterpolate(cmdbuf, sizeof(cmdbuf), fn->cmdline, table, 3);\n \n \tmemset(&child, 0, sizeof(child));\n \tchild.argv = args;\n@@ -833,101 +858,124 @@ static int ll_ext_merge(const char *cmd,\n /*\n * merge.default and merge.driver configuration items\n */\n-static struct user_merge_fn {\n-\tstruct user_merge_fn *next;\n-\tconst char *name;\n-\tchar *cmdline;\n-\tchar b_[1];\n-} *ll_user_merge_fns, **ll_user_merge_fns_tail;\n+static struct ll_merge_driver *ll_user_merge, **ll_user_merge_tail;\n static const char *default_ll_merge;\n \n static int read_merge_config(const char *var, const char *value)\n {\n-\tstruct user_merge_fn *fn;\n-\tint blen, nlen;\n+\tstruct ll_merge_driver *fn;\n+\tconst char *ep, *name;\n+\tint namelen;\n \n \tif (!strcmp(var, \"merge.default\")) {\n-\t\tdefault_ll_merge = strdup(value);\n+\t\tif (value)\n+\t\t\tdefault_ll_merge = strdup(value);\n \t\treturn 0;\n \t}\n \n-\tif (strcmp(var, \"merge.driver\"))\n+\t/*\n+\t * We are not interested in anything but \"merge..variable\";\n+\t * especially, we do not want to look at variables such as\n+\t * \"merge.summary\", \"merge.tool\", and \"merge.verbosity\".\n+\t */\n+\tif (prefixcmp(var, \"merge.\") || (ep = strchr(var + 6, '.')) == NULL)\n \t\treturn 0;\n-\tif (!value)\n-\t\treturn error(\"%s: lacks value\", var);\n+\n \t/*\n-\t * merge.driver is a multi-valued configuration, whose value is\n-\t * of form:\n-\t *\n-\t *\tname command-line\n-\t *\n-\t * The command-line will be interpolated with the following\n-\t * tokens and is given to the shell:\n-\t *\n-\t * %O - temporary file name for the merge base.\n-\t * %A - temporary file name for our version.\n-\t * %B - temporary file name for the other branches' version.\n-\t *\n-\t * The external merge driver should write the results in the file\n-\t * named by %A, and signal that it has done with exit status 0.\n+\t * Find existing one as we might be processing merge..var2\n+\t * after seeing merge..var1.\n \t */\n-\tfor (nlen = -1, blen = 0; value[blen]; blen++)\n-\t\tif (nlen < 0 && isspace(value[blen]))\n-\t\t\tnlen = blen;\n-\tif (nlen < 0)\n-\t\treturn error(\"%s '%s': lacks command line\", var, value);\n-\tfn = xcalloc(1, sizeof(struct user_merge_fn) + blen + 1);\n-\tmemcpy(fn->b_, value, blen + 1);\n-\tfn->name = fn->b_;\n-\tfn->b_[nlen] = 0;\n-\tfn->cmdline = fn->b_ + nlen + 1;\n-\tfn->next = *ll_user_merge_fns_tail;\n-\t*ll_user_merge_fns_tail = fn;\n+\tname = var + 6;\n+\tnamelen = ep - name;\n+\tfor (fn = ll_user_merge; fn; fn = fn->next)\n+\t\tif (!strncmp(fn->name, name, namelen) && !fn->name[namelen])\n+\t\t\tbreak;\n+\tif (!fn) {\n+\t\tchar *namebuf;\n+\t\tfn = xcalloc(1, sizeof(struct ll_merge_driver));\n+\t\tnamebuf = xmalloc(namelen + 1);\n+\t\tmemcpy(namebuf, name, namelen);\n+\t\tnamebuf[namelen] = 0;\n+\t\tfn->name = namebuf;\n+\t\tfn->fn = ll_ext_merge;\n+\t\tfn->next = *ll_user_merge_tail;\n+\t\t*ll_user_merge_tail = fn;\n+\t}\n+\n+\tep++;\n+\n+\tif (!strcmp(\"name\", ep)) {\n+\t\tif (!value)\n+\t\t\treturn error(\"%s: lacks value\", var);\n+\t\tfn->description = strdup(value);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(\"driver\", ep)) {\n+\t\tif (!value)\n+\t\t\treturn error(\"%s: lacks value\", var);\n+\t\t/*\n+\t\t * merge..driver specifies the command line:\n+\t\t *\n+\t\t *\tcommand-line\n+\t\t *\n+\t\t * The command-line will be interpolated with the following\n+\t\t * tokens and is given to the shell:\n+\t\t *\n+\t\t * %O - temporary file name for the merge base.\n+\t\t * %A - temporary file name for our version.\n+\t\t * %B - temporary file name for the other branches' version.\n+\t\t *\n+\t\t * The external merge driver should write the results in the\n+\t\t * file named by %A, and signal that it has done with zero exit\n+\t\t * status.\n+\t\t */\n+\t\tfn->cmdline = strdup(value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \n static void initialize_ll_merge(void)\n {\n-\tif (ll_user_merge_fns_tail)\n+\tif (ll_user_merge_tail)\n \t\treturn;\n-\tll_user_merge_fns_tail = &ll_user_merge_fns;\n+\tll_user_merge_tail = &ll_user_merge;\n \tgit_config(read_merge_config);\n }\n \n-static ll_merge_fn find_ll_merge_fn(void *merge_attr, const char **cmdline)\n+static const struct ll_merge_driver *find_ll_merge_driver(void *merge_attr)\n {\n-\tstruct user_merge_fn *fn;\n+\tstruct ll_merge_driver *fn;\n \tconst char *name;\n \tint i;\n \n \tinitialize_ll_merge();\n \n \tif (ATTR_TRUE(merge_attr))\n-\t\treturn ll_xdl_merge;\n+\t\treturn &ll_merge_drv[LL_TEXT_MERGE];\n \telse if (ATTR_FALSE(merge_attr))\n-\t\treturn ll_binary_merge;\n+\t\treturn &ll_merge_drv[LL_BINARY_MERGE];\n \telse if (ATTR_UNSET(merge_attr)) {\n \t\tif (!default_ll_merge)\n-\t\t\treturn ll_xdl_merge;\n+\t\t\treturn &ll_merge_drv[LL_TEXT_MERGE];\n \t\telse\n \t\t\tname = default_ll_merge;\n \t}\n \telse\n \t\tname = merge_attr;\n \n-\tfor (fn = ll_user_merge_fns; fn; fn = fn->next) {\n-\t\tif (!strcmp(fn->name, name)) {\n-\t\t\t*cmdline = fn->cmdline;\n-\t\t\treturn ll_ext_merge;\n-\t\t}\n-\t}\n+\tfor (fn = ll_user_merge; fn; fn = fn->next)\n+\t\tif (!strcmp(fn->name, name))\n+\t\t\treturn fn;\n \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+\tfor (i = 0; i < ARRAY_SIZE(ll_merge_drv); i++)\n+\t\tif (!strcmp(ll_merge_drv[i].name, name))\n+\t\t\treturn &ll_merge_drv[i];\n \n \t/* default to the 3-way */\n-\treturn ll_xdl_merge;\n+\treturn &ll_merge_drv[LL_TEXT_MERGE];\n }\n \n static void *git_path_check_merge(const char *path)\n@@ -953,8 +1001,7 @@ static int ll_merge(mmbuffer_t *result_buf,\n \tchar *name1, *name2;\n \tint merge_status;\n \tvoid *merge_attr;\n-\tll_merge_fn fn;\n-\tconst char *driver = NULL;\n+\tconst struct ll_merge_driver *driver;\n \n \tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n \tname2 = xstrdup(mkpath(\"%s:%s\", branch2, b->path));\n@@ -964,10 +1011,11 @@ static int ll_merge(mmbuffer_t *result_buf,\n \tfill_mm(b->sha1, &src2);\n \n \tmerge_attr = git_path_check_merge(a->path);\n-\tfn = find_ll_merge_fn(merge_attr, &driver);\n+\tdriver = find_ll_merge_driver(merge_attr);\n \n-\tmerge_status = fn(driver, &orig,\n-\t\t\t &src1, name1, &src2, name2, result_buf);\n+\tmerge_status = driver->fn(driver, a->path,\n+\t\t\t\t &orig, &src1, name1, &src2, name2,\n+\t\t\t\t result_buf);\n \n \tfree(name1);\n \tfree(name2);\ndiff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh\nindex 1732b60..56fc341 100755\n--- a/t/t6026-merge-attr.sh\n+++ b/t/t6026-merge-attr.sh\n@@ -98,7 +98,9 @@ test_expect_success 'custom merge backend' '\n \n \tgit reset --hard anchor &&\n \tgit config --replace-all \\\n-\tmerge.driver \"custom ./custom-merge %O %A %B 0\" &&\n+\tmerge.custom.driver \"./custom-merge %O %A %B 0\" &&\n+\tgit config --replace-all \\\n+\tmerge.custom.name \"custom merge driver for testing\" &&\n \n \tgit merge master &&\n \n@@ -117,7 +119,9 @@ test_expect_success 'custom merge backend' '\n \n \tgit reset --hard anchor &&\n \tgit config --replace-all \\\n-\tmerge.driver \"custom ./custom-merge %O %A %B 1\" &&\n+\tmerge.custom.driver \"./custom-merge %O %A %B 1\" &&\n+\tgit config --replace-all \\\n+\tmerge.custom.name \"custom merge driver for testing\" &&\n \n \tif git merge master\n \tthen\n-- \n1.5.1.1.901.g25ba\n"},{"id":"39807","messageId":"alpine.LFD.0.98.0704181136360.2828@woody.linux-foundation.org","threadId":"7724","inReplyTo":"7vslaxhamd.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T18:45:56Z","receivedAt":"2007-04-18T18:45:56Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Junio C Hamano wrote:\n> \n> While I agree with all of the above, I am currently fighting\n> with a back-to-drawing-board design problem.\n> \n> The series made the low-level 3-way merge machinery\n> customizable, which is a progress, but I think merge-recursive\n> needs a hook to affect middle-level merge decision, similar to\n> what git-merge-one-file does.\n\nHmm. I think that it's not so much a \"hook\", as a \"recursive merge is a \nseparate merge policy altogether, that just *defaults* to the same final \nmerge policy\".\n\nFor example, for a three-way merge, it makes sense to do the three-way \nmerge for the \"internal\" merge too. But it might be that that isn't true \nfor all strategies. For a \"blend\" merge, blending at the middle level \nmight be the wrong thing to do because it might duplicate the messages, \nfor example.\n\nSo maybe the rule could be:\n\n - for a file with merge strategy \"xyzzy\", a recursive merge will first \n look up the merge machinery for \"xyzzy-recursive\" and if no such merge \n strategy exists, it will look up merge machinery for \"xyzzy\".\n - the final merge will always use merge strategy \"xyzzy\".\n\nSo then, assuming I'm right that the \"blend\" strategy needs soemthing else \nin the middle (and I'm not sure I'm right at all, this is purely \nhypothetical ;), you could have something like\n\n\t# a recursive merge will always just pick the ORIGINAL\n\t# version of a file when blending - the blending will\n\t# be done only on the final merge\n\t[merge \"blend-recursive\"]\n\t\tname = recursive blend strategy\n\t\tdriver = cp %O %A\n\n\t[merge \"blend\"]\n\t\tname = blend (changelog) strategy\n\t\tdriver = blend %A %O %B\n\nor something like that?\n\nI _suspect_ that most merge strategies want to use the same strategy for \nthe recursive merge as for the final one, but I haven't really thought it \nthrough.\n\n\t\tLinus\n"},{"id":"39809","messageId":"7vzm55fqyv.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"alpine.LFD.0.98.0704181136360.2828@woody.linux-foundation.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T19:00:08Z","receivedAt":"2007-04-18T19:00:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Wed, 18 Apr 2007, Junio C Hamano wrote:\n>> \n>> While I agree with all of the above, I am currently fighting\n>> with a back-to-drawing-board design problem.\n>> \n>> The series made the low-level 3-way merge machinery\n>> customizable, which is a progress, but I think merge-recursive\n>> needs a hook to affect middle-level merge decision, similar to\n>> what git-merge-one-file does.\n>\n> Hmm. I think that it's not so much a \"hook\", as a \"recursive merge is a \n> separate merge policy altogether, that just *defaults* to the same final \n> merge policy\".\n\nI suspect I did not state my problem clearly.\n\n> For example, for a three-way merge, it makes sense to do the three-way \n> merge for the \"internal\" merge too. But it might be that that isn't true \n> for all strategies.\n\nYes, that is what I alluded to in one of my previous message,\nbut I think the \"foo-recursive\" idea is a good one. I was\nplanning to give only \"3-way\" or \"pick ancestor\" choice to\nexternal low-level merge drivers.\n\nBut I think that is a separate issue.\n\nThe logic I said I wanted to override in the message you are\nresponding to is the big toplevel if () else if () ... chain in\nprocess_entry(), and the nested if () else if () ... logic in\nmerge_file(). They roughly implement what git-merge-one-file\ndoes.\n\nCurrently low-level 3-way merge drivers are invoked only in some\ncases, exactly where git-merge-one-file used to invoke \"RCS\nmerge\". The specific case of RelNotes symlink I wanted to\noverride is one of the cases that ll_merge() is not even\ninvoked.\n"},{"id":"39811","messageId":"7vslaxfpvk.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"7vzm55fqyv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T19:23:43Z","receivedAt":"2007-04-18T19:23:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Linus Torvalds writes:\n>\n>> For example, for a three-way merge, it makes sense to do the three-way \n>> merge for the \"internal\" merge too. But it might be that that isn't true \n>> for all strategies.\n>\n> Yes, that is what I alluded to in one of my previous message,\n> but I think the \"foo-recursive\" idea is a good one. I was\n> planning to give only \"3-way\" or \"pick ancestor\" choice to\n> external low-level merge drivers.\n>\n> But I think that is a separate issue.\n\nThe syntax is a bit different from the one you came up in your\nMUA.\n\n\t# a recursive merge will always just pick the ORIGINAL\n\t# version of a file when blending - the blending will\n\t# be done only on the final merge\n\t[merge \"blend\"]\n\t\tname = blend (changelog) strategy\n\t\tdriver = blend %A %O %B\n\t\trecursive = binary\n\nThe variable merge.<>.recursive names a different\nlow level merge driver to be used while performing the virtual\nancestor merge. In this example, the built-in \"binary\" merge\nwhich picks 'ours' for the final round and 'origin' for the\ninternal merge already does what we want.\n\n-- >8 --\n[PATCH] Allow low-level driver to specify different behaviour during internal merge.\n\nThis allows [merge \"drivername\"] to have a variable \"recursive\"\nthat names a different low-level merge driver to be used when\nmerging common ancestors to come up with a virtual ancestor.\n\nSigned-off-by: Junio C Hamano \n---\n merge-recursive.c | 12 ++++++++++++\n 1 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4af69d7..43d6117 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -677,6 +677,7 @@ struct ll_merge_driver {\n \tconst char *name;\n \tconst char *description;\n \tll_merge_fn fn;\n+\tconst char *recursive;\n \tstruct ll_merge_driver *next;\n \tchar *cmdline;\n };\n@@ -934,6 +935,13 @@ static int read_merge_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(\"recursive\", ep)) {\n+\t\tif (!value)\n+\t\t\treturn error(\"%s: lacks value\", var);\n+\t\tfn->recursive = strdup(value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -1013,6 +1021,10 @@ static int ll_merge(mmbuffer_t *result_buf,\n \tmerge_attr = git_path_check_merge(a->path);\n \tdriver = find_ll_merge_driver(merge_attr);\n \n+\tif (index_only && driver->recursive) {\n+\t\tmerge_attr = git_attr(driver->recursive, strlen(driver->recursive));\n+\t\tdriver = find_ll_merge_driver(merge_attr);\n+\t}\n \tmerge_status = driver->fn(driver, a->path,\n \t\t\t\t &orig, &src1, name1, &src2, name2,\n \t\t\t\t result_buf);\n-- \n1.5.1.1.905.g4cad0\n"},{"id":"39813","messageId":"alpine.LFD.0.98.0704181243040.2828@woody.linux-foundation.org","threadId":"7724","inReplyTo":"7vslaxfpvk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T19:43:22Z","receivedAt":"2007-04-18T19:43:22Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Junio C Hamano wrote:\n> \n> The syntax is a bit different from the one you came up in your\n> MUA.\n\n..and I think yours is superior. Make it so.\n\n\t\tLinus\n"},{"id":"39814","messageId":"alpine.LFD.0.98.0704181246160.2828@woody.linux-foundation.org","threadId":"7724","inReplyTo":"7vzm55fqyv.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T19:51:38Z","receivedAt":"2007-04-18T19:51:38Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Junio C Hamano wrote:\n> \n> But I think that is a separate issue.\n> \n> The logic I said I wanted to override in the message you are\n> responding to is the big toplevel if () else if () ... chain in\n> process_entry(), and the nested if () else if () ... logic in\n> merge_file(). They roughly implement what git-merge-one-file\n> does.\n\nAhh. \n\nOk, in that case, I kind of disagree with you. I think the hardcoded \nmerging rules (ie when two of the SHA1's match etc) are the only sane \nthings to do _regardless_ of any merge strategy.\n\nSo yeah, we'll always do one level of merging at the pure SHA1 stage, and \npick that without actually doing any file-level merging at all. I don't \nthink that's wrong. Maybe it means that people cannot pick some really \nstrange merge that they want on a file-by-file basis, but I think that's \nsimply how \"merge-recursive\" is defined.\n\nIOW, I don't think you should be able to turn\n\n\tgit merge -s recursive\n\ninto\n\n\tgit merge -s ours\n\nby making attributes say something like\n\n\t*: merge=ours\n\nbecause I think the attributes are really about what we do WHEN we hit \nfile-level conflicts, while the \"merge strategy\" is a much higher-level \nthing. They are independent.\n\nBut maybe there is some real-world and sane usage that shows me wrong. \n\n\t\tLinus\n"},{"id":"39821","messageId":"Pine.LNX.4.63.0704181304220.2806@qynat.qvtvafvgr.pbz","threadId":"7724","inReplyTo":"alpine.LFD.0.98.0704181330050.2828@woody.linux-foundation.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"David Lang","fromEmail":"david.lang@digitalinsight.com","sentAt":"2007-04-18T20:09:43Z","receivedAt":"2007-04-18T20:09:43Z","isPatch":true,"sender":{"key":"david.lang@digitalinsight.com","avatar":null},"body":"On Wed, 18 Apr 2007, Linus Torvalds wrote:\n\n> On Wed, 18 Apr 2007, Junio C Hamano wrote:\n>>\n> So maybe each strategy could have \"sub-strategies\" for other file types.\n>\n> Ie something like\n>\n> \t[merge \"ours\"]\n> \t\tname = pick our own version\n> \t\tdriver = /bin/true\n> \t\tsymlinks = /bin/true\n>\n> ie we'd use tyhe \"driver\" name for regular files, and the \"symlinks\" name\n> for symlinks and if no \"symlinks\" entry exists, we error it out as a\n> conflict?\n\nthis is starting to sound odd here.\n\nwe have .gitattributes define file types and what merge to use on those types\n\nthen we have this section define different merges to use on different file \ntypes.\n\nI think that we would be better off defining a way for the existing type \ndefinitions to include the 'it's a symlink' type of info (and then deal with the \nmerges from there) instead of spreading the tyep info among different sections.\n\nI could see other applications careing if the thing being handed to it is a \ndirectory, or a named pipe, etc and wanting different rules for them (obviously \nthis wouldn't be relavent for C source, but I think I could see it for other \nthings)\n\nI coudl also see having one external program that can handle the different types \nof files and/or merges. rather than having a different line for each would it \nmake sense to add another variable that could be handed to the merge program \nthat would tell it what sort of merge to do?\n\nDavid Lang\n"},{"id":"39816","messageId":"7vk5w9fnjx.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"alpine.LFD.0.98.0704181246160.2828@woody.linux-foundation.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T20:13:54Z","receivedAt":"2007-04-18T20:13:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> by making attributes say something like\n>\n> \t*: merge=ours\n>\n> because I think the attributes are really about what we do WHEN we hit \n> file-level conflicts, while the \"merge strategy\" is a much higher-level \n> thing. They are independent.\n\nYes, but in the case I had trouble with, I know what I want to\nsee happen when we DO hit file-level conflict on RelNotes\nsymlink. The ancestor says Documentation/RelNotes-1.5.0.txt,\nthe maint branch says Documentation/RelNotes-1.5.0.1.txt, while\nthe current branch says Documentation/RelNotes-1.5.1.txt. The\nlogic in merge_file() simply says \"we would punt on file-level\nconflicts for symlinks\" without giving a chance for low-level\ndrivers to interfere.\n"},{"id":"39819","messageId":"alpine.LFD.0.98.0704181330050.2828@woody.linux-foundation.org","threadId":"7724","inReplyTo":"7vk5w9fnjx.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T20:35:05Z","receivedAt":"2007-04-18T20:35:05Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Junio C Hamano wrote:\n> \n> Yes, but in the case I had trouble with, I know what I want to\n> see happen when we DO hit file-level conflict on RelNotes\n> symlink. The ancestor says Documentation/RelNotes-1.5.0.txt,\n> the maint branch says Documentation/RelNotes-1.5.0.1.txt, while\n> the current branch says Documentation/RelNotes-1.5.1.txt. The\n> logic in merge_file() simply says \"we would punt on file-level\n> conflicts for symlinks\" without giving a chance for low-level\n> drivers to interfere.\n\nAhh, ok. So it really is a file-level conflict, it's just that our \ntraditional merger didn't handle them at all, so we said \"nobody can do \nit\". Fair enough. That does sound like a misfeature, although I would also \nclaim that expecting merge strategies to handle symlinks is likely to fail \nhorribly.\n\nSo maybe each strategy could have \"sub-strategies\" for other file types.\n\nIe something like\n\n\t[merge \"ours\"]\n\t\tname = pick our own version\n\t\tdriver = /bin/true\n\t\tsymlinks = /bin/true\n\nie we'd use tyhe \"driver\" name for regular files, and the \"symlinks\" name \nfor symlinks and if no \"symlinks\" entry exists, we error it out as a \nconflict?\n\n\t\tLinus\n"},{"id":"39826","messageId":"7vfy6xflca.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"Pine.LNX.4.63.0704181304220.2806@qynat.qvtvafvgr.pbz","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T21:01:41Z","receivedAt":"2007-04-18T21:01:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Lang writes:\n\n> On Wed, 18 Apr 2007, Linus Torvalds wrote:\n>\n> I could also see having one external program that can handle the\n> different types of files and/or merges. rather than having a different\n> line for each would it make sense to add another variable that could\n> be handed to the merge program that would tell it what sort of merge\n> to do?\n\nThat was the direction I was thinking about.\n"},{"id":"39866","messageId":"20070419075229.GA27208@admingilde.org","threadId":"7724","inReplyTo":"alpine.LFD.0.98.0704181330050.2828@woody.linux-foundation.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2007-04-19T07:52:30Z","receivedAt":"2007-04-19T07:52:30Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"On Wed, Apr 18, 2007 at 01:35:05PM -0700, Linus Torvalds wrote:\n> So maybe each strategy could have \"sub-strategies\" for other file types.\n> \n> Ie something like\n> \n> \t[merge \"ours\"]\n> \t\tname = pick our own version\n> \t\tdriver = /bin/true\n> \t\tsymlinks = /bin/true\n\nor different attributes for other file types:\n\n[attr] *@ merge=ours\t\t# for symlinks\n[attr] */ merge=submodule\t# for dirlinks\n\n-- \nMartin Waitz\n"},{"id":"39867","messageId":"7v1wig9477.fsf@assigned-by-dhcp.cox.net","threadId":"7724","inReplyTo":"20070419075229.GA27208@admingilde.org","subject":"Re: [PATCH 0/2] Custom low-level merge driver support.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-19T08:08:28Z","receivedAt":"2007-04-19T08:08:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz writes:\n\n> or different attributes for other file types:\n>\n> [attr] *@ merge=ours\t\t# for symlinks\n> [attr] */ merge=submodule\t# for dirlinks\nWe already have much more general mechanism to model this on for\nmerges. Look at how git-merge-one-file is invoked. It gets the\nmode bits which tells what kind of object the path is, for all\nthree (i.e. ancestor, ours, and theirs).\n\nSo a single, perhaps not so low-level, merge driver can take the\nsame set of attributes merge-one-file makes use of, not just the\ncurrent set of %O/%A/%B temporary files, and make intelligent\ndecision to override what is hardcoded in merge-recursive if it\nwants to.\n\nWe do not need to pollute the gitattribute mechanism for this\napplication; it can and should stay about path*name*s.\n"}]}