{"thread":{"id":"22935","subject":"[PATCH v4 0/7] add --ff option to cherry-pick","startedAt":"2010-03-06T20:34:39Z","lastAt":"2010-03-07T20:39:45Z","messageCount":12,"participants":["Christian Couder","Junio C Hamano","Paolo Bonzini"],"isPatch":true,"patchVersion":4,"patchTotal":7},"messages":[{"id":"136259","messageId":"20100306203250.2960.30309.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":null,"subject":"[PATCH v4 0/7] add --ff option to cherry-pick","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:39Z","receivedAt":"2010-03-06T20:34:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The goal of this patch series is to make it possible for \"git cherry-pick\"\nto fast forward instead of creating a new commit if the cherry picked commit\nhas the same parent as the one we are cherry-picking on.\n\nThe big change since previous version is that the first patches to implement\nthe --ff option are based on what Junio suggested.\n\nChristian Couder (4):\n cherry-pick: add tests for new --ff option\n Documentation: describe new cherry-pick --ff option\n cherry-pick: add a no-op --no-ff option to future proof scripts\n rebase -i: use new --ff cherry-pick option\n\nJunio C Hamano (3):\n parse-options: add parse_options_concat() to concat options\n builtin/merge: make checkout_fast_forward() non static\n revert: add --ff option to allow fast forward when cherry-picking\n\n Documentation/git-cherry-pick.txt | 10 +++-\n builtin/merge.c | 2 +-\n builtin/revert.c | 47 +++++++++++++++-\n cache.h | 3 +\n git-rebase--interactive.sh | 15 +----\n parse-options.c | 15 +++++\n parse-options.h | 1 +\n t/t3506-cherry-pick-ff.sh | 106 +++++++++++++++++++++++++++++++++++++\n 8 files changed, 182 insertions(+), 17 deletions(-)\n create mode 100755 t/t3506-cherry-pick-ff.sh\n"},{"id":"136265","messageId":"20100306203447.2960.38015.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 1/7] parse-options: add parse_options_concat() to concat options","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:40Z","receivedAt":"2010-03-06T20:34:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano \n\n\nSigned-off-by: Christian Couder \n---\n parse-options.c | 15 +++++++++++++++\n parse-options.h | 1 +\n 2 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex c83035d..8546d85 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -659,3 +659,18 @@ int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)\n \t*target = unset ? 2 : 1;\n \treturn 0;\n }\n+\n+int parse_options_concat(struct option *dst, size_t dst_size, struct option *src)\n+{\n+\tint i, j;\n+\n+\tfor (i = 0; i < dst_size; i++)\n+\t\tif (dst[i].type == OPTION_END)\n+\t\t\tbreak;\n+\tfor (j = 0; i < dst_size; i++, j++) {\n+\t\tdst[i] = src[j];\n+\t\tif (src[j].type == OPTION_END)\n+\t\t\treturn 0;\n+\t}\n+\treturn -1;\n+}\ndiff --git a/parse-options.h b/parse-options.h\nindex 9429f7e..7581e93 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -187,6 +187,7 @@ extern int parse_options_step(struct parse_opt_ctx_t *ctx,\n \n extern int parse_options_end(struct parse_opt_ctx_t *ctx);\n \n+extern int parse_options_concat(struct option *dst, size_t, struct option *src);\n \n /*----- some often used options -----*/\n extern int parse_opt_abbrev_cb(const struct option *, const char *, int);\n-- \n1.7.0.1.307.g861f4\n"},{"id":"136263","messageId":"20100306203447.2960.60379.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 2/7] builtin/merge: make checkout_fast_forward() non static","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:41Z","receivedAt":"2010-03-06T20:34:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano \n\nand also export it in \"cache.h\".\n\nSigned-off-by: Christian Couder \n---\n builtin/merge.c | 2 +-\n cache.h | 3 +++\n 2 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 3aaec7b..c043066 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -667,7 +667,7 @@ static int count_unmerged_entries(void)\n \treturn ret;\n }\n \n-static int checkout_fast_forward(unsigned char *head, unsigned char *remote)\n+int checkout_fast_forward(const unsigned char *head, const unsigned char *remote)\n {\n \tstruct tree *trees[MAX_UNPACK_TREES];\n \tstruct unpack_trees_options opts;\ndiff --git a/cache.h b/cache.h\nindex 89f6a40..0608b55 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1054,4 +1054,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix);\n char *alias_lookup(const char *alias);\n int split_cmdline(char *cmdline, const char ***argv);\n \n+/* builtin/merge.c */\n+int checkout_fast_forward(const unsigned char *from, const unsigned char *to);\n+\n #endif /* CACHE_H */\n-- \n1.7.0.1.307.g861f4\n"},{"id":"136261","messageId":"20100306203447.2960.42019.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:42Z","receivedAt":"2010-03-06T20:34:42Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Junio C Hamano \n\nAs \"git merge\" fast forwards if possible, it seems sensible to\nhave such a feature for \"git cherry-pick\" too, especially as it\ncould be used in git-rebase--interactive.sh.\n\nMaybe this option could be made the default in the long run, with\nanother --no-ff option to disable this default behavior, but that\ncould make some scripts backward incompatible and/or that would\nrequire testing if some GIT_AUTHOR_* environment variables are\nset. So we don't do that for now.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 43 ++++++++++++++++++++++++++++++++++++++++---\n 1 files changed, 40 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex eff5268..476f41e 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -13,6 +13,7 @@\n #include \"revision.h\"\n #include \"rerere.h\"\n #include \"merge-recursive.h\"\n+#include \"refs.h\"\n \n /*\n * This implements the builtins revert and cherry-pick.\n@@ -35,7 +36,7 @@ static const char * const cherry_pick_usage[] = {\n \tNULL\n };\n \n-static int edit, no_replay, no_commit, mainline, signoff;\n+static int edit, no_replay, no_commit, mainline, signoff, allow_ff;\n static enum { REVERT, CHERRY_PICK } action;\n static struct commit *commit;\n static const char *commit_name;\n@@ -60,8 +61,19 @@ 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_END(),\n+\t\tOPT_END(),\n+\t\tOPT_END(),\n \t};\n \n+\tif (action == CHERRY_PICK) {\n+\t\tstruct option cp_extra[] = {\n+\t\t\tOPT_BOOLEAN(0, \"ff\", &allow_ff, \"allow fast-forward\"),\n+\t\t\tOPT_END(),\n+\t\t};\n+\t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\n+\t\t\tdie(\"program error\");\n+\t}\n+\n \tif (parse_options(argc, argv, NULL, options, usage_str, 0) != 1)\n \t\tusage_with_options(usage_str, options);\n \n@@ -244,6 +256,17 @@ static NORETURN void die_dirty_index(const char *me)\n \t}\n }\n \n+static int fast_forward_to(const unsigned char *to, const unsigned char *from)\n+{\n+\tstruct ref_lock *ref_lock;\n+\n+\tread_cache();\n+\tif (checkout_fast_forward(from, to))\n+\t\texit(1); /* the callee should have complained already */\n+\tref_lock = lock_any_ref_for_update(\"HEAD\", from, 0);\n+\treturn write_ref_sha1(ref_lock, to, \"cherry-pick\");\n+}\n+\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n@@ -265,6 +288,17 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \tif (action == REVERT && !no_replay)\n \t\tdie(\"revert is incompatible with replay\");\n \n+\tif (allow_ff) {\n+\t\tif (signoff)\n+\t\t\tdie(\"cherry-pick --ff cannot be used with --signoff\");\n+\t\tif (no_commit)\n+\t\t\tdie(\"cherry-pick --ff cannot be used with --no-commit\");\n+\t\tif (no_replay)\n+\t\t\tdie(\"cherry-pick --ff cannot be used with -x\");\n+\t\tif (edit)\n+\t\t\tdie(\"cherry-pick --ff cannot be used with --edit\");\n+\t}\n+\n \tif (read_cache() < 0)\n \t\tdie(\"git %s: failed to read the index\", me);\n \tif (no_commit) {\n@@ -284,8 +318,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t}\n \tdiscard_cache();\n \n-\tindex_fd = hold_locked_index(&index_lock, 1);\n-\n \tif (!commit->parents) {\n \t\tif (action == REVERT)\n \t\t\tdie (\"Cannot revert a root commit\");\n@@ -314,6 +346,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \telse\n \t\tparent = commit->parents->item;\n \n+\tif (allow_ff && !hashcmp(parent->object.sha1, head))\n+\t\treturn fast_forward_to(commit->object.sha1, head);\n+\n \tif (!(message = commit->buffer))\n \t\tdie (\"Cannot get commit message for %s\",\n \t\t\t\tsha1_to_hex(commit->object.sha1));\n@@ -343,6 +378,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \n \toneline = get_oneline(message);\n \n+\tindex_fd = hold_locked_index(&index_lock, 1);\n+\n \tif (action == REVERT) {\n \t\tchar *oneline_body = strchr(oneline, ' ');\n \n-- \n1.7.0.1.307.g861f4\n"},{"id":"136260","messageId":"20100306203447.2960.31556.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 4/7] cherry-pick: add tests for new --ff option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:43Z","receivedAt":"2010-03-06T20:34:43Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n t/t3506-cherry-pick-ff.sh | 98 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 98 insertions(+), 0 deletions(-)\n create mode 100755 t/t3506-cherry-pick-ff.sh\n\ndiff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\nnew file mode 100755\nindex 0000000..e17ae71\n--- /dev/null\n+++ b/t/t3506-cherry-pick-ff.sh\n@@ -0,0 +1,98 @@\n+#!/bin/sh\n+\n+test_description='test cherry-picking with --ff option'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo first > file1 &&\n+\tgit add file1 &&\n+\ttest_tick &&\n+\tgit commit -m \"first\" &&\n+\tgit tag first &&\n+\n+\tgit checkout -b other &&\n+\techo second >> file1 &&\n+\tgit add file1 &&\n+\ttest_tick &&\n+\tgit commit -m \"second\" &&\n+\tgit tag second\n+'\n+\n+test_expect_success 'cherry-pick using --ff fast forwards' '\n+\tgit checkout master &&\n+\tgit reset --hard first &&\n+\ttest_tick &&\n+\tgit cherry-pick --ff second &&\n+\ttest \"$(git rev-parse --verify HEAD)\" = \"$(git rev-parse --verify second)\"\n+'\n+\n+test_expect_success 'cherry-pick not using --ff does not fast forwards' '\n+\tgit checkout master &&\n+\tgit reset --hard first &&\n+\ttest_tick &&\n+\tgit cherry-pick second &&\n+\ttest \"$(git rev-parse --verify HEAD)\" != \"$(git rev-parse --verify second)\"\n+'\n+\n+#\n+# We setup the following graph:\n+#\n+#\t B---C\n+#\t / /\n+#\tfirst---A\n+#\n+# (This has been taken from t3502-cherry-pick-merge.sh)\n+#\n+test_expect_success 'merge setup' '\n+\tgit checkout master &&\n+\tgit reset --hard first &&\n+\techo new line >A &&\n+\tgit add A &&\n+\ttest_tick &&\n+\tgit commit -m \"add line to A\" A &&\n+\tgit tag A &&\n+\tgit checkout -b side first &&\n+\techo new line >B &&\n+\tgit add B &&\n+\ttest_tick &&\n+\tgit commit -m \"add line to B\" B &&\n+\tgit tag B &&\n+\tgit checkout master &&\n+\tgit merge side &&\n+\tgit tag C &&\n+\tgit checkout -b new A\n+'\n+\n+test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '\n+\tgit reset --hard A -- &&\n+\ttest_must_fail git cherry-pick --ff -m 1 B &&\n+\tgit diff --exit-code A --\n+'\n+\n+test_expect_success 'cherry pick a merge with --ff but without -m should fail' '\n+\tgit reset --hard A -- &&\n+\ttest_must_fail git cherry-pick --ff C &&\n+\tgit diff --exit-code A --\n+'\n+\n+test_expect_success 'cherry pick with --ff a merge (1)' '\n+\tgit reset --hard A -- &&\n+\tgit cherry-pick --ff -m 1 C &&\n+\tgit diff --exit-code C &&\n+\ttest \"$(git rev-parse --verify HEAD)\" = \"$(git rev-parse --verify C)\"\n+'\n+\n+test_expect_success 'cherry pick with --ff a merge (2)' '\n+\tgit reset --hard B -- &&\n+\tgit cherry-pick --ff -m 2 C &&\n+\tgit diff --exit-code C &&\n+\ttest \"$(git rev-parse --verify HEAD)\" = \"$(git rev-parse --verify C)\"\n+'\n+\n+test_expect_success 'cherry pick a merge relative to nonexistent parent with --ff should fail' '\n+\tgit reset --hard B -- &&\n+\ttest_must_fail git cherry-pick --ff -m 3 C\n+'\n+\n+test_done\n-- \n1.7.0.1.307.g861f4\n"},{"id":"136266","messageId":"20100306203447.2960.15351.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 5/7] Documentation: describe new cherry-pick --ff option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:44Z","receivedAt":"2010-03-06T20:34:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n Documentation/git-cherry-pick.txt | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 78f4714..d71607a 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit\n \n SYNOPSIS\n --------\n-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] \n+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] \n \n DESCRIPTION\n -----------\n@@ -70,6 +70,10 @@ effect to your index in a row.\n --signoff::\n \tAdd Signed-off-by line at the end of the commit message.\n \n+--ff::\n+\tIf the current HEAD is the same as the parent of the\n+\tcherry-pick'ed commit, then a fast forward to this commit will\n+\tbe performed.\n \n Author\n ------\n-- \n1.7.0.1.307.g861f4\n"},{"id":"136264","messageId":"20100306203447.2960.5046.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 6/7] cherry-pick: add a no-op --no-ff option to future proof scripts","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:45Z","receivedAt":"2010-03-06T20:34:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"A --ff option to allow \"git cherry-pick\" to fast forward if\npossible was added in a previous patch, and this behavior may\nbecome the default one in the future.\n\nSo to future proof scripts that may rely on the current behavior\nit is safer to add a --no-ff option to make sure that the current\nbehavior will be used.\n\nRequested-by: Paolo Bonzini \nSigned-off-by: Christian Couder \n---\n Documentation/git-cherry-pick.txt | 6 +++++-\n builtin/revert.c | 6 +++++-\n t/t3506-cherry-pick-ff.sh | 8 ++++++++\n 3 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex d71607a..dfc8243 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit\n \n SYNOPSIS\n --------\n-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] \n+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--[no-]ff] \n \n DESCRIPTION\n -----------\n@@ -75,6 +75,10 @@ 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+--no-ff::\n+\tDoes nothing right now, but in the future this may disallow\n+\tfast forward if it becomes the default behavior.\n+\n Author\n ------\n Written by Junio C Hamano \ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 476f41e..0651e34 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -36,7 +36,7 @@ static const char * const cherry_pick_usage[] = {\n \tNULL\n };\n \n-static int edit, no_replay, no_commit, mainline, signoff, allow_ff;\n+static int edit, no_replay, no_commit, mainline, signoff, allow_ff, no_ff;\n static enum { REVERT, CHERRY_PICK } action;\n static struct commit *commit;\n static const char *commit_name;\n@@ -63,11 +63,13 @@ static void parse_args(int argc, const char **argv)\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n+\t\tOPT_END(),\n \t};\n \n \tif (action == CHERRY_PICK) {\n \t\tstruct option cp_extra[] = {\n \t\t\tOPT_BOOLEAN(0, \"ff\", &allow_ff, \"allow fast-forward\"),\n+\t\t\tOPT_BOOLEAN(0, \"no-ff\", &no_ff, \"disallow fast forward\"),\n \t\t\tOPT_END(),\n \t\t};\n \t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\n@@ -297,6 +299,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\t\tdie(\"cherry-pick --ff cannot be used with -x\");\n \t\tif (edit)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --edit\");\n+\t\tif (no_ff)\n+\t\t\tdie(\"cherry-pick --ff cannot be used with --no-ff\");\n \t}\n \n \tif (read_cache() < 0)\ndiff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\nindex e17ae71..2d7e532 100755\n--- a/t/t3506-cherry-pick-ff.sh\n+++ b/t/t3506-cherry-pick-ff.sh\n@@ -35,6 +35,14 @@ test_expect_success 'cherry-pick not using --ff does not fast forwards' '\n \ttest \"$(git rev-parse --verify HEAD)\" != \"$(git rev-parse --verify second)\"\n '\n \n+test_expect_success 'cherry-pick using --no-ff does not fast forwards' '\n+\tgit checkout master &&\n+\tgit reset --hard first &&\n+\ttest_tick &&\n+\tgit cherry-pick --no-ff second &&\n+\ttest \"$(git rev-parse --verify HEAD)\" != \"$(git rev-parse --verify second)\"\n+'\n+\n #\n # We setup the following graph:\n #\n-- \n1.7.0.1.307.g861f4\n"},{"id":"136262","messageId":"20100306203447.2960.47050.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"20100306203250.2960.30309.chriscool@tuxfamily.org","subject":"[PATCH v4 7/7] rebase -i: use new --ff cherry-pick option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-06T20:34:46Z","receivedAt":"2010-03-06T20:34:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This simplifies rebase -i a little bit.\n\nSigned-off-by: Christian Couder \n---\n git-rebase--interactive.sh | 15 +++------------\n 1 files changed, 3 insertions(+), 12 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 3e4fd14..92d19f5 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -222,8 +222,8 @@ do_with_author () {\n }\n \n pick_one () {\n-\tno_ff=\n-\tcase \"$1\" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac\n+\tff=--ff\n+\tcase \"$1\" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac\n \toutput git rev-parse --verify $sha1 || die \"Invalid commit name: $sha1\"\n \ttest -d \"$REWRITTEN\" &&\n \t\tpick_one_preserving_merges \"$@\" && return\n@@ -232,16 +232,7 @@ pick_one () {\n \t\toutput git cherry-pick \"$@\"\n \t\treturn\n \tfi\n-\tparent_sha1=$(git rev-parse --verify $sha1^) ||\n-\t\tdie \"Could not get the parent of $sha1\"\n-\tcurrent_sha1=$(git rev-parse --verify HEAD)\n-\tif test -z \"$no_ff\" && test \"$current_sha1\" = \"$parent_sha1\"\n-\tthen\n-\t\toutput git reset --hard $sha1\n-\t\toutput warn Fast-forward to $(git rev-parse --short $sha1)\n-\telse\n-\t\toutput git cherry-pick \"$@\"\n-\tfi\n+\toutput git cherry-pick $ff \"$@\"\n }\n \n pick_one_preserving_merges () {\n-- \n1.7.0.1.307.g861f4\n"},{"id":"136268","messageId":"7v6358dbv8.fsf@alter.siamese.dyndns.org","threadId":"22935","inReplyTo":"20100306203447.2960.42019.chriscool@tuxfamily.org","subject":"Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-07T03:55:23Z","receivedAt":"2010-03-07T03:55:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> From: Junio C Hamano \n>\n> As \"git merge\" fast forwards if possible, it seems sensible to\n> have such a feature for \"git cherry-pick\" too, especially as it\n> could be used in git-rebase--interactive.sh.\n>\n> Maybe this option could be made the default in the long run,\n\nI never said that.\n\nFor scripted use, like we can see in your [PATCH 7/7], \"cherry-pick --ff\"\ncould be a good ingredient (but even then the calling Porcelain script may\nalready know that what is being picked is a direct descendent (rebase -i\ncertainly should, as the last commit it processed from the todo file is at\nthe HEAD), but for manual use case it just feels silly to assume that the\nend user is so stupid that s/he doesn't know the commit being picked might\nbe a direct descendant, which would be the only reason why making --ff the\ndefault might make sense.\n\nAnd adding --no-ff and sprinkling that to scripts, only to support that\ndefault change as a future possibility, feels doubly silly.\n\nWhat was the real motive/use case of \"cherry-pick --ff\"?\n"},{"id":"136296","messageId":"4B936500.9020005@gnu.org","threadId":"22935","inReplyTo":"7v6358dbv8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2010-03-07T08:34:08Z","receivedAt":"2010-03-07T08:34:08Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 03/07/2010 04:55 AM, Junio C Hamano wrote:\n> What was the real motive/use case of \"cherry-pick --ff\"?\n\nAvoiding pointless separation of histories. It's true that nowadays \ngit-patch-id will make a good job of reconciling them, but they do look \nugly in gitk.\n\nPaolo\n"},{"id":"136298","messageId":"7vy6i4zem7.fsf@alter.siamese.dyndns.org","threadId":"22935","inReplyTo":"4B936500.9020005@gnu.org","subject":"Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-07T09:05:04Z","receivedAt":"2010-03-07T09:05:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paolo Bonzini writes:\n\n> On 03/07/2010 04:55 AM, Junio C Hamano wrote:\n>> What was the real motive/use case of \"cherry-pick --ff\"?\n>\n> Avoiding pointless separation of histories. It's true that nowadays\n> git-patch-id will make a good job of reconciling them, but they do\n> look ugly in gitk.\n\nSorry, but that is a not-very-useful XY answer.\n\nWhy were _you_ using \"cherry-pick\" on a commit that is an immediate child\nof the current commit? What were you trying to achieve by blindly using\ncherry-pick even in such a case?\n\nI am sort-of guessing that \"blindly\" is coming from a script that doesn't\nbother to check if the commit you are cherry-picking is a direct child,\nand I do not think it is such a bad thing to allow scripts to blindly call\ncherry-pick that way and at the same time avoid making cherry-picked\ncommits that are unnecessary. So in that sense I am not opposed to having\nan option to allow \"--ff\".\n\nBut if that is the real motivation, then making --ff default is a wrong\nthing to do, as existing scripts knew and trusted cherry-pick will _not_\nfast-forward, and the ones that do want fast-forward have already checked\njust like \"rebase -i\" does. Changing the default like Chriscool suggested\nwould not help anybody.\n\nThat is what I wanted to find out.\n"},{"id":"136353","messageId":"201003072139.45942.chriscool@tuxfamily.org","threadId":"22935","inReplyTo":"7vy6i4zem7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-03-07T20:39:45Z","receivedAt":"2010-03-07T20:39:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sunday 07 March 2010 10:05:04 Junio C Hamano wrote:\n> Paolo Bonzini writes:\n> > On 03/07/2010 04:55 AM, Junio C Hamano wrote:\n> >> What was the real motive/use case of \"cherry-pick --ff\"?\n> >\n> > Avoiding pointless separation of histories. It's true that nowadays\n> > git-patch-id will make a good job of reconciling them, but they do\n> > look ugly in gitk.\n> \n> Sorry, but that is a not-very-useful XY answer.\n> \n> Why were _you_ using \"cherry-pick\" on a commit that is an immediate child\n> of the current commit? What were you trying to achieve by blindly using\n> cherry-pick even in such a case?\n> \n> I am sort-of guessing that \"blindly\" is coming from a script that doesn't\n> bother to check if the commit you are cherry-picking is a direct child,\n> and I do not think it is such a bad thing to allow scripts to blindly call\n> cherry-pick that way and at the same time avoid making cherry-picked\n> commits that are unnecessary. So in that sense I am not opposed to having\n> an option to allow \"--ff\".\n> \n> But if that is the real motivation, then making --ff default is a wrong\n> thing to do, as existing scripts knew and trusted cherry-pick will _not_\n> fast-forward, and the ones that do want fast-forward have already checked\n> just like \"rebase -i\" does. Changing the default like Chriscool suggested\n> would not help anybody.\n\nThe wording is: \"Maybe this option could be made the default in the long run, \n...\"\n\nI didn't wrote something like: \"This option should be made the default in the \nlong run, ...\"\n\nI really didn't know if it should or should not be made the default now or in \nthe long run, but I thought I had to talk about it.\n\nWhy? Because Paolo said that he would like it to become the default, and also \nbecause if we implement \"git cherry-pick A..B\" and people start to use it a \nlot, they may find it boring to have to always add --ff to avoid creating lots \nof spurious commit (instead of reusing existing ones).\n\nNow if you want me to remove this part of the commit message, I will do it.\nBut my opinion is that it is interesting to document that we thought about \nmaking it the default now or in the future.\n\nDo you prefer something like:\n\n\"Making this option the default was discussed on the mailing list but we \ndecided that it is best left as non default for now. Though we recognized that \nit is best to have a --no-ff option too to future proof scripts in case the \ndecision to make fast forwarding the default is made in the future.\"\n\n?\n\nBest regards,\nChristian.\n"}]}