{"thread":{"id":"21758","subject":"[PATCH 0/8] The return of -Xours, -Xtheirs, -Xsubtree=dir","startedAt":"2009-11-26T02:23:52Z","lastAt":"2009-11-30T20:02:51Z","messageCount":26,"participants":["Avery Pennarun","Junio C Hamano","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"128422","messageId":"cover.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":null,"subject":"[PATCH 0/8] The return of -Xours, -Xtheirs, -Xsubtree=dir","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:52Z","receivedAt":"2009-11-26T02:23:52Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"As discussed earlier today, this brings back Junio's earlier patch series\nthat introduced (and then used) a -X option for configuring merge\nstrategies.  My favourite use of this is -Xsubtree=<dir>, which lets you\nprovide the actual subdir prefix when using the subtree merge strategy.\n\nAvery Pennarun (8):\n  git-merge-file --ours, --theirs\n  builtin-merge.c: call exclude_cmds() correctly.\n  git-merge-recursive-{ours,theirs}\n  Teach git-merge to pass -X<option> to the backend strategy module\n  Teach git-pull to pass -X<option> to git-merge\n  Make \"subtree\" part more orthogonal to the rest of merge-recursive.\n  Extend merge-subtree tests to test -Xsubtree=dir.\n  Document that merge strategies can now take their own options\n\n .gitignore                         |    2 +\n Documentation/git-merge-file.txt   |   12 +++++-\n Documentation/merge-options.txt    |    4 ++\n Documentation/merge-strategies.txt |   29 ++++++++++++++-\n builtin-checkout.c                 |    2 +-\n builtin-merge-file.c               |    5 ++-\n builtin-merge-recursive.c          |   24 ++++++++++---\n builtin-merge.c                    |   44 +++++++++++++++++++++--\n cache.h                            |    1 +\n contrib/examples/git-merge.sh      |    3 +-\n git-compat-util.h                  |    1 +\n git-pull.sh                        |   17 ++++++++-\n git.c                              |    2 +\n ll-merge.c                         |   20 +++++-----\n ll-merge.h                         |    2 +-\n match-trees.c                      |   69 +++++++++++++++++++++++++++++++++++-\n merge-recursive.c                  |   35 +++++++++++++++---\n merge-recursive.h                  |    7 +++-\n strbuf.c                           |    9 +++++\n t/t6029-merge-subtree.sh           |   47 ++++++++++++++++++++++++-\n t/t6034-merge-ours-theirs.sh       |   64 +++++++++++++++++++++++++++++++++\n xdiff/xdiff.h                      |    7 +++-\n xdiff/xmerge.c                     |   11 +++++-\n 23 files changed, 377 insertions(+), 40 deletions(-)\n create mode 100755 t/t6034-merge-ours-theirs.sh\n"},{"id":"128423","messageId":"d243a513ffb8da4272f7a0e13a711f9b65195c25.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"cover.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 1/8] git-merge-file --ours, --theirs","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:53Z","receivedAt":"2009-11-26T02:23:53Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"Often people want their conflicting merges autoresolved by favouring\nupstream changes (or their own --- it's the same thing), and hinted to run\n\"git diff --name-only | xargs git checkout MERGE_HEAD --\".  This is\nessentially to accept automerge results for the paths that are fully\nresolved automatically while taking their version of the file in full for\npaths that have conflicts.\n\nThis is problematic on two counts.\n\nOne problem is that this is not exactly what these people want.  They\nusually want to salvage as much automerge result as possible.  In\nparticular, they want to keep autoresolved parts in conflicting paths, as\nwell as the paths that are fully autoresolved.\n\nThis patch teaches two new modes of operation to the lowest-lever merge\nmachinery, xdl_merge().  Instead of leaving the conflicted lines from both\nsides enclosed in <<<, ===, and >>> markers, you can tell the conflicts to\nbe resolved favouring your side or their side of changes.\n\nA larger problem is that this tends to encourage a bad workflow by\nallowing them to record such a mixed up half-merge result as a full commit\nwithout auditing.  This commit does not tackle this latter issue.  In git,\nwe usually give long enough rope to users with strange wishes as long as\nthe risky features is not on by default.\n\n(Patch originally by Junio Hamano <gitster@pobox.com>.)\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n Documentation/git-merge-file.txt |   12 ++++++++++--\n builtin-merge-file.c             |    5 ++++-\n xdiff/xdiff.h                    |    7 ++++++-\n xdiff/xmerge.c                   |   11 +++++++++--\n 4 files changed, 29 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex 3035373..b9d2276 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -10,7 +10,8 @@ SYNOPSIS\n --------\n [verse]\n 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]\n-\t[-p|--stdout] [-q|--quiet] <current-file> <base-file> <other-file>\n+\t[--ours|--theirs] [-p|--stdout] [-q|--quiet]\n+\t<current-file> <base-file> <other-file>\n \n \n DESCRIPTION\n@@ -34,7 +35,9 @@ normally outputs a warning and brackets the conflict with lines containing\n \t>>>>>>> B\n \n If there are conflicts, the user should edit the result and delete one of\n-the alternatives.\n+the alternatives.  When `--ours` or `--theirs` option is in effect, however,\n+these conflicts are resolved favouring lines from `<current-file>` or\n+lines from `<other-file>` respectively.\n \n The exit value of this program is negative on error, and the number of\n conflicts otherwise. If the merge was clean, the exit value is 0.\n@@ -62,6 +65,11 @@ OPTIONS\n -q::\n \tQuiet; do not warn about conflicts.\n \n+--ours::\n+--theirs::\n+\tInstead of leaving conflicts in the file, resolve conflicts\n+\tfavouring our (or their) side of the lines.\n+\n \n EXAMPLES\n --------\ndiff --git a/builtin-merge-file.c b/builtin-merge-file.c\nindex afd2ea7..8f22aa8 100644\n--- a/builtin-merge-file.c\n+++ b/builtin-merge-file.c\n@@ -29,11 +29,14 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tint ret = 0, i = 0, to_stdout = 0;\n \tint merge_level = XDL_MERGE_ZEALOUS_ALNUM;\n \tint merge_style = 0, quiet = 0;\n+\tint merge_favor = 0;\n \tint nongit;\n \n \tstruct option options[] = {\n \t\tOPT_BOOLEAN('p', \"stdout\", &to_stdout, \"send results to standard output\"),\n \t\tOPT_SET_INT(0, \"diff3\", &merge_style, \"use a diff3 based merge\", XDL_MERGE_DIFF3),\n+\t\tOPT_SET_INT(0, \"ours\", &merge_favor, \"for conflicts, use our version\", XDL_MERGE_FAVOR_OURS),\n+\t\tOPT_SET_INT(0, \"theirs\", &merge_favor, \"for conflicts, use their version\", XDL_MERGE_FAVOR_THEIRS),\n \t\tOPT__QUIET(&quiet),\n \t\tOPT_CALLBACK('L', NULL, names, \"name\",\n \t\t\t     \"set labels for file1/orig_file/file2\", &label_cb),\n@@ -68,7 +71,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t}\n \n \tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n-\t\t\t&xpp, merge_level | merge_style, &result);\n+\t\t\t&xpp, merge_level | merge_style | merge_favor, &result);\n \n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfs[i].ptr);\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 4da052a..2cce49d 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -58,6 +58,11 @@ extern \"C\" {\n #define XDL_MERGE_ZEALOUS_ALNUM 3\n #define XDL_MERGE_LEVEL_MASK 0x0f\n \n+/* merge favor modes */\n+#define XDL_MERGE_FAVOR_OURS 0x0010\n+#define XDL_MERGE_FAVOR_THEIRS 0x0020\n+#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)\n+\n /* merge output styles */\n #define XDL_MERGE_DIFF3 0x8000\n #define XDL_MERGE_STYLE_MASK 0x8000\n@@ -110,7 +115,7 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,\n \n int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,\n \t\tmmfile_t *mf2, const char *name2,\n-\t\txpparam_t const *xpp, int level, mmbuffer_t *result);\n+\t\txpparam_t const *xpp, int flags, mmbuffer_t *result);\n \n #ifdef __cplusplus\n }\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 1cb65a9..2325f6d 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -214,11 +214,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \n static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n \t\t\t\t xdfenv_t *xe2, const char *name2,\n+\t\t\t\t int favor,\n \t\t\t\t xdmerge_t *m, char *dest, int style)\n {\n \tint size, i;\n \n \tfor (size = i = 0; m; m = m->next) {\n+\t\tif (favor && !m->mode)\n+                \tm->mode = favor;\n+\t\t\n \t\tif (m->mode == 0)\n \t\t\tsize = fill_conflict_hunk(xe1, name1, xe2, name2,\n \t\t\t\t\t\t  size, i, style, m, dest);\n@@ -391,6 +395,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \tint i0, i1, i2, chg0, chg1, chg2;\n \tint level = flags & XDL_MERGE_LEVEL_MASK;\n \tint style = flags & XDL_MERGE_STYLE_MASK;\n+\tint favor = XDL_MERGE_FAVOR(flags);\n \n \tif (style == XDL_MERGE_DIFF3) {\n \t\t/*\n@@ -523,14 +528,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t/* output */\n \tif (result) {\n \t\tint size = xdl_fill_merge_buffer(xe1, name1, xe2, name2,\n-\t\t\tchanges, NULL, style);\n+\t\t\tfavor, changes, NULL, style);\n \t\tresult->ptr = xdl_malloc(size);\n \t\tif (!result->ptr) {\n \t\t\txdl_cleanup_merge(changes);\n \t\t\treturn -1;\n \t\t}\n \t\tresult->size = size;\n-\t\txdl_fill_merge_buffer(xe1, name1, xe2, name2, changes,\n+\t\txdl_fill_merge_buffer(xe1, name1, xe2, name2, favor, changes,\n \t\t\t\t      result->ptr, style);\n \t}\n \treturn xdl_cleanup_merge(changes);\n@@ -542,6 +547,8 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,\n \txdchange_t *xscr1, *xscr2;\n \txdfenv_t xe1, xe2;\n \tint status;\n+\tint level = flags & XDL_MERGE_LEVEL_MASK;\n+\tint favor = XDL_MERGE_FAVOR(flags);\n \n \tresult->ptr = NULL;\n \tresult->size = 0;\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128429","messageId":"905749faf5ccb2c7c54d3318dbc662d69daf8d0e.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"d243a513ffb8da4272f7a0e13a711f9b65195c25.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 2/8] builtin-merge.c: call exclude_cmds() correctly.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:54Z","receivedAt":"2009-11-26T02:23:54Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"We need to call exclude_cmds() after the loop, not during the loop, because\nexcluding a command from the array can change the indexes of objects in the\narray.  The result is that, depending on file ordering, some commands\nweren't excluded as they should have been.\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n builtin-merge.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 57eedd4..855cf65 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -107,8 +107,8 @@ static struct strategy *get_strategy(const char *name)\n \t\t\t\t\tfound = 1;\n \t\t\tif (!found)\n \t\t\t\tadd_cmdname(&not_strategies, ent->name, ent->len);\n-\t\t\texclude_cmds(&main_cmds, &not_strategies);\n \t\t}\n+\t\texclude_cmds(&main_cmds, &not_strategies);\n \t}\n \tif (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {\n \t\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\", name);\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128428","messageId":"7e1f1179fc5fe2f568e2c75f75366fa40d7bbbfb.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"905749faf5ccb2c7c54d3318dbc662d69daf8d0e.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:55Z","receivedAt":"2009-11-26T02:23:55Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"This uses the low-level mechanism for \"ours\" and \"theirs\" autoresolution\nintroduced by the previous commit to introduce two additional merge\nstrategies, merge-recursive-ours and merge-recursive-theirs.\n\n(Patch originally by Junio Hamano <gitster@pobox.com>.)\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n .gitignore                    |    2 +\n Makefile                      |    3 ++\n builtin-checkout.c            |    2 +-\n builtin-merge-recursive.c     |    9 ++++--\n builtin-merge.c               |    4 ++-\n contrib/examples/git-merge.sh |    3 +-\n git-compat-util.h             |    1 +\n git.c                         |    2 +\n ll-merge.c                    |   20 +++++++-------\n ll-merge.h                    |    2 +-\n merge-recursive.c             |   21 ++++++++++++++-\n merge-recursive.h             |    6 +++-\n strbuf.c                      |    9 ++++++\n t/t6034-merge-ours-theirs.sh  |   56 +++++++++++++++++++++++++++++++++++++++++\n 14 files changed, 120 insertions(+), 20 deletions(-)\n create mode 100755 t/t6034-merge-ours-theirs.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex ac02a58..87467d6 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -79,6 +79,8 @@\n /git-merge-one-file\n /git-merge-ours\n /git-merge-recursive\n+/git-merge-recursive-ours\n+/git-merge-recursive-theirs\n /git-merge-resolve\n /git-merge-subtree\n /git-mergetool\ndiff --git a/Makefile b/Makefile\nindex 5a0b3d4..f92b375 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -401,6 +401,8 @@ BUILT_INS += git-format-patch$X\n BUILT_INS += git-fsck-objects$X\n BUILT_INS += git-get-tar-commit-id$X\n BUILT_INS += git-init$X\n+BUILT_INS += git-merge-recursive-ours$X\n+BUILT_INS += git-merge-recursive-theirs$X\n BUILT_INS += git-merge-subtree$X\n BUILT_INS += git-peek-remote$X\n BUILT_INS += git-repo-config$X\n@@ -1909,6 +1911,7 @@ check-docs::\n \tdo \\\n \t\tcase \"$$v\" in \\\n \t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n+\t\tgit-merge-recursive-ours | git-merge-recursive-theirs | \\\n \t\tgit-merge-resolve | git-merge-subtree | \\\n \t\tgit-fsck-objects | git-init-db | \\\n \t\tgit-?*--?* ) continue ;; \\\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 64f3a11..b392d1b 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -167,7 +167,7 @@ static int checkout_merged(int pos, struct checkout *state)\n \tfill_mm(active_cache[pos+2]->sha1, &theirs);\n \n \tstatus = ll_merge(&result_buf, path, &ancestor,\n-\t\t\t  &ours, \"ours\", &theirs, \"theirs\", 1);\n+\t\t\t  &ours, \"ours\", &theirs, \"theirs\", 1, 0);\n \tfree(ancestor.ptr);\n \tfree(ours.ptr);\n \tfree(theirs.ptr);\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 710674c..f5082da 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -27,9 +27,12 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tinit_merge_options(&o);\n \tif (argv[0]) {\n \t\tint namelen = strlen(argv[0]);\n-\t\tif (8 < namelen &&\n-\t\t    !strcmp(argv[0] + namelen - 8, \"-subtree\"))\n-\t\t\to.subtree_merge = 1;\n+\t\tif (!suffixcmp(argv[0], \"-subtree\"))\n+\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\telse if (!suffixcmp(argv[0], \"-ours\"))\n+\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n+\t\telse if (!suffixcmp(argv[0], \"-theirs\"))\n+\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n \t}\n \n \tif (argc < 4)\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 855cf65..df089bb 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -55,6 +55,8 @@ static int verbosity;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\",  DEFAULT_TWOHEAD | NO_TRIVIAL },\n+\t{ \"recursive-ours\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n+\t{ \"recursive-theirs\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n \t{ \"octopus\",    DEFAULT_OCTOPUS },\n \t{ \"resolve\",    0 },\n \t{ \"ours\",       NO_FAST_FORWARD | NO_TRIVIAL },\n@@ -563,7 +565,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \n \t\tinit_merge_options(&o);\n \t\tif (!strcmp(strategy, \"subtree\"))\n-\t\t\to.subtree_merge = 1;\n+\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n \n \t\to.branch1 = head_arg;\n \t\to.branch2 = remoteheads->item->util;\ndiff --git a/contrib/examples/git-merge.sh b/contrib/examples/git-merge.sh\nindex 500635f..8f617fc 100755\n--- a/contrib/examples/git-merge.sh\n+++ b/contrib/examples/git-merge.sh\n@@ -31,10 +31,11 @@ LF='\n '\n \n all_strategies='recur recursive octopus resolve stupid ours subtree'\n+all_strategies=\"$all_strategies recursive-ours recursive-theirs\"\n default_twohead_strategies='recursive'\n default_octopus_strategies='octopus'\n no_fast_forward_strategies='subtree ours'\n-no_trivial_strategies='recursive recur subtree ours'\n+no_trivial_strategies='recursive recur subtree ours recursive-ours recursive-theirs'\n use_strategies=\n \n allow_fast_forward=t\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 5c59687..f64cc45 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -198,6 +198,7 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))\n extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));\n \n extern int prefixcmp(const char *str, const char *prefix);\n+extern int suffixcmp(const char *str, const char *suffix);\n extern time_t tm_to_time_t(const struct tm *tm);\n \n static inline const char *skip_prefix(const char *str, const char *prefix)\ndiff --git a/git.c b/git.c\nindex 11544cd..4735f11 100644\n--- a/git.c\n+++ b/git.c\n@@ -332,6 +332,8 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"merge-file\", cmd_merge_file },\n \t\t{ \"merge-ours\", cmd_merge_ours, RUN_SETUP },\n \t\t{ \"merge-recursive\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"merge-recursive-ours\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"merge-recursive-theirs\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"mktree\", cmd_mktree, RUN_SETUP },\n \t\t{ \"mv\", cmd_mv, RUN_SETUP | NEED_WORK_TREE },\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 2d6b6d6..cc6814f 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -18,7 +18,7 @@ typedef int (*ll_merge_fn)(const struct ll_merge_driver *,\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   int virtual_ancestor);\n+\t\t\t   int virtual_ancestor, int favor);\n \n struct ll_merge_driver {\n \tconst char *name;\n@@ -38,7 +38,7 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_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   int virtual_ancestor)\n+\t\t\t   int virtual_ancestor, int favor)\n {\n \t/*\n \t * The tentative merge result is \"ours\" for the final round,\n@@ -59,7 +59,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_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\tint virtual_ancestor)\n+\t\t\tint virtual_ancestor, int favor)\n {\n \txpparam_t xpp;\n \tint style = 0;\n@@ -73,7 +73,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\n \t\t\t\t       path,\n \t\t\t\t       orig, src1, name1,\n \t\t\t\t       src2, name2,\n-\t\t\t\t       virtual_ancestor);\n+\t\t\t\t       virtual_ancestor, favor);\n \t}\n \n \tmemset(&xpp, 0, sizeof(xpp));\n@@ -82,7 +82,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,\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 | style,\n+\t\t\t &xpp, XDL_MERGE_ZEALOUS | style | favor,\n \t\t\t result);\n }\n \n@@ -92,7 +92,7 @@ static int ll_union_merge(const struct ll_merge_driver *drv_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  int virtual_ancestor)\n+\t\t\t  int virtual_ancestor, int favor)\n {\n \tchar *src, *dst;\n \tlong size;\n@@ -104,7 +104,7 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,\n \tgit_xmerge_style = 0;\n \tstatus = ll_xdl_merge(drv_unused, result, path_unused,\n \t\t\t      orig, src1, NULL, src2, NULL,\n-\t\t\t      virtual_ancestor);\n+\t\t\t      virtual_ancestor, favor);\n \tgit_xmerge_style = saved_style;\n \tif (status <= 0)\n \t\treturn status;\n@@ -165,7 +165,7 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,\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\tint virtual_ancestor)\n+\t\t\tint virtual_ancestor, int favor)\n {\n \tchar temp[3][50];\n \tstruct strbuf cmd = STRBUF_INIT;\n@@ -356,7 +356,7 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     mmfile_t *ancestor,\n \t     mmfile_t *ours, const char *our_label,\n \t     mmfile_t *theirs, const char *their_label,\n-\t     int virtual_ancestor)\n+\t     int virtual_ancestor, int favor)\n {\n \tconst char *ll_driver_name;\n \tconst struct ll_merge_driver *driver;\n@@ -369,5 +369,5 @@ int ll_merge(mmbuffer_t *result_buf,\n \treturn driver->fn(driver, result_buf, path,\n \t\t\t  ancestor,\n \t\t\t  ours, our_label,\n-\t\t\t  theirs, their_label, virtual_ancestor);\n+\t\t\t  theirs, their_label, virtual_ancestor, favor);\n }\ndiff --git a/ll-merge.h b/ll-merge.h\nindex 5388422..2c94fdb 100644\n--- a/ll-merge.h\n+++ b/ll-merge.h\n@@ -10,6 +10,6 @@ int ll_merge(mmbuffer_t *result_buf,\n \t     mmfile_t *ancestor,\n \t     mmfile_t *ours, const char *our_label,\n \t     mmfile_t *theirs, const char *their_label,\n-\t     int virtual_ancestor);\n+\t     int virtual_ancestor, int favor);\n \n #endif\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a91208f..257bf8f 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -642,6 +642,23 @@ static int merge_3way(struct merge_options *o,\n \tmmfile_t orig, src1, src2;\n \tchar *name1, *name2;\n \tint merge_status;\n+\tint favor;\n+\t\n+\tif (o->call_depth)\n+        \tfavor = 0;\n+\telse {\n+\t\tswitch (o->recursive_variant) {\n+\t\tcase MERGE_RECURSIVE_OURS:\n+\t\t\tfavor = XDL_MERGE_FAVOR_OURS;\n+\t\t\tbreak;\n+\t\tcase MERGE_RECURSIVE_THEIRS:\n+\t\t\tfavor = XDL_MERGE_FAVOR_THEIRS;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tfavor = 0;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \n \tif (strcmp(a->path, b->path)) {\n \t\tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n@@ -657,7 +674,7 @@ static int merge_3way(struct merge_options *o,\n \n \tmerge_status = ll_merge(result_buf, a->path, &orig,\n \t\t\t\t&src1, name1, &src2, name2,\n-\t\t\t\to->call_depth);\n+\t\t\t\to->call_depth, favor);\n \n \tfree(name1);\n \tfree(name2);\n@@ -1196,7 +1213,7 @@ int merge_trees(struct merge_options *o,\n {\n \tint code, clean;\n \n-\tif (o->subtree_merge) {\n+\tif (o->recursive_variant == MERGE_RECURSIVE_SUBTREE) {\n \t\tmerge = shift_tree_object(head, merge);\n \t\tcommon = shift_tree_object(head, common);\n \t}\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex fd138ca..9d54219 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -6,7 +6,11 @@\n struct merge_options {\n \tconst char *branch1;\n \tconst char *branch2;\n-\tunsigned subtree_merge : 1;\n+\tenum {\n+        \tMERGE_RECURSIVE_SUBTREE = 1,\n+        \tMERGE_RECURSIVE_OURS,\n+        \tMERGE_RECURSIVE_THEIRS,\n+\t} recursive_variant;\n \tunsigned buffer_output : 1;\n \tint verbosity;\n \tint diff_rename_limit;\ndiff --git a/strbuf.c b/strbuf.c\nindex a6153dc..d71a623 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -10,6 +10,15 @@ int prefixcmp(const char *str, const char *prefix)\n \t\t\treturn (unsigned char)*prefix - (unsigned char)*str;\n }\n \n+int suffixcmp(const char *str, const char *suffix)\n+{\n+\tint len = strlen(str), suflen = strlen(suffix);\n+\tif (len < suflen)\n+\t\treturn -1;\n+\telse\n+\t\treturn strcmp(str + len - suflen, suffix);\n+}\n+\n /*\n  * Used as the default ->buf value, so that people can always assume\n  * buf is non NULL and ->buf is NUL terminated even for a freshly\ndiff --git a/t/t6034-merge-ours-theirs.sh b/t/t6034-merge-ours-theirs.sh\nnew file mode 100755\nindex 0000000..56a9247\n--- /dev/null\n+++ b/t/t6034-merge-ours-theirs.sh\n@@ -0,0 +1,56 @@\n+#!/bin/sh\n+\n+test_description='Merge-recursive ours and theirs variants'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tfor i in 1 2 3 4 5 6 7 8 9\n+\tdo\n+\t\techo \"$i\"\n+\tdone >file &&\n+\tgit add file &&\n+\tcp file elif &&\n+\tgit commit -m initial &&\n+\n+\tsed -e \"s/1/one/\" -e \"s/9/nine/\" >file <elif &&\n+\tgit commit -a -m ours &&\n+\n+\tgit checkout -b side HEAD^ &&\n+\n+\tsed -e \"s/9/nueve/\" >file <elif &&\n+\tgit commit -a -m theirs &&\n+\n+\tgit checkout master^0\n+'\n+\n+test_expect_success 'plain recursive - should conflict' '\n+\tgit reset --hard master &&\n+\ttest_must_fail git merge -s recursive side &&\n+\tgrep nine file &&\n+\tgrep nueve file &&\n+\t! grep 9 file &&\n+\tgrep one file &&\n+\t! grep 1 file\n+'\n+\n+test_expect_success 'recursive favouring theirs' '\n+\tgit reset --hard master &&\n+\tgit merge -s recursive-theirs side &&\n+\t! grep nine file &&\n+\tgrep nueve file &&\n+\t! grep 9 file &&\n+\tgrep one file &&\n+\t! grep 1 file\n+'\n+\n+test_expect_success 'recursive favouring ours' '\n+\tgit reset --hard master &&\n+\tgit merge -s recursive-ours side &&\n+\tgrep nine file &&\n+\t! grep nueve file &&\n+\t! grep 9 file &&\n+\tgrep one file &&\n+\t! grep 1 file\n+'\n+\n+test_done\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128425","messageId":"73a42e99b4a083c74b017caf2970d1bbf5886b96.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"7e1f1179fc5fe2f568e2c75f75366fa40d7bbbfb.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 4/8] Teach git-merge to pass -X<option> to the backend strategy module","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:56Z","receivedAt":"2009-11-26T02:23:56Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"Distinguishing slight variation of modes of operation between the vanilla\nmerge-recursive and merge-recursive-ours by the command name may have been\nan easy way to experiment, but we should bite the bullet and allow backend\nspecific options to be given by the end user.\n\n(Patch originally by Junio Hamano <gitster@pobox.com>.)\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n Makefile                     |    3 ---\n builtin-merge-recursive.c    |   21 +++++++++++++++------\n builtin-merge.c              |   40 ++++++++++++++++++++++++++++++++++++----\n t/t6034-merge-ours-theirs.sh |    4 ++--\n 4 files changed, 53 insertions(+), 15 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex f92b375..5a0b3d4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -401,8 +401,6 @@ BUILT_INS += git-format-patch$X\n BUILT_INS += git-fsck-objects$X\n BUILT_INS += git-get-tar-commit-id$X\n BUILT_INS += git-init$X\n-BUILT_INS += git-merge-recursive-ours$X\n-BUILT_INS += git-merge-recursive-theirs$X\n BUILT_INS += git-merge-subtree$X\n BUILT_INS += git-peek-remote$X\n BUILT_INS += git-repo-config$X\n@@ -1911,7 +1909,6 @@ check-docs::\n \tdo \\\n \t\tcase \"$$v\" in \\\n \t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n-\t\tgit-merge-recursive-ours | git-merge-recursive-theirs | \\\n \t\tgit-merge-resolve | git-merge-subtree | \\\n \t\tgit-fsck-objects | git-init-db | \\\n \t\tgit-?*--?* ) continue ;; \\\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex f5082da..53f8f05 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -29,18 +29,27 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\tint namelen = strlen(argv[0]);\n \t\tif (!suffixcmp(argv[0], \"-subtree\"))\n \t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n-\t\telse if (!suffixcmp(argv[0], \"-ours\"))\n-\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\telse if (!suffixcmp(argv[0], \"-theirs\"))\n-\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n \t}\n \n \tif (argc < 4)\n \t\tusagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n \n \tfor (i = 1; i < argc; ++i) {\n-\t\tif (!strcmp(argv[i], \"--\"))\n-\t\t\tbreak;\n+\t\tconst char *arg = argv[i];\n+\n+\t\tif (!prefixcmp(arg, \"--\")) {\n+\t\t\tif (!arg[2])\n+\t\t\t\tbreak;\n+\t\t\tif (!strcmp(arg+2, \"ours\"))\n+\t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n+\t\t\telse if (!strcmp(arg+2, \"theirs\"))\n+\t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n+\t\t\telse if (!strcmp(arg+2, \"subtree\"))\n+\t\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\t\telse\n+\t\t\t\tdie(\"Unknown option %s\", arg);\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (bases_count < ARRAY_SIZE(bases)-1) {\n \t\t\tunsigned char *sha = xmalloc(20);\n \t\t\tif (get_sha1(argv[i], sha))\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex df089bb..9a95bc8 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -50,13 +50,13 @@ static struct commit_list *remoteheads;\n static unsigned char head[20], stash[20];\n static struct strategy **use_strategies;\n static size_t use_strategies_nr, use_strategies_alloc;\n+static const char **xopts;\n+static size_t xopts_nr, xopts_alloc;\n static const char *branch;\n static int verbosity;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\",  DEFAULT_TWOHEAD | NO_TRIVIAL },\n-\t{ \"recursive-ours\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n-\t{ \"recursive-theirs\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n \t{ \"octopus\",    DEFAULT_OCTOPUS },\n \t{ \"resolve\",    0 },\n \t{ \"ours\",       NO_FAST_FORWARD | NO_TRIVIAL },\n@@ -148,6 +148,17 @@ static int option_parse_strategy(const struct option *opt,\n \treturn 0;\n }\n \n+static int option_parse_x(const struct option *opt,\n+\t\t\t  const char *arg, int unset)\n+{\n+\tif (unset)\n+\t\treturn 0;\n+\t\n+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n+\txopts[xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n static int option_parse_n(const struct option *opt,\n \t\t\t  const char *arg, int unset)\n {\n@@ -174,6 +185,8 @@ static struct option builtin_merge_options[] = {\n \t\t\"abort if fast-forward is not possible\"),\n \tOPT_CALLBACK('s', \"strategy\", &use_strategies, \"strategy\",\n \t\t\"merge strategy to use\", option_parse_strategy),\n+\tOPT_CALLBACK('X', \"extended\", &xopts, \"option=value\",\n+\t\t\"option for selected merge strategy\", option_parse_x),\n \tOPT_CALLBACK('m', \"message\", &merge_msg, \"message\",\n \t\t\"message to be used for the merge commit (if any)\",\n \t\toption_parse_message),\n@@ -536,7 +549,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\t      const char *head_arg)\n {\n \tconst char **args;\n-\tint i = 0, ret;\n+\tint i = 0, x = 0, ret;\n \tstruct commit_list *j;\n \tstruct strbuf buf = STRBUF_INIT;\n \tint index_fd;\n@@ -566,6 +579,17 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\tinit_merge_options(&o);\n \t\tif (!strcmp(strategy, \"subtree\"))\n \t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\t\t\n+\t\tfor (x = 0; x < xopts_nr; x++) {\n+\t\t\tif (!strcmp(xopts[x], \"ours\"))\n+\t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n+\t\t\telse if (!strcmp(xopts[x], \"theirs\"))\n+\t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n+\t\t\telse if (!strcmp(xopts[x], \"subtree\"))\n+\t\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\t\telse\n+\t\t\t\tdie(\"Unknown option for merge-recursive: -X%s\", xopts[x]);\n+\t\t}\n \n \t\to.branch1 = head_arg;\n \t\to.branch2 = remoteheads->item->util;\n@@ -583,10 +607,16 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\trollback_lock_file(lock);\n \t\treturn clean ? 0 : 1;\n \t} else {\n-\t\targs = xmalloc((4 + commit_list_count(common) +\n+\t\targs = xmalloc((4 + xopts_nr + commit_list_count(common) +\n \t\t\t\t\tcommit_list_count(remoteheads)) * sizeof(char *));\n \t\tstrbuf_addf(&buf, \"merge-%s\", strategy);\n \t\targs[i++] = buf.buf;\n+\t\tfor (x = 0; x < xopts_nr; x++) {\n+\t\t\tchar *s = xmalloc(strlen(xopts[x])+2+1);\n+\t\t\tstrcpy(s, \"--\");\n+\t\t\tstrcpy(s+2, xopts[x]);\n+\t\t\targs[i++] = s;\n+\t\t}\n \t\tfor (j = common; j; j = j->next)\n \t\t\targs[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));\n \t\targs[i++] = \"--\";\n@@ -597,6 +627,8 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\tret = run_command_v_opt(args, RUN_GIT_CMD);\n \t\tstrbuf_release(&buf);\n \t\ti = 1;\n+\t\tfor (x = 0; x < xopts_nr; x++)\n+\t\t\tfree((void *)args[i++]);\n \t\tfor (j = common; j; j = j->next)\n \t\t\tfree((void *)args[i++]);\n \t\ti += 2;\ndiff --git a/t/t6034-merge-ours-theirs.sh b/t/t6034-merge-ours-theirs.sh\nindex 56a9247..08c9f79 100755\n--- a/t/t6034-merge-ours-theirs.sh\n+++ b/t/t6034-merge-ours-theirs.sh\n@@ -35,7 +35,7 @@ test_expect_success 'plain recursive - should conflict' '\n \n test_expect_success 'recursive favouring theirs' '\n \tgit reset --hard master &&\n-\tgit merge -s recursive-theirs side &&\n+\tgit merge -s recursive -Xtheirs side &&\n \t! grep nine file &&\n \tgrep nueve file &&\n \t! grep 9 file &&\n@@ -45,7 +45,7 @@ test_expect_success 'recursive favouring theirs' '\n \n test_expect_success 'recursive favouring ours' '\n \tgit reset --hard master &&\n-\tgit merge -s recursive-ours side &&\n+\tgit merge -s recursive -X ours side &&\n \tgrep nine file &&\n \t! grep nueve file &&\n \t! grep 9 file &&\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128427","messageId":"1ff0b2f7e3fae4cc6c7610c92711f33df9a3d07c.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"73a42e99b4a083c74b017caf2970d1bbf5886b96.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 5/8] Teach git-pull to pass -X<option> to git-merge","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:57Z","receivedAt":"2009-11-26T02:23:57Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"(Patch originally by Junio Hamano <gitster@pobox.com>.)\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n git-pull.sh                  |   17 +++++++++++++++--\n t/t6034-merge-ours-theirs.sh |    8 ++++++++\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex bfeb4a0..6d961b6 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -18,6 +18,7 @@ test -z \"$(git ls-files -u)\" ||\n \n strategy_args= diffstat= no_commit= squash= no_ff= ff_only=\n log_arg= verbosity=\n+merge_args=\n curr_branch=$(git symbolic-ref -q HEAD)\n curr_branch_short=$(echo \"$curr_branch\" | sed \"s|refs/heads/||\")\n rebase=$(git config --bool branch.$curr_branch_short.rebase)\n@@ -62,6 +63,18 @@ do\n \t\tesac\n \t\tstrategy_args=\"${strategy_args}-s $strategy \"\n \t\t;;\n+\t-X*)\n+\t\tcase \"$#,$1\" in\n+\t\t1,-X)\n+\t\t\tusage ;;\n+\t\t*,-X)\n+\t\t\txx=\"-X $2\"\n+\t\t\tshift ;;\n+\t\t*,*)\n+\t\t\txx=\"$1\" ;;\n+\t\tesac\n+\t\tmerge_args=\"$merge_args$xx \"\n+\t\t;;\n \t-r|--r|--re|--reb|--reba|--rebas|--rebase)\n \t\trebase=true\n \t\t;;\n@@ -216,7 +229,7 @@ fi\n \n merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n test true = \"$rebase\" &&\n-\texec git-rebase $diffstat $strategy_args --onto $merge_head \\\n+\texec git-rebase $diffstat $strategy_args $merge_args --onto $merge_head \\\n \t${oldremoteref:-$merge_head}\n-exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \\\n+exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args $merge_args \\\n \t\"$merge_name\" HEAD $merge_head $verbosity\ndiff --git a/t/t6034-merge-ours-theirs.sh b/t/t6034-merge-ours-theirs.sh\nindex 08c9f79..8ab3d61 100755\n--- a/t/t6034-merge-ours-theirs.sh\n+++ b/t/t6034-merge-ours-theirs.sh\n@@ -53,4 +53,12 @@ test_expect_success 'recursive favouring ours' '\n \t! grep 1 file\n '\n \n+test_expect_success 'pull with -X' '\n+\tgit reset --hard master && git pull -s recursive -Xours . side &&\n+\tgit reset --hard master && git pull -s recursive -X ours . side &&\n+\tgit reset --hard master && git pull -s recursive -Xtheirs . side &&\n+\tgit reset --hard master && git pull -s recursive -X theirs . side &&\n+\tgit reset --hard master && ! git pull -s recursive -X bork . side\n+'\n+\n test_done\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128424","messageId":"c78d4c177f470e0f2f64314321df12e1a59077bf.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"1ff0b2f7e3fae4cc6c7610c92711f33df9a3d07c.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 6/8] Make \"subtree\" part more orthogonal to the rest of merge-recursive.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:58Z","receivedAt":"2009-11-26T02:23:58Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"This makes \"subtree\" more orthogonal to the rest of recursive merge, so\nthat you can use subtree and ours/theirs features at the same time.  For\nexample, you can now say:\n\n\tgit merge -s subtree -Xtheirs other\n\nto merge with \"other\" branch while shifting it up or down to match the\nshape of the tree of the current branch, and resolving conflicts favoring\nthe changes \"other\" branch made over changes made in the current branch.\n\nIt also allows the prefix used to shift the trees to be specified using\nthe \"-Xsubtree=$prefix\" option.  Giving an empty prefix tells the command\nto figure out how much to shift trees automatically as we have always\ndone.  \"merge -s subtree\" is the same as \"merge -s recursive -Xsubtree=\"\n(or \"merge -s recursive -Xsubtree\").\n\n(Patch originally by Junio Hamano <gitster@pobox.com>.)\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n builtin-merge-recursive.c |    6 ++-\n builtin-merge.c           |    6 ++-\n cache.h                   |    1 +\n match-trees.c             |   69 ++++++++++++++++++++++++++++++++++++++++++++-\n merge-recursive.c         |   16 +++++++---\n merge-recursive.h         |    3 +-\n 6 files changed, 90 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 53f8f05..d9404e1 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -28,7 +28,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tif (argv[0]) {\n \t\tint namelen = strlen(argv[0]);\n \t\tif (!suffixcmp(argv[0], \"-subtree\"))\n-\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\t\to.subtree_shift = \"\";\n \t}\n \n \tif (argc < 4)\n@@ -45,7 +45,9 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\t\telse if (!strcmp(arg+2, \"theirs\"))\n \t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n \t\t\telse if (!strcmp(arg+2, \"subtree\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\t\t\to.subtree_shift = \"\";\n+\t\t\telse if (!prefixcmp(arg+2, \"subtree=\"))\n+\t\t\t\to.subtree_shift = arg + 10;\n \t\t\telse\n \t\t\t\tdie(\"Unknown option %s\", arg);\n \t\t\tcontinue;\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 9a95bc8..a64b8f2 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -578,7 +578,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \n \t\tinit_merge_options(&o);\n \t\tif (!strcmp(strategy, \"subtree\"))\n-\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+\t\t\to.subtree_shift = \"\";\n \t\t\t\n \t\tfor (x = 0; x < xopts_nr; x++) {\n \t\t\tif (!strcmp(xopts[x], \"ours\"))\n@@ -586,7 +586,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\telse if (!strcmp(xopts[x], \"theirs\"))\n \t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n \t\t\telse if (!strcmp(xopts[x], \"subtree\"))\n-\t\t\t\to.recursive_variant = MERGE_RECURSIVE_SUBTREE;\n+                        \to.subtree_shift = \"\";\n+\t\t\telse if (!prefixcmp(xopts[x], \"subtree=\"))\n+\t\t\t\to.subtree_shift = xopts[x]+8;\n \t\t\telse\n \t\t\t\tdie(\"Unknown option for merge-recursive: -X%s\", xopts[x]);\n \t\t}\ndiff --git a/cache.h b/cache.h\nindex bf468e5..c6902d2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -993,6 +993,7 @@ extern int diff_auto_refresh_index;\n \n /* match-trees.c */\n void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);\n+void shift_tree_by(const unsigned char *, const unsigned char *, unsigned char *, const char *);\n \n /*\n  * whitespace rules.\ndiff --git a/match-trees.c b/match-trees.c\nindex 0fd6df7..26f7ed1 100644\n--- a/match-trees.c\n+++ b/match-trees.c\n@@ -185,7 +185,7 @@ static void match_trees(const unsigned char *hash1,\n  * tree object by replacing it with another tree \"hash2\".\n  */\n static int splice_tree(const unsigned char *hash1,\n-\t\t       char *prefix,\n+\t\t       const char *prefix,\n \t\t       const unsigned char *hash2,\n \t\t       unsigned char *result)\n {\n@@ -264,6 +264,13 @@ void shift_tree(const unsigned char *hash1,\n \tchar *del_prefix;\n \tint add_score, del_score;\n \n+\t/*\n+\t * NEEDSWORK: this limits the recursion depth to hardcoded\n+\t * value '2' to avoid excessive overhead.\n+\t */\n+\tif (!depth_limit)\n+\t\tdepth_limit = 2;\n+\n \tadd_score = del_score = score_trees(hash1, hash2);\n \tadd_prefix = xcalloc(1, 1);\n \tdel_prefix = xcalloc(1, 1);\n@@ -301,3 +308,63 @@ void shift_tree(const unsigned char *hash1,\n \n \tsplice_tree(hash1, add_prefix, hash2, shifted);\n }\n+\n+/*\n+ * The user says the trees will be shifted by this much.\n+ * Unfortunately we cannot fundamentally tell which one to\n+ * be prefixed, as recursive merge can work in either direction.\n+ */\n+void shift_tree_by(const unsigned char *hash1,\n+\t\t   const unsigned char *hash2,\n+\t\t   unsigned char *shifted,\n+\t\t   const char *shift_prefix)\n+{\n+\tunsigned char sub1[20], sub2[20];\n+\tunsigned mode1, mode2;\n+\tunsigned candidate = 0;\n+\n+\t/* Can hash2 be a tree at shift_prefix in tree hash1? */\n+\tif (!get_tree_entry(hash1, shift_prefix, sub1, &mode1) &&\n+\t    S_ISDIR(mode1))\n+\t\tcandidate |= 1;\n+\n+\t/* Can hash1 be a tree at shift_prefix in tree hash2? */\n+\tif (!get_tree_entry(hash2, shift_prefix, sub2, &mode2) &&\n+\t    S_ISDIR(mode2))\n+\t\tcandidate |= 2;\n+\n+\tif (candidate == 3) {\n+\t\t/* Both are plausible -- we need to evaluate the score */\n+\t\tint best_score = score_trees(hash1, hash2);\n+\t\tint score;\n+\n+\t\tcandidate = 0;\n+\t\tscore = score_trees(sub1, hash2);\n+\t\tif (score > best_score) {\n+\t\t\tcandidate = 1;\n+\t\t\tbest_score = score;\n+\t\t}\n+\t\tscore = score_trees(sub2, hash1);\n+\t\tif (score > best_score)\n+\t\t\tcandidate = 2;\n+\t}\n+\n+\tif (!candidate) {\n+\t\t/* Neither is plausible -- do not shift */\n+\t\thashcpy(shifted, hash2);\n+\t\treturn;\n+\t}\n+\n+\tif (candidate == 1)\n+\t\t/*\n+\t\t * shift tree2 down by adding shift_prefix above it\n+\t\t * to match tree1.\n+\t\t */\n+\t\tsplice_tree(hash1, shift_prefix, hash2, shifted);\n+\telse\n+\t\t/*\n+\t\t * shift tree2 up by removing shift_prefix from it\n+\t\t * to match tree1.\n+\t\t */\n+\t\thashcpy(shifted, sub2);\n+}\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 257bf8f..79b45ed 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -21,7 +21,8 @@\n #include \"merge-recursive.h\"\n #include \"dir.h\"\n \n-static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n+static struct tree *shift_tree_object(struct tree *one, struct tree *two,\n+\t\t\t\t      const char *subtree_shift)\n {\n \tunsigned char shifted[20];\n \n@@ -29,7 +30,12 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n \t * NEEDSWORK: this limits the recursion depth to hardcoded\n \t * value '2' to avoid excessive overhead.\n \t */\n-\tshift_tree(one->object.sha1, two->object.sha1, shifted, 2);\n+\tif (!*subtree_shift) {\n+\t\tshift_tree(one->object.sha1, two->object.sha1, shifted, 0);\n+\t} else {\n+\t\tshift_tree_by(one->object.sha1, two->object.sha1, shifted,\n+\t\t\t      subtree_shift);\n+\t}\n \tif (!hashcmp(two->object.sha1, shifted))\n \t\treturn two;\n \treturn lookup_tree(shifted);\n@@ -1213,9 +1219,9 @@ int merge_trees(struct merge_options *o,\n {\n \tint code, clean;\n \n-\tif (o->recursive_variant == MERGE_RECURSIVE_SUBTREE) {\n-\t\tmerge = shift_tree_object(head, merge);\n-\t\tcommon = shift_tree_object(head, common);\n+\tif (o->subtree_shift) {\n+\t\tmerge = shift_tree_object(head, merge, o->subtree_shift);\n+\t\tcommon = shift_tree_object(head, common, o->subtree_shift);\n \t}\n \n \tif (sha_eq(common->object.sha1, merge->object.sha1)) {\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 9d54219..d9347ce 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -7,10 +7,11 @@ struct merge_options {\n \tconst char *branch1;\n \tconst char *branch2;\n \tenum {\n-        \tMERGE_RECURSIVE_SUBTREE = 1,\n+\t\tMERGE_RECURSIVE_NORMAL = 0,\n         \tMERGE_RECURSIVE_OURS,\n         \tMERGE_RECURSIVE_THEIRS,\n \t} recursive_variant;\n+\tconst char *subtree_shift;\n \tunsigned buffer_output : 1;\n \tint verbosity;\n \tint diff_rename_limit;\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128426","messageId":"3acdc84af78453622df67b8c7ce6763bd316db4b.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"c78d4c177f470e0f2f64314321df12e1a59077bf.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 7/8] Extend merge-subtree tests to test -Xsubtree=dir.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:23:59Z","receivedAt":"2009-11-26T02:23:59Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"This tests the configurable -Xsubtree feature of merge-recursive.\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n t/t6029-merge-subtree.sh |   47 +++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 46 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh\nindex 5bbfa44..3900d9f 100755\n--- a/t/t6029-merge-subtree.sh\n+++ b/t/t6029-merge-subtree.sh\n@@ -52,6 +52,7 @@ test_expect_success 'initial merge' '\n \tgit merge -s ours --no-commit gui/master &&\n \tgit read-tree --prefix=git-gui/ -u gui/master &&\n \tgit commit -m \"Merge git-gui as our subdirectory\" &&\n+\tgit checkout -b work &&\n \tgit ls-files -s >actual &&\n \t(\n \t\techo \"100644 $o1 0\tgit-gui/git-gui.sh\"\n@@ -65,9 +66,10 @@ test_expect_success 'merge update' '\n \techo git-gui2 > git-gui.sh &&\n \to3=$(git hash-object git-gui.sh) &&\n \tgit add git-gui.sh &&\n+\tgit checkout -b master2 &&\n \tgit commit -m \"update git-gui\" &&\n \tcd ../git &&\n-\tgit pull -s subtree gui master &&\n+\tgit pull -s subtree gui master2 &&\n \tgit ls-files -s >actual &&\n \t(\n \t\techo \"100644 $o3 0\tgit-gui/git-gui.sh\"\n@@ -76,4 +78,47 @@ test_expect_success 'merge update' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'initial ambiguous subtree' '\n+\tcd ../git &&\n+\tgit reset --hard master &&\n+\tgit checkout -b master2 &&\n+\tgit merge -s ours --no-commit gui/master &&\n+\tgit read-tree --prefix=git-gui2/ -u gui/master &&\n+\tgit commit -m \"Merge git-gui2 as our subdirectory\" &&\n+\tgit checkout -b work2 &&\n+\tgit ls-files -s >actual &&\n+\t(\n+\t\techo \"100644 $o1 0\tgit-gui/git-gui.sh\"\n+\t\techo \"100644 $o1 0\tgit-gui2/git-gui.sh\"\n+\t\techo \"100644 $o2 0\tgit.c\"\n+\t) >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'merge using explicit' '\n+\tcd ../git &&\n+\tgit reset --hard master2 &&\n+\tgit pull -Xsubtree=git-gui gui master2 &&\n+\tgit ls-files -s >actual &&\n+\t(\n+\t\techo \"100644 $o3 0\tgit-gui/git-gui.sh\"\n+\t\techo \"100644 $o1 0\tgit-gui2/git-gui.sh\"\n+\t\techo \"100644 $o2 0\tgit.c\"\n+\t) >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'merge2 using explicit' '\n+\tcd ../git &&\n+\tgit reset --hard master2 &&\n+\tgit pull -Xsubtree=git-gui2 gui master2 &&\n+\tgit ls-files -s >actual &&\n+\t(\n+\t\techo \"100644 $o1 0\tgit-gui/git-gui.sh\"\n+\t\techo \"100644 $o3 0\tgit-gui2/git-gui.sh\"\n+\t\techo \"100644 $o2 0\tgit.c\"\n+\t) >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128430","messageId":"accf3a5caae4cd73dd6c46e0ddd46eb8f566ad1c.1259201377.git.apenwarr@gmail.com","threadId":"21758","inReplyTo":"3acdc84af78453622df67b8c7ce6763bd316db4b.1259201377.git.apenwarr@gmail.com","subject":"[PATCH 8/8] Document that merge strategies can now take their own options","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T02:24:00Z","receivedAt":"2009-11-26T02:24:00Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"Also document the recently added -Xtheirs, -Xours and -Xsubtree[=path]\noptions to the merge-recursive strategy.\n\n(Patch originally by Junio Hamano <gitster@pobox.com>.)\n\nSigned-off-by: Avery Pennarun <apenwarr@gmail.com>\n---\n Documentation/merge-options.txt    |    4 ++++\n Documentation/merge-strategies.txt |   29 ++++++++++++++++++++++++++++-\n 2 files changed, 32 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex fec3394..95244d2 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -74,3 +74,7 @@ option can be used to override --squash.\n -v::\n --verbose::\n \tBe verbose.\n+\n+-X<option>::\n+\tPass merge strategy specific option through to the merge\n+\tstrategy.\ndiff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt\nindex 42910a3..360dd6f 100644\n--- a/Documentation/merge-strategies.txt\n+++ b/Documentation/merge-strategies.txt\n@@ -1,6 +1,11 @@\n MERGE STRATEGIES\n ----------------\n \n+The merge mechanism ('git-merge' and 'git-pull' commands) allows the\n+backend 'merge strategies' to be chosen with `-s` option.  Some strategies\n+can also take their own options, which can be passed by giving `-X<option>`\n+arguments to 'git-merge' and/or 'git-pull'.\n+\n resolve::\n \tThis can only resolve two heads (i.e. the current branch\n \tand another branch you pulled from) using a 3-way merge\n@@ -20,6 +25,27 @@ recursive::\n \tAdditionally this can detect and handle merges involving\n \trenames.  This is the default merge strategy when\n \tpulling or merging one branch.\n++\n+The 'recursive' strategy can take the following options:\n+\n+ours;;\n+\tThis option forces conflicting hunks to be auto-resolved cleanly by\n+\tfavoring 'our' version.  Changes from the other tree that do not\n+\tconflict with our side are reflected to the merge result.\n++\n+This should not be confused with the 'ours' merge strategy, which does not\n+even look at what the other tree contains at all.  That one discards everything\n+the other tree did, declaring 'our' history contains all that happened in it.\n+\n+theirs;;\n+\tThis is opposite of 'ours'.\n+\n+subtree[=path];;\n+\tThis option is a more advanced form of 'subtree' strategy, where\n+\tthe strategy makes a guess on how two trees must be shifted to\n+\tmatch with each other when merging.  Instead, the specified path\n+\tis prefixed (or stripped from the beginning) to make the shape of\n+\ttwo trees to match.\n \n octopus::\n \tThis resolves cases with more than two heads, but refuses to do\n@@ -33,7 +59,8 @@ ours::\n \tmerge is always that of the current branch head, effectively\n \tignoring all changes from all other branches.  It is meant to\n \tbe used to supersede old development history of side\n-\tbranches.\n+\tbranches.  Note that this is different from the -Xours option to\n+\tthe 'recursive' merge strategy.\n \n subtree::\n \tThis is a modified recursive strategy. When merging trees A and\n-- \n1.6.6.rc0.62.gaccf\n"},{"id":"128432","messageId":"7vpr75hmpq.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"905749faf5ccb2c7c54d3318dbc662d69daf8d0e.1259201377.git.apenwarr@gmail.com","subject":"Re: [PATCH 2/8] builtin-merge.c: call exclude_cmds() correctly.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T05:36:01Z","receivedAt":"2009-11-26T05:36:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Avery Pennarun\" <apenwarr@gmail.com> writes:\n\n> We need to call exclude_cmds() after the loop, not during the loop, because\n> excluding a command from the array can change the indexes of objects in the\n> array.  The result is that, depending on file ordering, some commands\n> weren't excluded as they should have been.\n\nAs an independent bugfix, I would prefer this to be made against 'maint'\nand not as a part of this series.\n\nHow did you notice it?  Can you make a test case out of your experience of\nnoticing this bug in the first place, by the way (I am suspecting that you\nsaw some breakage and chased it in the debugger)?\n"},{"id":"128433","messageId":"7vr5rlerqf.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"7e1f1179fc5fe2f568e2c75f75366fa40d7bbbfb.1259201377.git.apenwarr@gmail.com","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T06:15:52Z","receivedAt":"2009-11-26T06:15:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Avery Pennarun\" <apenwarr@gmail.com> writes:\n\n> This uses the low-level mechanism for \"ours\" and \"theirs\" autoresolution\n> introduced by the previous commit to introduce two additional merge\n> strategies, merge-recursive-ours and merge-recursive-theirs.\n>\n> (Patch originally by Junio Hamano <gitster@pobox.com>.)\n>\n> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>\n\nTwo comments.\n\n - The original series was done over a few weeks in 'pu' and this\n   intermediate step was done before a better alternative of not using\n   these two extra merge strategies were discovered (\"...may have been an\n   easy way to experiment, but we should bite the bullet\", in the next\n   patch).\n\n   As the second round to seriously polish the series for inclusion, it\n   would make much more sense to squash this with the next patch to erase\n   this failed approach that has already been shown as clearly inferiour.\n\n - I think we should avoid adding the extra argument to ll_merge_fn() by\n   combining virtual_ancestor and favor into one \"flags\" parameter.  If\n   you do so, we do not have to change the callsites again next time we\n   need to add new optional features that needs only a few bits.\n\n   I vaguely recall that I did the counterpart of this patch that way\n   exactly for the above reason, but it is more than a year ago, so maybe\n   I didn't do it that way.\n"},{"id":"128434","messageId":"7vk4xderq0.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"73a42e99b4a083c74b017caf2970d1bbf5886b96.1259201377.git.apenwarr@gmail.com","subject":"Re: [PATCH 4/8] Teach git-merge to pass -X<option> to the backend strategy module","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T06:16:07Z","receivedAt":"2009-11-26T06:16:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun <apenwarr@gmail.com> writes:\n\n> Distinguishing slight variation of modes of operation between the vanilla\n> merge-recursive and merge-recursive-ours by the command name may have been\n> an easy way to experiment, but we should bite the bullet and allow backend\n> specific options to be given by the end user.\n>\n> (Patch originally by Junio Hamano <gitster@pobox.com>.)\n>\n> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>\n> ---\n>  Makefile                     |    3 ---\n>  builtin-merge-recursive.c    |   21 +++++++++++++++------\n>  builtin-merge.c              |   40 ++++++++++++++++++++++++++++++++++++----\n>  t/t6034-merge-ours-theirs.sh |    4 ++--\n>  4 files changed, 53 insertions(+), 15 deletions(-)\n\nYou added .gitignore entries for the two programs previously, and are\nremoving them in this patch, but forgot to remove them from .gitignore in\nthis patch.\n\nAs I already suggested you to, if you squash this to the previous one, it\nis not a big deal, though.\n\n> diff --git a/builtin-merge.c b/builtin-merge.c\n> index df089bb..9a95bc8 100644\n> --- a/builtin-merge.c\n> +++ b/builtin-merge.c\n> @@ -148,6 +148,17 @@ static int option_parse_strategy(const struct option *opt,\n>  \treturn 0;\n>  }\n>  \n> +static int option_parse_x(const struct option *opt,\n> +\t\t\t  const char *arg, int unset)\n> +{\n> +\tif (unset)\n> +\t\treturn 0;\n\nShould \"merge --no-extended\" silently be ignored, or be diagnosed as an\nerror?\n\n> @@ -174,6 +185,8 @@ static struct option builtin_merge_options[] = {\n>  \t\t\"abort if fast-forward is not possible\"),\n>  \tOPT_CALLBACK('s', \"strategy\", &use_strategies, \"strategy\",\n>  \t\t\"merge strategy to use\", option_parse_strategy),\n> +\tOPT_CALLBACK('X', \"extended\", &xopts, \"option=value\",\n> +\t\t\"option for selected merge strategy\", option_parse_x),\n\nI actually didn't name X for \"extended\" but more for \"external\" (to the\nmerge program proper).  \"--strategy-option\" perhaps?\n"},{"id":"128435","messageId":"7vd435erpc.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"1ff0b2f7e3fae4cc6c7610c92711f33df9a3d07c.1259201377.git.apenwarr@gmail.com","subject":"Re: [PATCH 5/8] Teach git-pull to pass -X<option> to git-merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T06:16:31Z","receivedAt":"2009-11-26T06:16:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun <apenwarr@gmail.com> writes:\n\n> (Patch originally by Junio Hamano <gitster@pobox.com>.)\n>\n> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>\n\nYou should take the full authorship of this patch without even mentioning\n\"originally by\".  It has no code from me.\n\n> @@ -216,7 +229,7 @@ fi\n>  \n>  merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n>  test true = \"$rebase\" &&\n> -\texec git-rebase $diffstat $strategy_args --onto $merge_head \\\n> +\texec git-rebase $diffstat $strategy_args $merge_args --onto $merge_head \\\n>  \t${oldremoteref:-$merge_head}\n> -exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \\\n> +exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args $merge_args \\\n>  \t\"$merge_name\" HEAD $merge_head $verbosity\n\nThis part needs the usual \"sq-then-eval\" trick; -X subtree=\"My Programs\"\non the command line will be split by the shell if you didn't do so.\n"},{"id":"128436","messageId":"7v638xeroc.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"c78d4c177f470e0f2f64314321df12e1a59077bf.1259201377.git.apenwarr@gmail.com","subject":"Re: [PATCH 6/8] Make \"subtree\" part more orthogonal to the rest of merge-recursive.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T06:17:07Z","receivedAt":"2009-11-26T06:17:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Avery Pennarun\" <apenwarr@gmail.com> writes:\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 257bf8f..79b45ed 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -21,7 +21,8 @@\n>  #include \"merge-recursive.h\"\n>  #include \"dir.h\"\n>  \n> -static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n> +static struct tree *shift_tree_object(struct tree *one, struct tree *two,\n> +\t\t\t\t      const char *subtree_shift)\n>  {\n>  \tunsigned char shifted[20];\n>  \n> @@ -29,7 +30,12 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n>  \t * NEEDSWORK: this limits the recursion depth to hardcoded\n>  \t * value '2' to avoid excessive overhead.\n>  \t */\n> -\tshift_tree(one->object.sha1, two->object.sha1, shifted, 2);\n\nThe block comment with NEEDSWORK should be removed from here.  I may have\nforgotten to in the original one, but that is not an excuse to replicate a\nbad job ;-)\n"},{"id":"128437","messageId":"7vy6ltdd2l.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"d243a513ffb8da4272f7a0e13a711f9b65195c25.1259201377.git.apenwarr@gmail.com","subject":"Re: [PATCH 1/8] git-merge-file --ours, --theirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T06:17:54Z","receivedAt":"2009-11-26T06:17:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Avery Pennarun\" <apenwarr@gmail.com> writes:\n\n> ...\n> A larger problem is that this tends to encourage a bad workflow by\n> allowing them to record such a mixed up half-merge result as a full commit\n> without auditing.  This commit does not tackle this latter issue.  In git,\n> we usually give long enough rope to users with strange wishes as long as\n> the risky features is not on by default.\n\nTypo/Grammo.  \"risky features are not on by default\".\n\n> (Patch originally by Junio Hamano <gitster@pobox.com>.)\n>\n> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>\n\nExcept for parse-optification, this one is more or less a verbatim copy of\nmy patch, and I think I probably deserve an in-body \"From: \" line for this\n[PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of\nthem.\n\n> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\n> index 4da052a..2cce49d 100644\n> --- a/xdiff/xdiff.h\n> +++ b/xdiff/xdiff.h\n> @@ -58,6 +58,11 @@ extern \"C\" {\n>  #define XDL_MERGE_ZEALOUS_ALNUM 3\n>  #define XDL_MERGE_LEVEL_MASK 0x0f\n>  \n> +/* merge favor modes */\n> +#define XDL_MERGE_FAVOR_OURS 0x0010\n> +#define XDL_MERGE_FAVOR_THEIRS 0x0020\n> +#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)\n\nThis is a bad change.  It forces the high-level layer of the resulting\ncode to be aware that the favor bits are shifted by 4 and it is different\nfrom what the low-level layer expects.  If I were porting it to\nparse-options, I would have kept OURS = 1 and THEIRS = 2 as the original\npatch, and instead did something like:\n\n \tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n-\t\t\t&xpp, merge_level | merge_style, &result);\n+\t\t\t&xpp, XDL_MERGE_FLAGS(merge_level, merge_style, merge_favor), &result);\n\nwith an updated definition like this:\n\n    #define XDL_MERGE_FLAGS(level, style, favor) ((level)|(style)|((favor)<<4)\n"},{"id":"128438","messageId":"20091126153726.6117@nanako3.lavabit.com","threadId":"21758","inReplyTo":"7vy6ltdd2l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/8] git-merge-file --ours, --theirs","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-11-26T06:37:26Z","receivedAt":"2009-11-26T06:37:26Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com>\n\n> Except for parse-optification, this one is more or less a verbatim copy of\n> my patch, and I think I probably deserve an in-body \"From: \" line for this\n> [PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of\n> them.\n\nCould you give an guideline to decide when to take authorship and when to\ngive it to others?  The above seems somewhat arbitrary to me.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"128439","messageId":"7vvdgxbwav.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"20091126153726.6117@nanako3.lavabit.com","subject":"Re: [PATCH 1/8] git-merge-file --ours, --theirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-26T07:05:28Z","receivedAt":"2009-11-26T07:05:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> Quoting Junio C Hamano <gitster@pobox.com>\n>\n>> Except for parse-optification, this one is more or less a verbatim copy of\n>> my patch, and I think I probably deserve an in-body \"From: \" line for this\n>> [PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of\n>> them.\n>\n> Could you give an guideline to decide when to take authorship and when to\n> give it to others?  The above seems somewhat arbitrary to me.\n\nIt might seem that way to you, as you do not write much C, but I am\nreasonably sure Avery understands after having worked on the series.\n\nImagine that Avery were an area expert (the subsystem maintainer) on \"git\nmerge\" and downwards, and somebody who did not know that \"merge\" has\nalready been rewritten in C, nor some parts of the system have been\nrewritten to use parse-options, submitted a patch series for review and\nAvery is helping to polish it up [*1*].\n\nAs the subsystem maintainer, Avery looks at the patches, updates parts of\nthe code that is based on obsolete infrastructure, adds lacking tests and\ndocumentation as necessary, and forwards the tested result upwards for\ninclusion.  How would the messages from Avery to me look?\n\nPatches that were majorly reworked should be attributed to Avery, and\nobviously new patches that are added for missing tests should be, too.\nFor example, changes made to git-merge.sh by the original submitter must\nbe discarded and redone from scratch to builtin-merge.c, and if you look\nat the changes, it would be quite obvious that the original patch wouldn't\nhave served as anything more than giving the specification.\n\nBut the ones with minor updates should retain the original authorship.\nIt unfortunately is not black-and-white, though.\n\nIn any case, where does Avery's credit go?  Is there a point in helping to\npolish others' patches?\n\nIt is recoded on the Signed-off-by line.  When somebody passes a patch\nfrom somebody else, an S-o-b is added for DCO purposes, but it also leaves\nthe \"patch trail\"---these people looked at the patch, spent effort to make\nsure it is suitable for inclusion by reviewing, polishing, and enhancing.\nA subsystem maintainer, or anybody who helps to polish others\ncontribution, may not necessarily have his name as the \"author\" of the\npatch, and if the patch forwarding is done via e-mail, his name won't be\non the \"committer\" line either.  But the contribution is still noted and\nappreciated, and the hint to pay attention to is by counting non-author\nS-o-b and Tested-by lines in the commit messages.\n\ncf. http://lwn.net/SubscriberLink/363456/50efdecf49af77ba/ check the last\ntable.\n\n\n[Footnote]\n\n*1* That somebody happens to be me from 18 months ago, but the important\npoint here is that the person is not Avery as the subsystem maintainer (in\nother words, it is immaterial that it happens to be the same person as the\ntoplevel maintainer).\n"},{"id":"128441","messageId":"20091126163018.6117@nanako3.lavabit.com","threadId":"21758","inReplyTo":"7vvdgxbwav.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/8] git-merge-file --ours, --theirs","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-11-26T07:30:18Z","receivedAt":"2009-11-26T07:30:18Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com> writes:\n\n> In any case, where does Avery's credit go?  Is there a point in helping to\n> polish others' patches?\n>\n> It is recoded on the Signed-off-by line.  When somebody passes a patch\n> from somebody else, an S-o-b is added for DCO purposes, but it also leaves\n> the \"patch trail\"---these people looked at the patch, spent effort to make\n> sure it is suitable for inclusion by reviewing, polishing, and enhancing.\n> A subsystem maintainer, or anybody who helps to polish others\n> contribution, may not necessarily have his name as the \"author\" of the\n> patch, and if the patch forwarding is done via e-mail, his name won't be\n> on the \"committer\" line either.  But the contribution is still noted and\n> appreciated, and the hint to pay attention to is by counting non-author\n> S-o-b and Tested-by lines in the commit messages.\n\nI understand. Thank you for a detailed explanation. \n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"128507","messageId":"32541b130911261355y2900b0cbtbf081c93c8fb10d6@mail.gmail.com","threadId":"21758","inReplyTo":"7vy6ltdd2l.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/8] git-merge-file --ours, --theirs","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T21:55:29Z","receivedAt":"2009-11-26T21:55:29Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Thu, Nov 26, 2009 at 1:17 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Except for parse-optification, this one is more or less a verbatim copy of\n> my patch, and I think I probably deserve an in-body \"From: \" line for this\n> [PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of\n> them.\n[...]\n> Imagine that Avery were an area expert (the subsystem maintainer) on \"git\n> merge\" and downwards, and somebody who did not know that \"merge\" has\n> already been rewritten in C, nor some parts of the system have been\n> rewritten to use parse-options, submitted a patch series for review and\n> Avery is helping to polish it up [*1*].\n\nI'm quite open to doing this however you want; I definitely consider\nit your patch series.  My main measurable contribution is just the\nunit tests that I wrote.\n\nHowever, when thinking about this, I wasn't worried so much about the\ncorrect placement of credit as of discredit.  The merge code has\nchanged sufficiently since you wrote this patch series that every one\nof them required quite a lot of conflict resolution.  Most of the\nconflicts were pretty obvious how to resolve, but it was tedious and\nerror prone, and there's a reasonably high probability that I screwed\nup something while doing so.\n\nI imagined what people would expect to see when they do 'git blame' to\nexplain the source of a problem.  If they see your name, you might be\nblamed for my errors; if they see my name with a \"based on a patch by\nJunio\" in the changelog, then I would be (probably correctly) blamed\nfor the errors, while you can retain credit for the success.\n\nMostly, however, I didn't want to be sending out patches in your name\nthat weren't actually done by you.  If you'd like me to do so,\nhowever, then I will :)\n\n>> +/* merge favor modes */\n>> +#define XDL_MERGE_FAVOR_OURS 0x0010\n>> +#define XDL_MERGE_FAVOR_THEIRS 0x0020\n>> +#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)\n>\n> This is a bad change.  It forces the high-level layer of the resulting\n> code to be aware that the favor bits are shifted by 4 and it is different\n> from what the low-level layer expects.  If I were porting it to\n> parse-options, I would have kept OURS = 1 and THEIRS = 2 as the original\n> patch, [...]\n\nOuch, yes, that wasn't very clear thinking on my part.  I meant for\nXDL_MERGE_FAVOR(flags) to return either XDL_MERGE_FAVOR_OURS or\nXDL_MERGE_FAVOR_THEIRS, but clearly it doesn't.  Will fix.\n\nAvery\n"},{"id":"128509","messageId":"32541b130911261400t6b1b439em6305c4e1bfe135f8@mail.gmail.com","threadId":"21758","inReplyTo":"7vpr75hmpq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/8] builtin-merge.c: call exclude_cmds() correctly.","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T22:00:59Z","receivedAt":"2009-11-26T22:00:59Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Thu, Nov 26, 2009 at 12:36 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Avery Pennarun\" <apenwarr@gmail.com> writes:\n>\n>> We need to call exclude_cmds() after the loop, not during the loop, because\n>> excluding a command from the array can change the indexes of objects in the\n>> array.  The result is that, depending on file ordering, some commands\n>> weren't excluded as they should have been.\n>\n> As an independent bugfix, I would prefer this to be made against 'maint'\n> and not as a part of this series.\n>\n> How did you notice it?  Can you make a test case out of your experience of\n> noticing this bug in the first place, by the way (I am suspecting that you\n> saw some breakage and chased it in the debugger)?\n\nThe story behind this one is a bit silly, but since you asked: I\nforgot to add recursive-ours and recursive-theirs to the list of known\nmerge strategies, but was surprised to find that my test for\nrecursive-theirs passed, while recursive-ours didn't.  Investigating\nfurther, I found that the printed list of \"found\" strategies included\nrecursive-theirs but not recursive-ours.  Turns out that this is\nbecause when recursive-ours was being (correctly) removed, that slot\nin the array was being filled by recursive-theirs, and then\nimmediately i++, which meant that recursive-theirs was never checked\nfor exclusion as it should have been.\n\nOf course, after fixing this bug *both* tests were broken, but the\ncorrect thing to do was add both strategies to the list, which hides\nthe effect of this bugfix.\n\nSince the bug is actually that *too many* strategies are listed\ninstead of too few, it's pretty minor and I doubt it needs to go into\nmaint.  Also, I don't know of a way to test it that would be reliable.\n And I doubt this particular bug will recur anyway.  (If it were too\n*few* strategies listed, I'm guessing it would be caught by any number\nof other tests.)\n\nSuggestions welcome.\n\nThanks,\n\nAvery\n"},{"id":"128511","messageId":"32541b130911261405q6564d8f2o30b7d7fd6f708d05@mail.gmail.com","threadId":"21758","inReplyTo":"7vr5rlerqf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-26T22:05:23Z","receivedAt":"2009-11-26T22:05:23Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Thu, Nov 26, 2009 at 1:15 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>  - The original series was done over a few weeks in 'pu' and this\n>   intermediate step was done before a better alternative of not using\n>   these two extra merge strategies were discovered (\"...may have been an\n>   easy way to experiment, but we should bite the bullet\", in the next\n>   patch).\n>\n>   As the second round to seriously polish the series for inclusion, it\n>   would make much more sense to squash this with the next patch to erase\n>   this failed approach that has already been shown as clearly inferiour.\n\nok.\n\n>  - I think we should avoid adding the extra argument to ll_merge_fn() by\n>   combining virtual_ancestor and favor into one \"flags\" parameter.  If\n>   you do so, we do not have to change the callsites again next time we\n>   need to add new optional features that needs only a few bits.\n>\n>   I vaguely recall that I did the counterpart of this patch that way\n>   exactly for the above reason, but it is more than a year ago, so maybe\n>   I didn't do it that way.\n\nYou did do that, in fact, but I had to redo a bunch of the flag stuff\nanyway since a few other flags had been added in the meantime.\n\nI actually tried it both ways (with and without an extra parameter),\nbut I observed that:\n\n- There are more lines of code (and more confusion) if you use an\nall-in-one flags vs. what I did.\n\n- Several functions have the same signature with all-in-one flags vs.\ntheir current boolean parameter, so the code would compile (and then\nsubtly not work) if I forgot to modify a particular function.\n\n- When we go to add a third flag parameter, it wouldn't be any harder\nto join them together at that time, and because it would *again*\nmodify the function signatures (from two flag params back down to\none), the compiler would *again* be able to catch any functions we\nforgot to adjust.\n\nIf you think this logic doesn't work, I can redo it with all-in-one\nflags as you request.\n\nAvery\n"},{"id":"128722","messageId":"7vvdgs1qip.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"32541b130911261405q6564d8f2o30b7d7fd6f708d05@mail.gmail.com","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T06:21:50Z","receivedAt":"2009-11-30T06:21:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun <apenwarr@gmail.com> writes:\n\n>>  - I think we should avoid adding the extra argument to ll_merge_fn() by\n>>   combining virtual_ancestor and favor into one \"flags\" parameter.  If\n>>   you do so, we do not have to change the callsites again next time we\n>>   need to add new optional features that needs only a few bits.\n>>\n>>   I vaguely recall that I did the counterpart of this patch that way\n>>   exactly for the above reason, but it is more than a year ago, so maybe\n>>   I didn't do it that way.\n>\n> You did do that, in fact,... <<rationale omitted>>\n\nThink of the \"flag\" parameter as a mini \"struct option\".  When you add a\nfeature to a function at or near the leaf level of call chains that are\npotentially deep, you add one element to the option structure, and take\nadvantage of the fact that existing callers put a sane default value in\nthe new field, i.e. 0, by doing a \"memset(&opt, 0, sizeof(opt))\" already,\nso that the callsites that do not even have to know about the new feature\nwill keep working the same old way without breakage.  You saw this exact\npattern in the [1/8] patch in your series to cram new \"favor this side\"\ninformation into an existing parameter.\n\nAs you mentioned, sometimes changing function signature is preferred to\ncatch semantic differences at compilation time.  The report given by the\ncompiler of extra or missing parameter at the call site is a wonderful way\nto find out that you forgot to convert them to the new semantics of the\nfunction.  This also helps when there is an in-flight patch that adds a\nnew callsite to the function whose semantics you are changing.  The\nsemantic conflict is caught when compiling the result of a merge with a\nbranch with such a patch.  It is a trick worth knowing about.\n\nThe approach however cuts both ways.  When you are adding an optional\nfeature that is used only in a very few call sites, the semantic merge\nconflict resulting from such a function signature change is rarely worth\nit.\n\nAs long as you choose the default \"no-op\" value carefully enough so that\nexisting callers will naturally use it without modification, the old code\nwill work the way they did before the new optional feature was added.  In\nother words, \"let's implement this as purely an opt-in feature\" is often\npreferrable over \"let's force semantic conflict and compilation failure,\njust in case existing callsites may also want to trigger this new\nfeature\".\n\nThat is why [1/8] patch in your series uses 0 to mean \"don't do the funny\n'favor' trick, but just leave the conflicts there\".\n\nI've queued the series with minor fixes to 'pu' and pushed it out.\n"},{"id":"128793","messageId":"32541b130911301008v4156f0c6ge9f30952565392f9@mail.gmail.com","threadId":"21758","inReplyTo":"7vvdgs1qip.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-30T18:08:58Z","receivedAt":"2009-11-30T18:08:58Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Mon, Nov 30, 2009 at 1:21 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> As long as you choose the default \"no-op\" value carefully enough so that\n> existing callers will naturally use it without modification, the old code\n> will work the way they did before the new optional feature was added.  In\n> other words, \"let's implement this as purely an opt-in feature\" is often\n> preferrable over \"let's force semantic conflict and compilation failure,\n> just in case existing callsites may also want to trigger this new\n> feature\".\n>\n> That is why [1/8] patch in your series uses 0 to mean \"don't do the funny\n> 'favor' trick, but just leave the conflicts there\".\n\nThere's just one bit to add to this: when converting a non-bitfield\nint into a bitfield, really odd things can happen.  That was my main\nrationale for avoiding the change to bitfield without changing the\nsignature.  That said, the amount of code isn't really that big so\nthis point doesn't matter much.\n\n> I've queued the series with minor fixes to 'pu' and pushed it out.\n\nSince I see you didn't change a couple of things you mentioned in\nearlier comments (the NEEDSWORK comment and the sq-then-eval trick) do\nyou still want me to respin this series?\n\nThanks,\n\nAvery\n"},{"id":"128801","messageId":"7viqcrlrb8.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"32541b130911301008v4156f0c6ge9f30952565392f9@mail.gmail.com","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T19:56:43Z","receivedAt":"2009-11-30T19:56:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Avery Pennarun <apenwarr@gmail.com> writes:\n\n>> I've queued the series with minor fixes to 'pu' and pushed it out.\n>\n> Since I see you didn't change a couple of things you mentioned in\n> earlier comments (the NEEDSWORK comment and the sq-then-eval trick) do\n> you still want me to respin this series?\n\nThe commit still is NEEDSWORK and shouldn't be in 'next' in its current\nshape.  I don't think the topic is 1.6.6 material yet, and we will be in\npre-release feature freeze any minute now, so there is no urgency.\n\nAs I did the sq-then-eval in many places in our Porcelain scripts (and\nmany of them are converted to C and lost the need for the trick), I may\nget tempted to fix it up when I am bored ;-).  But no promises.\n\nThanks.\n"},{"id":"128802","messageId":"7v1vjflr3v.fsf@alter.siamese.dyndns.org","threadId":"21758","inReplyTo":"7viqcrlrb8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T20:01:08Z","receivedAt":"2009-11-30T20:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Avery Pennarun <apenwarr@gmail.com> writes:\n>\n>>> I've queued the series with minor fixes to 'pu' and pushed it out.\n>>\n>> Since I see you didn't change a couple of things you mentioned in\n>> earlier comments (the NEEDSWORK comment and the sq-then-eval trick) do\n>> you still want me to respin this series?\n>\n> The commit still is NEEDSWORK and shouldn't be in 'next' in its current\n> shape.\n\nOh, I think you meant the \"NEEDSWORK -- we limit to depth 2 when we\nguess\" and that has been with us ever since we added subtree merge, and it\nis no reason to block the topic.  I had the sq-then-eval stuff in mind\nwhen I wrote above.\n\nSorry for the confusion.\n"},{"id":"128803","messageId":"32541b130911301202j2b551d80v650d252b7934eb98@mail.gmail.com","threadId":"21758","inReplyTo":"7viqcrlrb8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/8] git-merge-recursive-{ours,theirs}","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2009-11-30T20:02:51Z","receivedAt":"2009-11-30T20:02:51Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Mon, Nov 30, 2009 at 2:56 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Avery Pennarun <apenwarr@gmail.com> writes:\n>>> I've queued the series with minor fixes to 'pu' and pushed it out.\n>>\n>> Since I see you didn't change a couple of things you mentioned in\n>> earlier comments (the NEEDSWORK comment and the sq-then-eval trick) do\n>> you still want me to respin this series?\n>\n> The commit still is NEEDSWORK and shouldn't be in 'next' in its current\n> shape.  I don't think the topic is 1.6.6 material yet, and we will be in\n> pre-release feature freeze any minute now, so there is no urgency.\n>\n> As I did the sq-then-eval in many places in our Porcelain scripts (and\n> many of them are converted to C and lost the need for the trick), I may\n> get tempted to fix it up when I am bored ;-).  But no promises.\n\nI'll interpret that as \"no, I should not respin the series because\nJunio plans to deal with it\" :)\n\nDo let me know if there's anything I should do to help this advance\nfrom pu->next sooner (if they delay is not simply because of the code\nfreeze).\n\nHave fun,\n\nAvery\n"}]}