{"thread":{"id":"26022","subject":"[RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","startedAt":"2010-12-11T00:51:44Z","lastAt":"2010-12-28T19:37:00Z","messageCount":8,"participants":["Jonathan Nieder","Junio C Hamano","Martin von Zweigbergk","Justin Frankel"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"157859","messageId":"20101211005144.GA6634@burratino","threadId":"26022","inReplyTo":null,"subject":"[RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-11T00:51:44Z","receivedAt":"2010-12-11T00:51:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"For example, this would allow cherry-picking or reverting patches from\na piece of history with a different end-of-line style, like so:\n\n\t$ git revert -Xrenormalize old-problematic-commit\n\nCurrently that is possible with manual use of merge-recursive but the\ncherry-pick/revert porcelain does not expose the functionality.\n\nWhile at it, document the existing support for --strategy.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThoughts?\n\n Documentation/git-cherry-pick.txt  |   32 ++++++++++++++++++++++++++++++++\n Documentation/git-revert.txt       |   10 ++++++++++\n builtin/merge.c                    |    6 ++++--\n builtin/revert.c                   |   29 ++++++++++++++++++++++-------\n contrib/examples/git-revert.sh     |   13 ++++++++++++-\n merge-recursive.h                  |    4 +++-\n t/t3032-merge-recursive-options.sh |   14 ++++++++++++++\n 7 files changed, 97 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 7300870..749d68a 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -79,6 +79,16 @@ effect to your index in a row.\n \tcherry-pick'ed commit, then a fast forward to this commit will\n \tbe performed.\n \n+--strategy=<strategy>::\n+\tUse the given merge strategy.  Should only be used once.\n+\tSee the MERGE STRATEGIES section in linkgit:git-merge[1]\n+\tfor details.\n+\n+-X<option>::\n+--strategy-option=<option>::\n+\tPass the merge strategy-specific option through to the\n+\tmerge strategy.  See linkgit:git-merge[1] for details.\n+\n EXAMPLES\n --------\n git cherry-pick master::\n@@ -120,6 +130,28 @@ git rev-list --reverse master \\-- README | git cherry-pick -n --stdin::\n \tso the result can be inspected and made into a single new\n \tcommit if suitable.\n \n+The following sequence attempts to backport a patch, bails out because\n+the code the patch applies to has changed too much, and then tries\n+again, this time exercising more care about matching up context lines.\n+\n+------------\n+$ git cherry-pick topic^             <1>\n+$ git diff                           <2>\n+$ git reset --merge ORIG_HEAD        <3>\n+$ git cherry-pick -Xpatience topic^  <4>\n+------------\n+<1> apply the change that would be shown by `git show topic^`.\n+In this example, the patch does not apply cleanly, so\n+information about the conflict is written to the index and\n+working tree and no new commit results.\n+<2> summarize changes to be reconciled\n+<3> cancel the cherry-pick.  In other words, return to the\n+pre-cherry-pick state, preserving any local modifications you had in\n+the working tree.\n+<4> try to apply the change introduced by `topic^` again,\n+spending extra time to avoid mistakes based on incorrectly matching\n+context lines.\n+\n Author\n ------\n Written by Junio C Hamano <gitster@pobox.com>\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 752fc88..45be851 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -80,6 +80,16 @@ effect to your index in a row.\n --signoff::\n \tAdd Signed-off-by line at the end of the commit message.\n \n+--strategy=<strategy>::\n+\tUse the given merge strategy.  Should only be used once.\n+\tSee the MERGE STRATEGIES section in linkgit:git-merge[1]\n+\tfor details.\n+\n+-X<option>::\n+--strategy-option=<option>::\n+\tPass the merge strategy-specific option through to the\n+\tmerge strategy.  See linkgit:git-merge[1] for details.\n+\n EXAMPLES\n --------\n git revert HEAD~3::\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 3921cd3..c57d06e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -582,7 +582,8 @@ static void write_tree_trivial(unsigned char *sha1)\n \t\tdie(\"git write-tree failed to write a tree\");\n }\n \n-int try_merge_command(const char *strategy, struct commit_list *common,\n+int try_merge_command(const char *strategy, size_t xopts_nr,\n+\t\t      const char **xopts, struct commit_list *common,\n \t\t      const char *head_arg, struct commit_list *remotes)\n {\n \tconst char **args;\n@@ -680,7 +681,8 @@ 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\treturn try_merge_command(strategy, common, head_arg, remoteheads);\n+\t\treturn try_merge_command(strategy, xopts_nr, xopts,\n+\t\t\t\t\t\tcommon, head_arg, remoteheads);\n \t}\n }\n \ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex bb6e9e8..dc1b702 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -44,7 +44,11 @@ static const char **commit_argv;\n static int allow_rerere_auto;\n \n static const char *me;\n+\n+/* Merge strategy. */\n static const char *strategy;\n+static const char **xopts;\n+static size_t xopts_nr, xopts_alloc;\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n@@ -55,6 +59,17 @@ static const char * const *revert_or_cherry_pick_usage(void)\n \treturn action == REVERT ? revert_usage : cherry_pick_usage;\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+\n+\tALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);\n+\txopts[xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n static void parse_args(int argc, const char **argv)\n {\n \tconst char * const * usage_str = revert_or_cherry_pick_usage();\n@@ -67,6 +82,8 @@ static void parse_args(int argc, const char **argv)\n \t\tOPT_INTEGER('m', \"mainline\", &mainline, \"parent number\"),\n \t\tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n \t\tOPT_STRING(0, \"strategy\", &strategy, \"strategy\", \"merge strategy\"),\n+\t\tOPT_CALLBACK('X', \"strategy-option\", &xopts, \"option\",\n+\t\t\t\"option for merge strategy\", option_parse_x),\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n@@ -311,18 +328,13 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \tstruct merge_options o;\n \tstruct tree *result, *next_tree, *base_tree, *head_tree;\n \tint clean, index_fd;\n+\tconst char **xopt;\n \tstatic struct lock_file index_lock;\n \n \tindex_fd = hold_locked_index(&index_lock, 1);\n \n \tread_cache();\n \n-\t/*\n-\t * NEEDSWORK: cherry-picking between branches with\n-\t * different end-of-line normalization is a pain;\n-\t * plumb in an option to set o.renormalize?\n-\t * (or better: arbitrary -X options)\n-\t */\n \tinit_merge_options(&o);\n \to.ancestor = base ? base_label : \"(empty tree)\";\n \to.branch1 = \"HEAD\";\n@@ -332,6 +344,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \tnext_tree = next ? next->tree : empty_tree();\n \tbase_tree = base ? base->tree : empty_tree();\n \n+\tfor (xopt = xopts; xopt != xopts + xopts_nr; xopt++)\n+\t\tparse_merge_opt(&o, *xopt);\n+\n \tclean = merge_trees(&o,\n \t\t\t    head_tree,\n \t\t\t    next_tree, base_tree, &result);\n@@ -503,7 +518,7 @@ static int do_pick_commit(void)\n \n \t\tcommit_list_insert(base, &common);\n \t\tcommit_list_insert(next, &remotes);\n-\t\tres = try_merge_command(strategy, common,\n+\t\tres = try_merge_command(strategy, xopts_nr, xopts, common,\n \t\t\t\t\tsha1_to_hex(head), remotes);\n \t\tfree_commit_list(common);\n \t\tfree_commit_list(remotes);\ndiff --git a/contrib/examples/git-revert.sh b/contrib/examples/git-revert.sh\nindex 60a05a8..6bf155c 100755\n--- a/contrib/examples/git-revert.sh\n+++ b/contrib/examples/git-revert.sh\n@@ -26,6 +26,7 @@ require_work_tree\n cd_to_toplevel\n \n no_commit=\n+xopt=\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n@@ -44,6 +45,16 @@ do\n \t-x|--i-really-want-to-expose-my-private-commit-object-name)\n \t\treplay=\n \t\t;;\n+\t-X?*)\n+\t\txopt=\"$xopt$(git rev-parse --sq-quote \"--${1#-X}\")\"\n+\t\t;;\n+\t--strategy-option=*)\n+\t\txopt=\"$xopt$(git rev-parse --sq-quote \"--${1#--strategy-option=}\")\"\n+\t\t;;\n+\t-X|--strategy-option)\n+\t\tshift\n+\t\txopt=\"$xopt$(git rev-parse --sq-quote \"--$1\")\"\n+\t\t;;\n \t-*)\n \t\tusage\n \t\t;;\n@@ -159,7 +170,7 @@ export GITHEAD_$head GITHEAD_$next\n # and $prev on top of us (when reverting), or the change between\n # $prev and $commit on top of us (when cherry-picking or replaying).\n \n-git-merge-recursive $base -- $head $next &&\n+eval \"git merge-recursive $xopt $base -- $head $next\" &&\n result=$(git-write-tree 2>/dev/null) || {\n \tmv -f .msg \"$GIT_DIR/MERGE_MSG\"\n \t{\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex c8135b0..981ed6a 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -57,6 +57,8 @@ struct tree *write_tree_from_memory(struct merge_options *o);\n int parse_merge_opt(struct merge_options *out, const char *s);\n \n /* builtin/merge.c */\n-int try_merge_command(const char *strategy, struct commit_list *common, const char *head_arg, struct commit_list *remotes);\n+int try_merge_command(const char *strategy, size_t xopts_nr,\n+\t\tconst char **xopts, struct commit_list *common,\n+\t\tconst char *head_arg, struct commit_list *remotes);\n \n #endif\ndiff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-options.sh\nindex 2293797..796f616 100755\n--- a/t/t3032-merge-recursive-options.sh\n+++ b/t/t3032-merge-recursive-options.sh\n@@ -107,6 +107,20 @@ test_expect_success '--ignore-space-change makes merge succeed' '\n \tgit merge-recursive --ignore-space-change HEAD^ -- HEAD remote\n '\n \n+test_expect_success 'naive cherry-pick fails' '\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git cherry-pick --no-commit remote &&\n+\tgit read-tree --reset -u HEAD &&\n+\ttest_must_fail git cherry-pick remote &&\n+\ttest_must_fail git update-index --refresh &&\n+\tgrep \"<<<<<<\" text.txt\n+'\n+\n+test_expect_success '-Xignore-space-change makes cherry-pick succeed' '\n+\tgit read-tree --reset -u HEAD &&\n+\tgit cherry-pick --no-commit -Xignore-space-change remote\n+'\n+\n test_expect_success '--ignore-space-change: our w/s-only change wins' '\n \tq_to_cr <<-\\EOF >expected &&\n \t    justice and holiness and is the nurse of his age and theQ\n-- \n1.7.2.4\n"},{"id":"157862","messageId":"7vvd319h50.fsf@alter.siamese.dyndns.org","threadId":"26022","inReplyTo":"20101211005144.GA6634@burratino","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-11T02:29:15Z","receivedAt":"2010-12-11T02:29:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> For example, this would allow cherry-picking or reverting patches from\n> a piece of history with a different end-of-line style, like so:\n>\n> \t$ git revert -Xrenormalize old-problematic-commit\n>\n> Currently that is possible with manual use of merge-recursive but the\n> cherry-pick/revert porcelain does not expose the functionality.\n>\n> While at it, document the existing support for --strategy.\n>\n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> ---\n> Thoughts?\n\nI guess this can also take \"ignore whitespace\", which might be a better\noption for this particular use case?\n"},{"id":"157863","messageId":"20101211023537.GA12161@burratino","threadId":"26022","inReplyTo":"7vvd319h50.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-11T02:35:37Z","receivedAt":"2010-12-11T02:35:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> \t$ git revert -Xrenormalize old-problematic-commit\n[...]\n> I guess this can also take \"ignore whitespace\", which might be a better\n> option for this particular use case?\n\nSuppose in olden days you checked in files with \\r\\n line endings and\nnow you have switched to \\n (attribute \"crlf\" or \"text\"), and in\nbetween there was a day in which the line endings were switched.\n\nNow you notice that old-problematic-commit is broken.  If that commit\nremoved lines (which had \\r\\n line endings), then even with\n-Xignore-space-at-eol, \"git revert\" will add them back verbatim.  By\ncontrast, \"git revert -Xrenormalize\" would add them back in such a way\nas to follow the current line ending style.\n"},{"id":"158624","messageId":"alpine.DEB.1.10.1012271630560.788@debian","threadId":"26022","inReplyTo":"20101227212515.GA32352@burratino","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-12-27T15:38:43Z","receivedAt":"2010-12-27T15:38:43Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, 27 Dec 2010, Jonathan Nieder wrote:\n\n> Jonathan Nieder wrote:\n> \n> > For example, this would allow cherry-picking or reverting patches from\n> > a piece of history with a different end-of-line style, like so:\n> > \n> > \t$ git revert -Xrenormalize old-problematic-commit\n> > \n> > Currently that is possible with manual use of merge-recursive but the\n> > cherry-pick/revert porcelain does not expose the functionality.\n> > \n> > While at it, document the existing support for --strategy.\n> > \n> > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> > ---\n> > Thoughts?\n> \n> Ping?  I use this with -Xpatience fairly often.  Am I the only one who\n> has wanted such a thing?\n\nFWIW, I have wanted it for the end-of-line scenario you describe\nabove.\n\n/Martin\n"},{"id":"158622","messageId":"20101227212515.GA32352@burratino","threadId":"26022","inReplyTo":"20101211005144.GA6634@burratino","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-27T21:25:15Z","receivedAt":"2010-12-27T21:25:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> For example, this would allow cherry-picking or reverting patches from\n> a piece of history with a different end-of-line style, like so:\n> \n> \t$ git revert -Xrenormalize old-problematic-commit\n> \n> Currently that is possible with manual use of merge-recursive but the\n> cherry-pick/revert porcelain does not expose the functionality.\n> \n> While at it, document the existing support for --strategy.\n> \n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> ---\n> Thoughts?\n\nPing?  I use this with -Xpatience fairly often.  Am I the only one who\nhas wanted such a thing?\n"},{"id":"158626","messageId":"4D190E0E.7080000@cockos.com","threadId":"26022","inReplyTo":"20101227212515.GA32352@burratino","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Justin Frankel","fromEmail":"justin@cockos.com","sentAt":"2010-12-27T22:07:10Z","receivedAt":"2010-12-27T22:07:10Z","isPatch":true,"sender":{"key":"justin@cockos.com","avatar":"https://avatars.githubusercontent.com/u/5803984?v=4"},"body":"I think this is a good thing -- all commands that use an underlying \nmerge should support these options.\n\nWould it make sense at some level to allow some other means of cleanly \npassing this information around? Perhaps an environment variable for \nmerge options?\n\n\nJonathan Nieder wrote:\n> Jonathan Nieder wrote:\n> \n>> For example, this would allow cherry-picking or reverting patches from\n>> a piece of history with a different end-of-line style, like so:\n>>\n>> \t$ git revert -Xrenormalize old-problematic-commit\n>>\n>> Currently that is possible with manual use of merge-recursive but the\n>> cherry-pick/revert porcelain does not expose the functionality.\n>>\n>> While at it, document the existing support for --strategy.\n>>\n>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n>> ---\n>> Thoughts?\n> \n> Ping?  I use this with -Xpatience fairly often.  Am I the only one who\n> has wanted such a thing?\n"},{"id":"158676","messageId":"7v7hetyeom.fsf@alter.siamese.dyndns.org","threadId":"26022","inReplyTo":"20101227212515.GA32352@burratino","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-12-28T17:45:13Z","receivedAt":"2010-12-28T17:45:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Jonathan Nieder wrote:\n>\n>> For example, this would allow cherry-picking or reverting patches from\n>> a piece of history with a different end-of-line style, like so:\n>> \n>> \t$ git revert -Xrenormalize old-problematic-commit\n>> \n>> Currently that is possible with manual use of merge-recursive but the\n>> cherry-pick/revert porcelain does not expose the functionality.\n>> \n>> While at it, document the existing support for --strategy.\n>> \n>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n>> ---\n>> Thoughts?\n>\n> Ping?  I use this with -Xpatience fairly often.  Am I the only one who\n> has wanted such a thing?\n\nPong, if you are pinging me.  I thought you wanted to only discuss it\nfirst with RFC and meant to follow up on a real-for-inclusion patch.\n\nThe goal stated in the proposed commit log is a good thing to have, and\nexactly the kind of thing -X<strategy option> interface was invented for\nin the first place, I think.\n\nI didn't look at the code though.\n"},{"id":"158685","messageId":"20101228193700.GA4303@burratino","threadId":"26022","inReplyTo":"7v7hetyeom.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] cherry-pick/revert: add support for -X/--strategy-option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-28T19:37:00Z","receivedAt":"2010-12-28T19:37:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Pong, if you are pinging me.  I thought you wanted to only discuss it\n> first with RFC and meant to follow up on a real-for-inclusion patch.\n\nYou thought correctly.  In this case I can't think of any change this\nneeds on top to make this real for inclusion so I just bumped the\nthread.\n\nThere were two hints at improvements in this thread (separate topic):\nthe \"git merge\" manual could use some discussion of the distinction\nbetween --renormalize and --ignore-whitespace-at-eol, and am, rebase,\nstash, and \"checkout -m <tree>\" ought to have -X plumbed, too.\n"}]}