{"thread":{"id":"50785","subject":"[RFC PATCH] cherry-pick: set default `--mainline` parent to 1","startedAt":"2019-03-20T03:55:03Z","lastAt":"2019-03-25T15:42:13Z","messageCount":23,"participants":["C.J. Jameson","Junio C Hamano","Duy Nguyen","Sergey Organov","Elijah Newren","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"371949","messageId":"CALm+SVJR5BCHi_r7B279gKDukD4mYDQuv=K5guje73YDVmOxug@mail.gmail.com","threadId":"50785","inReplyTo":null,"subject":"[RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"C.J. Jameson","fromEmail":"cjcjameson@gmail.com","sentAt":"2019-03-20T03:54:48Z","receivedAt":"2019-03-20T03:55:03Z","isPatch":true,"sender":{"key":"cjcjameson@gmail.com","avatar":"https://gravatar.com/avatar/2d485726dfe7eeb8bfc3de4527e0dbc8ce36d880cd4930f2c0b4321da8257c37?d=mp&s=160"},"body":"From fbb0e1fef08a2abd7f1620134925788f7a0e011f Mon Sep 17 00:00:00 2001\nFrom: \"C.J. Jameson\" <cjcjameson@gmail.com>\nDate: Tue, 19 Mar 2019 17:57:31 -0700\nSubject: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1\n\nWhen cherry-picking from a standard merge commit, the parent should\nalways be 1, as it has the meaningful new changes which were added to\nthe mainline. If the source commit is unique in some way and a user\nwants to specify a value other than 1, they still can, but it's up to\nthem to be aware of which parent is which.\n\nThis builds on commit 37897bfc27 (\"cherry-pick: do not error on\nnon-merge commits when '-m 1' is specified\", 2018-12-14) which tolerated\n`--mainline` for non-merge commits\n\nSigned-off-by: C.J. Jameson <cjcjameson@gmail.com>\n---\n Documentation/git-cherry-pick.txt |  6 +++---\n Documentation/git-revert.txt      |  6 +++---\n builtin/revert.c                  |  4 ++--\n t/t3502-cherry-pick-merge.sh      | 19 ++++++++++++++++++-\n 4 files changed, 26 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt\nb/Documentation/git-cherry-pick.txt\nindex b8cfeec67e..ec526d4cb8 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -76,13 +76,13 @@ OPTIONS\n  described above, and `-r` was to disable it.  Now the\n  default is not to do `-x` so this option is a no-op.\n\n--m parent-number::\n---mainline parent-number::\n+-m [parent-number]::\n+--mainline [parent-number]::\n  Usually you cannot cherry-pick a merge because you do not know which\n  side of the merge should be considered the mainline.  This\n  option specifies the parent number (starting from 1) of\n  the mainline and allows cherry-pick to replay the change\n- relative to the specified parent.\n+ relative to the specified parent. The default parent-number is 1.\n\n -n::\n --no-commit::\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex 837707a8fd..57192a7c34 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -46,13 +46,13 @@ OPTIONS\n  message prior to committing the revert. This is the default if\n  you run the command from a terminal.\n\n--m parent-number::\n---mainline parent-number::\n+-m [parent-number]::\n+--mainline [parent-number]::\n  Usually you cannot revert a merge because you do not know which\n  side of the merge should be considered the mainline.  This\n  option specifies the parent number (starting from 1) of\n  the mainline and allows revert to reverse the change\n- relative to the specified parent.\n+ relative to the specified parent. The default parent-number is 1.\n +\n Reverting a merge commit declares that you will never want the tree changes\n brought in by the merge.  As a result, later merges will only bring in tree\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex a47b53ceaf..280d1f00a9 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -105,8 +105,8 @@ static int run_sequencer(int argc, const char\n**argv, struct replay_opts *opts)\n  OPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n  OPT_NOOP_NOARG('r', NULL),\n  OPT_BOOL('s', \"signoff\", &opts->signoff, N_(\"add Signed-off-by:\")),\n- OPT_CALLBACK('m', \"mainline\", opts, N_(\"parent-number\"),\n-      N_(\"select mainline parent\"), option_parse_m),\n+ { OPTION_INTEGER, 'm', \"mainline\", &opts->mainline, N_(\"parent-number\"),\n+ N_(\"select mainline parent\"), PARSE_OPT_OPTARG, NULL, 1 },\n  OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),\n  OPT_STRING(0, \"strategy\", &opts->strategy, N_(\"strategy\"), N_(\"merge\nstrategy\")),\n  OPT_CALLBACK('X', \"strategy-option\", &opts, N_(\"option\"),\ndiff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh\nindex 8b635a196d..4f248be582 100755\n--- a/t/t3502-cherry-pick-merge.sh\n+++ b/t/t3502-cherry-pick-merge.sh\n@@ -34,7 +34,6 @@ test_expect_success setup '\n test_expect_success 'cherry-pick -m complains of bogus numbers' '\n  # expect 129 here to distinguish between cases where\n  # there was nothing to cherry-pick\n- test_expect_code 129 git cherry-pick -m &&\n  test_expect_code 129 git cherry-pick -m foo b &&\n  test_expect_code 129 git cherry-pick -m -1 b &&\n  test_expect_code 129 git cherry-pick -m 0 b\n@@ -67,6 +66,15 @@ test_expect_success 'cherry pick a merge (1)' '\n\n '\n\n+test_expect_success 'cherry pick a merge with default parent (1)' '\n+\n+ git reset --hard &&\n+ git checkout a^0 &&\n+ git cherry-pick -m c &&\n+ git diff --exit-code c\n+\n+'\n+\n test_expect_success 'cherry pick a merge (2)' '\n\n  git reset --hard &&\n@@ -111,6 +119,15 @@ test_expect_success 'revert a merge (1)' '\n\n '\n\n+test_expect_success 'revert a merge with default parent (1)' '\n+\n+ git reset --hard &&\n+ git checkout c^0 &&\n+ git revert -m 1 c &&\n+ git diff --exit-code a --\n+\n+'\n+\n test_expect_success 'revert a merge (2)' '\n\n  git reset --hard &&\n-- \n2.21.0\n"},{"id":"371950","messageId":"xmqq1s32w3vu.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"CALm+SVJR5BCHi_r7B279gKDukD4mYDQuv=K5guje73YDVmOxug@mail.gmail.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-20T04:45:09Z","receivedAt":"2019-03-20T04:45:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"C.J. Jameson\" <cjcjameson@gmail.com> writes:\n\n> When cherry-picking from a standard merge commit, the parent should\n> always be 1, as it has the meaningful new changes which were added to\n> the mainline. If the source commit is unique in some way and a user\n> wants to specify a value other than 1, they still can, but it's up to\n> them to be aware of which parent is which.\n\nI do not have a firm opinion for or against the \"if the commit being\ncherry-picked happens to be a merge, cherry-pick it relative to the\nfirst parent by default, without requiring the '-m 1' option\".  It\nmay not be such a bad idea after all.\n\nBut I do have a very strong opinion against adding yet another\noption that takes an optional argument.  If we want to allow\ncherry-picking a merge commit just as easy as cherrry-picking a\nsingle-parent commit, \"git cherry-pick -m merge\" (assuming 'merge'\nis the tip of a branch that is a merge commit) that still requires\nthe user to say \"-m\" is not a good improvement.  We should just\naccept \"git cherry-pick merge\" without any \"-m\" if we want to move\nin this direction, I would think.\n\n> --m parent-number::\n> ---mainline parent-number::\n> +-m [parent-number]::\n> +--mainline [parent-number]::\n>   Usually you cannot cherry-pick a merge because you do not know which\n>   side of the merge should be considered the mainline.  This\n>   option specifies the parent number (starting from 1) of\n>   the mainline and allows cherry-pick to replay the change\n> - relative to the specified parent.\n> + relative to the specified parent. The default parent-number is 1.\n\nI somehow sense a total whitespace breakage.  Usually these lines\nare HT indented, but I see only a single whitespace indent around\nhere.\n\nSo, the first part of the paragraph needs to get revamped.  It won't\nbe \"because\", but \"unless\", and with your change, you no longer know\nwhich side is mainline---unless you tell Git otherwise, the first\nparent is assumed to be the mainline in the new world order.\n\n\t-m <parent-number>::\n\t--mainline <parent-number>::\n                When cherry-picking a merge, by default, the first\n                parent is taken as the mainline, and the command\n                replays the change relative to the first parent of\n                the given commit.  If the merge commit you are\n                cherry-picking records the mainline as second or\n                later parent, you can use this option to replay\n                the change relative to the specified parent (parents\n                count from 1, which is the first parent).\n\t+\n\tThe default parent-number is 1.  Giving a parent number\n\tlarger than the number of parent the cherry-picked commit has\n\tis an error.\n\nor something like that.\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index a47b53ceaf..280d1f00a9 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -105,8 +105,8 @@ static int run_sequencer(int argc, const char\n> **argv, struct replay_opts *opts)\n\nLine-wrapped patch cannot be applied.\n\n>   OPT_BOOL('e', \"edit\", &opts->edit, N_(\"edit the commit message\")),\n>   OPT_NOOP_NOARG('r', NULL),\n>   OPT_BOOL('s', \"signoff\", &opts->signoff, N_(\"add Signed-off-by:\")),\n> - OPT_CALLBACK('m', \"mainline\", opts, N_(\"parent-number\"),\n> -      N_(\"select mainline parent\"), option_parse_m),\n> + { OPTION_INTEGER, 'm', \"mainline\", &opts->mainline, N_(\"parent-number\"),\n> + N_(\"select mainline parent\"), PARSE_OPT_OPTARG, NULL, 1 },\n\nLet's not do optarg.  Use of OPT_INTEGER is fine, if you really want\nto, but then you are losing the last caller of option_parse_m() and\nthe callback function itself must also be removed. \n\nYou'd also need to do a few more things:\n\n#1 make it NONEG, so that we reject \"--no-mainline\"; in the new\n   world order, cherry-picking a non-merge commit is replaying\n   against the first parent; there is no situation where use of\n   --no-mainline makes sense (perhaps other than cherry-picking the\n   initial commit, for which there is no mainline parent).\n\n#2 initialize opts->mainline to 1, so that with no --mainline option\n   from the command line, we will still get opt->mainline == 1\n   (alternatively, you could initialize opt->mainline == -1 and\n   treat opt->mainline <= 0 as if it were set to 1 when the code\n   must choose against which parent to replay the changes in a much\n   deeper place in the codepath).\n\n#3 think about how you'd loosen option compatibility check around\n   --mainline (if you do #2, as opt->mainline would never be 0 now)\n   and adjust the verify_opt_compatible() call(s).\n\n#4 as OPT_INTEGER won't ensure that you get a positive integer,\n   you'd need to somehow reject --mainline=0 (or --mainline=-1 for\n   that matter) given from the command line.\n"},{"id":"371972","messageId":"CACsJy8Av87XHBX0EKS3W-yZ0yKxfa47d9NaAeUGmh5icGiTGYw@mail.gmail.com","threadId":"50785","inReplyTo":"CALm+SVJR5BCHi_r7B279gKDukD4mYDQuv=K5guje73YDVmOxug@mail.gmail.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-03-20T09:44:05Z","receivedAt":"2019-03-20T09:44:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 20, 2019 at 10:57 AM C.J. Jameson <cjcjameson@gmail.com> wrote:\n> --m parent-number::\n> ---mainline parent-number::\n> +-m [parent-number]::\n\nCareful with this. Optional argument with a short option means the\n\"stuck\" form which is known to cause confusion [1].\n\nTry \"git revert -mX HEAD\" and \"git revert -m X HEAD\". The first one\nreports invalid number (good). The second assumes \"X\" not to be an\nargument of \"-m\" and leave it for the caller (revert in this case) to\nhandle, which could do unexpected things.\n\nPersonally I'm not excited to have more stuck forms like this.\n\n[1] https://public-inbox.org/git/20190208024800.GA11392@sigill.intra.peff.net/\n\n\n> +--mainline [parent-number]::\n>   Usually you cannot cherry-pick a merge because you do not know which\n>   side of the merge should be considered the mainline.  This\n>   option specifies the parent number (starting from 1) of\n>   the mainline and allows cherry-pick to replay the change\n> - relative to the specified parent.\n> + relative to the specified parent. The default parent-number is 1.\n--\nDuy\n"},{"id":"372001","messageId":"871s31vjo7.fsf@javad.com","threadId":"50785","inReplyTo":"xmqq1s32w3vu.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-20T12:01:44Z","receivedAt":"2019-03-20T12:01:48Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n[...]\n\n> But I do have a very strong opinion against adding yet another\n> option that takes an optional argument.  If we want to allow\n> cherry-picking a merge commit just as easy as cherrry-picking a\n> single-parent commit, \"git cherry-pick -m merge\" (assuming 'merge'\n> is the tip of a branch that is a merge commit) that still requires\n> the user to say \"-m\" is not a good improvement.  We should just\n> accept \"git cherry-pick merge\" without any \"-m\" if we want to move\n> in this direction, I would think.\n\nLet's just make '-m 1' the default option indeed. No need for further\ncomplexities.\n\nExactly according to what Junio has already said before. Here:\n\nhttps://public-inbox.org/git/xmqqsh5gt9sm.fsf@gitster-ct.c.googlers.com\n\nJunio wrote:\n\n> Now, it appears, at least to me, that the world pretty much accepted\n> that the first-parent worldview is often very convenient and worth\n> supporting by the tool, so the next logical step might be to set\n> opts->mainline to 1 by default (and allow an explicit \"-m $n\" from\n> the command line to override it).  But that should happen after this\n> patch lands---it is logically a separate step, I would think.\n\n... and as that patch already landed...\n\n-- Sergey\n"},{"id":"372009","messageId":"CABPp-BEe56GFM_2g7EyXmSrULFwRAvSPBomQ66jEQmCs=HhWpg@mail.gmail.com","threadId":"50785","inReplyTo":"871s31vjo7.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2019-03-20T14:39:16Z","receivedAt":"2019-03-20T14:39:30Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Mar 20, 2019 at 8:09 AM Sergey Organov <sorganov@gmail.com> wrote:\n>\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n> [...]\n>\n> > But I do have a very strong opinion against adding yet another\n> > option that takes an optional argument.  If we want to allow\n> > cherry-picking a merge commit just as easy as cherrry-picking a\n> > single-parent commit, \"git cherry-pick -m merge\" (assuming 'merge'\n> > is the tip of a branch that is a merge commit) that still requires\n> > the user to say \"-m\" is not a good improvement.  We should just\n> > accept \"git cherry-pick merge\" without any \"-m\" if we want to move\n> > in this direction, I would think.\n>\n> Let's just make '-m 1' the default option indeed. No need for further\n> complexities.\n>\n> Exactly according to what Junio has already said before. Here:\n>\n> https://public-inbox.org/git/xmqqsh5gt9sm.fsf@gitster-ct.c.googlers.com\n>\n> Junio wrote:\n>\n> > Now, it appears, at least to me, that the world pretty much accepted\n> > that the first-parent worldview is often very convenient and worth\n> > supporting by the tool, so the next logical step might be to set\n> > opts->mainline to 1 by default (and allow an explicit \"-m $n\" from\n> > the command line to override it).  But that should happen after this\n> > patch lands---it is logically a separate step, I would think.\n>\n> ... and as that patch already landed...\n\nThis worries me that it'll lead to bad surprises.  Perhaps some folks\ncherry-pick merges around intentionally, but I would want that to be a\nrare occurrence at most.  There are lots of folks at $DAYJOB that\ncherry-pick things, not all of them are expert git-users, and I am\ncertain several have erroneously attempted to cherry-pick merges\nbefore.  I would much rather they continued to get an error message\nand then asked other folks for help so that someone can explain to\nthem what they should instead be doing rather than silently changing\nthe current error into an unwanted operation.  Granted, the users will\nat least get a confusing \"Merge branch <foo>\" commit message for\nsomething that isn't a merge, but I don't think the users will notice\nthat either.  It just means we've got both confusing and ugly history\nwithout the necessary individual commits or with too much having been\ncherry-picked.  If -m 1 is too much to ask people to specify, could we\nprovide some other shorthand?  Or at least make a default-off config\noption people would have to set if they want a cherry-pick of a merge\nto succeed without specifying -m?\n\nElijah\n"},{"id":"372010","messageId":"87lg19mt95.fsf@javad.com","threadId":"50785","inReplyTo":"CABPp-BEe56GFM_2g7EyXmSrULFwRAvSPBomQ66jEQmCs=HhWpg@mail.gmail.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-20T15:59:34Z","receivedAt":"2019-03-20T15:59:38Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> On Wed, Mar 20, 2019 at 8:09 AM Sergey Organov <sorganov@gmail.com> wrote:\n>>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>> [...]\n>>\n>> > But I do have a very strong opinion against adding yet another\n>> > option that takes an optional argument.  If we want to allow\n>> > cherry-picking a merge commit just as easy as cherrry-picking a\n>> > single-parent commit, \"git cherry-pick -m merge\" (assuming 'merge'\n>> > is the tip of a branch that is a merge commit) that still requires\n>> > the user to say \"-m\" is not a good improvement.  We should just\n>> > accept \"git cherry-pick merge\" without any \"-m\" if we want to move\n>> > in this direction, I would think.\n>>\n>> Let's just make '-m 1' the default option indeed. No need for further\n>> complexities.\n>>\n>> Exactly according to what Junio has already said before. Here:\n>>\n>> https://public-inbox.org/git/xmqqsh5gt9sm.fsf@gitster-ct.c.googlers.com\n>>\n>> Junio wrote:\n>>\n>> > Now, it appears, at least to me, that the world pretty much accepted\n>> > that the first-parent worldview is often very convenient and worth\n>> > supporting by the tool, so the next logical step might be to set\n>> > opts->mainline to 1 by default (and allow an explicit \"-m $n\" from\n>> > the command line to override it).  But that should happen after this\n>> > patch lands---it is logically a separate step, I would think.\n>>\n>> ... and as that patch already landed...\n>\n> This worries me that it'll lead to bad surprises.  Perhaps some folks\n> cherry-pick merges around intentionally, but I would want that to be a\n> rare occurrence at most.  There are lots of folks at $DAYJOB that\n> cherry-pick things, not all of them are expert git-users, and I am\n> certain several have erroneously attempted to cherry-pick merges\n> before.\n\nWow, random Joes cherry-picking here and there... Sounds like a bigger\nproblem is lurking here.\n\n> I would much rather they continued to get an error message\n> and then asked other folks for help so that someone can explain to\n> them what they should instead be doing rather than silently changing\n> the current error into an unwanted operation.  Granted, the users will\n> at least get a confusing \"Merge branch <foo>\" commit message for\n> something that isn't a merge, but I don't think the users will notice\n> that either.  It just means we've got both confusing and ugly history\n> without the necessary individual commits or with too much having been\n> cherry-picked.\n\nTo me it seems that cherry-picking wrong commit is cherry-picking wrong\ncommit, no matter if it's a merge or not. I don't think that trying to\nsave a user from such a mistake worth the trouble, given that\ncherry-pick is reversible operation, but I still see your point.\n\n> If -m 1 is too much to ask people to specify, could we provide some\n> other shorthand? Or at least make a default-off config option people\n> would have to set if they want a cherry-pick of a merge to succeed\n> without specifying -m?\n\nIf we decide we still need this safety precaution, I'd opt to continue\nto require '-m 1' to cherry-pick a merge, rather than adding some\nspecial support. Not such a big deal.\n\nBTW, doesn't git have generic configuration support to add default\noption to a command, I wonder (I'm aware of aliases, but they don't\nseem to fit)? The C.J. then would simply add '-m 1' to 'cherry-pick' in\nconfiguration. No luck?\n\n-- Sergey\n\n"},{"id":"372047","messageId":"CALm+SVL1JkfaKt-B5oEHybBn40hy+b0zhBCtwv-ijN7EY14-yQ@mail.gmail.com","threadId":"50785","inReplyTo":"87lg19mt95.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"C.J. Jameson","fromEmail":"cjcjameson@gmail.com","sentAt":"2019-03-21T01:51:46Z","receivedAt":"2019-03-21T01:52:02Z","isPatch":true,"sender":{"key":"cjcjameson@gmail.com","avatar":"https://gravatar.com/avatar/2d485726dfe7eeb8bfc3de4527e0dbc8ce36d880cd4930f2c0b4321da8257c37?d=mp&s=160"},"body":"On Wed, Mar 20, 2019 at 8:59 AM Sergey Organov <sorganov@gmail.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> writes:\n>\n> > On Wed, Mar 20, 2019 at 8:09 AM Sergey Organov <sorganov@gmail.com> wrote:\n> >>\n> >> Junio C Hamano <gitster@pobox.com> writes:\n> >>\n> >> [...]\n> >>\n> >> > But I do have a very strong opinion against adding yet another\n> >> > option that takes an optional argument.  If we want to allow\n> >> > cherry-picking a merge commit just as easy as cherrry-picking a\n> >> > single-parent commit, \"git cherry-pick -m merge\" (assuming 'merge'\n> >> > is the tip of a branch that is a merge commit) that still requires\n> >> > the user to say \"-m\" is not a good improvement.  We should just\n> >> > accept \"git cherry-pick merge\" without any \"-m\" if we want to move\n> >> > in this direction, I would think.\n> >>\n> >> Let's just make '-m 1' the default option indeed. No need for further\n> >> complexities.\n> >>\n> >> Exactly according to what Junio has already said before. Here:\n> >>\n> >> https://public-inbox.org/git/xmqqsh5gt9sm.fsf@gitster-ct.c.googlers.com\n> >>\n> >> Junio wrote:\n> >>\n> >> > Now, it appears, at least to me, that the world pretty much accepted\n> >> > that the first-parent worldview is often very convenient and worth\n> >> > supporting by the tool, so the next logical step might be to set\n> >> > opts->mainline to 1 by default (and allow an explicit \"-m $n\" from\n> >> > the command line to override it).  But that should happen after this\n> >> > patch lands---it is logically a separate step, I would think.\n> >>\n> >> ... and as that patch already landed...\n> >\n> > This worries me that it'll lead to bad surprises.  Perhaps some folks\n> > cherry-pick merges around intentionally, but I would want that to be a\n> > rare occurrence at most.  There are lots of folks at $DAYJOB that\n> > cherry-pick things, not all of them are expert git-users, and I am\n> > certain several have erroneously attempted to cherry-pick merges\n> > before.\n>\n> Wow, random Joes cherry-picking here and there... Sounds like a bigger\n> problem is lurking here.\n\nIndeed, I was the de facto release manager at $DAYJOB and I didn't know\nabout needing `--mainline` until we happened to need it. At this point,\nit's just more fuel for me arguing that the merge commits are extraneous\nnoise in our repo's history, but projects have inertia-of-git-workflow.\n>\n> > I would much rather they continued to get an error message\n> > and then asked other folks for help so that someone can explain to\n> > them what they should instead be doing rather than silently changing\n> > the current error into an unwanted operation.  Granted, the users will\n> > at least get a confusing \"Merge branch <foo>\" commit message for\n> > something that isn't a merge, but I don't think the users will notice\n> > that either.  It just means we've got both confusing and ugly history\n> > without the necessary individual commits or with too much having been\n> > cherry-picked.\n>\n> To me it seems that cherry-picking wrong commit is cherry-picking wrong\n> commit, no matter if it's a merge or not. I don't think that trying to\n> save a user from such a mistake worth the trouble, given that\n> cherry-pick is reversible operation, but I still see your point.\n>\nI'm with Sergey: that people would know what code is being brought in.\n\nWe could specifically code it up so that 3-way merges still need\nan explicit `-m` to be set...\n\n> > If -m 1 is too much to ask people to specify, could we provide some\n> > other shorthand? Or at least make a default-off config option people\n> > would have to set if they want a cherry-pick of a merge to succeed\n> > without specifying -m?\n>\n> If we decide we still need this safety precaution, I'd opt to continue\n> to require '-m 1' to cherry-pick a merge, rather than adding some\n> special support. Not such a big deal.\n>\n> BTW, doesn't git have generic configuration support to add default\n> option to a command, I wonder (I'm aware of aliases, but they don't\n> seem to fit)? The C.J. then would simply add '-m 1' to 'cherry-pick' in\n> configuration. No luck?\n>\nI'm not quite sure how I'd do that; pointers?\n\n> -- Sergey\n>\n"},{"id":"372049","messageId":"xmqqd0mlt1h1.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"CABPp-BEe56GFM_2g7EyXmSrULFwRAvSPBomQ66jEQmCs=HhWpg@mail.gmail.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-21T02:17:46Z","receivedAt":"2019-03-21T02:17:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> This worries me that it'll lead to bad surprises.  Perhaps some folks\n> cherry-pick merges around intentionally, but I would want that to be a\n> rare occurrence at most.  There are lots of folks at $DAYJOB that\n> cherry-pick things, not all of them are expert git-users, and I am\n> certain several have erroneously attempted to cherry-pick merges\n> before.\n\nIt was a lot simpler back when \"git cherry-pick\" did not accept\nranges.  You are either knowingly cherry-picking a merge, or doing\nso by mistake, and because the command rejected cherry-picking a\nmerge without being told with \"-m $n\" which parent the mainline is\nby default, we are assured that the user knew that s/he was picking\na merge commit when we saw \"-m $n\".\n\nIt's not so simple in the world after we started allowing picks of a\nrange.  \"cherry-pick -m1 A..B\" did not work when the range A..B is a\nmixture of merges and non-merges (which is the case 100% of the\ntime), as the command used to error out when given the -m option for\na single-parent commit.  Earlier we said that \"as long as the $n\ndoes not exceed the number of actual parents, let's allow '-m $n'\neven for non-merge commits.\" to fix it.\n\nWe can just reject this RFC patch and we'd be in a slightly safer\nplace.  You still need to tell us with \"-m 1\" on the command line\nthat you are picking a range with merges in it.  But then I am sure\nthat clueless people blindly would alias \"pick = cherry-pick -m1\" and\nuse \"git pick\" and blindly pick ranges here and there, so I am not\nsure such a slightly-more safety buys us very much.\n\n"},{"id":"372051","messageId":"xmqq4l7wuddk.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"xmqqd0mlt1h1.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-21T03:15:19Z","receivedAt":"2019-03-21T03:15:24Z","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> Elijah Newren <newren@gmail.com> writes:\n>\n>> This worries me that it'll lead to bad surprises.  Perhaps some folks\n>> cherry-pick merges around intentionally, but I would want that to be a\n>> rare occurrence at most.\n>\n> We can just reject this RFC patch and we'd be in a slightly safer\n> place.  You still need to tell us with \"-m 1\" on the command line\n> that you are picking a range with merges in it.  But then I am sure\n> that clueless people blindly would alias \"pick = cherry-pick -m1\" and\n> use \"git pick\" and blindly pick ranges here and there, so I am not\n> sure such a slightly-more safety buys us very much.\n\nTo put it a bit differently, I share with you that picking merges\nshould be deliberate and it is safer to make sure allowing it only\nwhen the told us that s/he knows the commit being picked is a merge,\nbut when we started allowing \"-m 1\" for non-merge commits in the\ncurrent world where cherry-pick can work on a range, the ship has\nalready sailed.\n"},{"id":"372054","messageId":"87pnqklr8a.fsf@javad.com","threadId":"50785","inReplyTo":"xmqq4l7wuddk.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-21T05:40:53Z","receivedAt":"2019-03-21T05:40:57Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Elijah Newren <newren@gmail.com> writes:\n>>\n>>> This worries me that it'll lead to bad surprises.  Perhaps some folks\n>>> cherry-pick merges around intentionally, but I would want that to be a\n>>> rare occurrence at most.\n>>\n>> We can just reject this RFC patch and we'd be in a slightly safer\n>> place.  You still need to tell us with \"-m 1\" on the command line\n>> that you are picking a range with merges in it.  But then I am sure\n>> that clueless people blindly would alias \"pick = cherry-pick -m1\" and\n>> use \"git pick\" and blindly pick ranges here and there, so I am not\n>> sure such a slightly-more safety buys us very much.\n>\n> To put it a bit differently, I share with you that picking merges\n> should be deliberate and it is safer to make sure allowing it only\n> when the told us that s/he knows the commit being picked is a merge,\n\nSomething like \"--[no-]ban-merges\" then [*], having \"--ban-merges\" as\ndefault?\n\n> but when we started allowing \"-m 1\" for non-merge commits in the\n> current world where cherry-pick can work on a range, the ship has\n> already sailed.\n\nExcept that it could be a different ship, provided we've got\n\"--ban-merges\". Having \"-m 1\" as default stops to be an issue, and\nexplicit \"-m 1\" could then imply --no-ban-merges, that could be in turn\noverwritten by explicit \"--ban-merges\", if necessary.\n\n[*] \"--[no]-merges\" won't do, as one would expect merges to be silently\ndropped from a range of cherry-picks, similar to what \"git log\" or \"git\nrev-list\" does. BTW, is it a good idea for cherry-pick to start to use\n\"git rev-list\" and support all the options available there?\n\n-- Sergey\n"},{"id":"372055","messageId":"xmqqlg18srrp.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"87pnqklr8a.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-21T05:47:22Z","receivedAt":"2019-03-21T05:47:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n>> To put it a bit differently, I share with you that picking merges\n>> should be deliberate and it is safer to make sure allowing it only\n>> when the told us that s/he knows the commit being picked is a merge,\n>\n> Something like \"--[no-]ban-merges\" then [*], having \"--ban-merges\" as\n> default?\n>\n>> but when we started allowing \"-m 1\" for non-merge commits in the\n>> current world where cherry-pick can work on a range, the ship has\n>> already sailed.\n>\n> Except that it could be a different ship, provided we've got\n> \"--ban-merges\". Having \"-m 1\" as default stops to be an issue, and\n> explicit \"-m 1\" could then imply --no-ban-merges, that could be in turn\n> overwritten by explicit \"--ban-merges\", if necessary.\n\nThe same effect can be had by just reverting \"let's allow -m1 for\nsingle-parent commit\", can't it?  That is a far simpler solution, I\nwould say.\n"},{"id":"372058","messageId":"878sx8lpqw.fsf@javad.com","threadId":"50785","inReplyTo":"xmqqlg18srrp.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-21T06:12:55Z","receivedAt":"2019-03-21T06:12:59Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>>> To put it a bit differently, I share with you that picking merges\n>>> should be deliberate and it is safer to make sure allowing it only\n>>> when the told us that s/he knows the commit being picked is a merge,\n>>\n>> Something like \"--[no-]ban-merges\" then [*], having \"--ban-merges\" as\n>> default?\n>>\n>>> but when we started allowing \"-m 1\" for non-merge commits in the\n>>> current world where cherry-pick can work on a range, the ship has\n>>> already sailed.\n>>\n>> Except that it could be a different ship, provided we've got\n>> \"--ban-merges\". Having \"-m 1\" as default stops to be an issue, and\n>> explicit \"-m 1\" could then imply --no-ban-merges, that could be in turn\n>> overwritten by explicit \"--ban-merges\", if necessary.\n>\n> The same effect can be had by just reverting \"let's allow -m1 for\n> single-parent commit\", can't it?  That is a far simpler solution, I\n> would say.\n\nThose one didn't introduce the issue currently at hand, as we still\ndon't allow merges by default, so why do we need to rewind it?\n\n-- Sergey\n"},{"id":"372071","messageId":"87lg18k99w.fsf@javad.com","threadId":"50785","inReplyTo":"xmqqlg18srrp.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-21T06:54:03Z","receivedAt":"2019-03-21T06:54:09Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>>> To put it a bit differently, I share with you that picking merges\n>>> should be deliberate and it is safer to make sure allowing it only\n>>> when the told us that s/he knows the commit being picked is a merge,\n>>\n>> Something like \"--[no-]ban-merges\" then [*], having \"--ban-merges\" as\n>> default?\n>>\n>>> but when we started allowing \"-m 1\" for non-merge commits in the\n>>> current world where cherry-pick can work on a range, the ship has\n>>> already sailed.\n>>\n>> Except that it could be a different ship, provided we've got\n>> \"--ban-merges\". Having \"-m 1\" as default stops to be an issue, and\n>> explicit \"-m 1\" could then imply --no-ban-merges, that could be in turn\n>> overwritten by explicit \"--ban-merges\", if necessary.\n>\n> The same effect can be had by just reverting \"let's allow -m1 for\n> single-parent commit\", can't it?  That is a far simpler solution, I\n> would say.\n\nTo clarify further my short answer:\n\nS>  Those one didn't introduce the issue currently at hand, as we still\nS>  don't allow merges by default, so why do we need to rewind it?\n\nThe \"let's allow -m1 for single-parent commit\" even doesn't affect merge\ncommits at all, so I fail to see how reverting it may help to resolve\nthe issue that C.J. wants to solve with his proposed patch.\n\nIf we want to disallow picking merges by default, it's already so right\nnow, no reverts or patches required.\n\nThen, as I see it, it's the current way of allowing merges, that\ncryptically reads as \"-m 1\", that makes the OP unhappy. This problem was\nalready there before the aforementioned patch, so reverting the patch\nwon't solve it. OTOH, it's exactly this problem that \"--[no]-ban-merges\"\nwould solve.\n\nFinally, \"--[no-]ban-merges\" would allow to easily make \"-m 1\" the\ndefault later, as a separate change, should we decide it's the right\nthing to do.\n\n-- Sergey\n"},{"id":"372073","messageId":"xmqqd0mksk6n.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"878sx8lpqw.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-21T08:31:12Z","receivedAt":"2019-03-21T08:31:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n>> The same effect can be had by just reverting \"let's allow -m1 for\n>> single-parent commit\", can't it?  That is a far simpler solution, I\n>> would say.\n>\n> Those one didn't introduce the issue currently at hand, as we still\n> don't allow merges by default, so why do we need to rewind it?\n\nWith it reverted, \"[alias] cp = cherry-pick -m1\" can be used to\ntrain the user to blindly pick a range that has a merge without\nthinking, which is what I meant by \"ship has already sailed\".\n\nWith it reverted, a range pick of a straight single strand of pearls\nwould still work just fine.  And the user is forced to think and\nchop a range with a merge into a set of subranges each of which is a\nsingle strand of pearls, plus picking individual merges (if picking\nthese merges is really what the user wants, that is).  As ensuring\nthe users to think is the whole point of excercise, the original\nsystem before we allowed \"-m1\" for single parent commit was after\nall giving us the right balance, I guess, without another new\noptions.\n"},{"id":"372074","messageId":"xmqqbm24sk68.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"878sx8lpqw.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-21T08:31:27Z","receivedAt":"2019-03-21T08:31:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n>> The same effect can be had by just reverting \"let's allow -m1 for\n>> single-parent commit\", can't it?  That is a far simpler solution, I\n>> would say.\n>\n> Those one didn't introduce the issue currently at hand, as we still\n> don't allow merges by default, so why do we need to rewind it?\n\nWith it reverted, \"[alias] cp = cherry-pick -m1\" can be used to\ntrain the user to blindly pick a range that has a merge without\nthinking, which is what I meant by \"ship has already sailed\".\n\nWith it reverted, a range pick of a straight single strand of pearls\nwould still work just fine.  And the user is forced to think and\nchop a range with a merge into a set of subranges each of which is a\nsingle strand of pearls, plus picking individual merges (if picking\nthese merges is really what the user wants, that is).  As ensuring\nthe users to think is the whole point of excercise, the original\nsystem before we allowed \"-m1\" for single parent commit was after\nall giving us the right balance, I guess, without having to add yet\nanother new option.\n\n"},{"id":"372091","messageId":"87k1gsh1zs.fsf@javad.com","threadId":"50785","inReplyTo":"xmqqbm24sk68.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-21T11:59:35Z","receivedAt":"2019-03-21T11:59:39Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>>> The same effect can be had by just reverting \"let's allow -m1 for\n>>> single-parent commit\", can't it?  That is a far simpler solution, I\n>>> would say.\n>>\n>> Those one didn't introduce the issue currently at hand, as we still\n>> don't allow merges by default, so why do we need to rewind it?\n>\n> With it reverted, \"[alias] cp = cherry-pick -m1\" can be used to train\n> the user to blindly pick a range that has a merge without thinking,\n> which is what I meant by \"ship has already sailed\".\n\nDid you mean \"With it *not* reverted\" here? Because with it reverted,\n\"cherry-pick -m1\" will rather loudly fail on the first non-merge commit\nin the range. Such alias would be useless without that patch.\n\nThose who don't like such alias are still free not to define or use it.\n\n> With it reverted, a range pick of a straight single strand of pearls\n> would still work just fine.  And the user is forced to think and\n> chop a range with a merge into a set of subranges each of which is a\n> single strand of pearls, plus picking individual merges (if picking\n> these merges is really what the user wants, that is).\n\nUnfortunately, this gets us back to the exact problem with practical\nuse-case the patch was provoked by. There was a large number of commits\nto be picked, and the set has been carefully built by \"git rev-list\" (as\n\"git cherry-pick\" built-in features were not enough: yet another issue).\nSome of these commits were merges, and handling all this manually (and\nrepeatedly) would be a real pain. It was easier to locally patch \"git\ncherry-pick -m1\" to achieve the goal. Then I thought that it could help\nothers in the future, and then took time to provide the patch. It'd be\na real pity to get back to where I started.\n\nBTW, the above also shows that the issue with \"-m 1\" existed even before\n\"git cherry-pick\" started to accept ranges, as I was not using these\nranges anyway.\n\nOverall, I mean that I still need a way to tell \"git cherry-pick\" that I\ndo know what I'm doing, so that it stops complaining on a non-issue.\nThus if the patch is reverted, a new option should be added just for\nthis goal, -- back to where we are now, but an option already added.\n\n> As ensuring the users to think is the whole point of excercise,\n\nYeah, provided they do have a suitable way out. Without that patch,\nthere was none.\n\n> the original system before we allowed \"-m1\" for single parent commit\n> was after all giving us the right balance, I guess, without having to\n> add yet another new option.\n\nNo, unfortunately it didn't, and to me the patch in question still seems\nto be in the right direction, legitimate, and useful.\n\nMoreover, I still can't see how it's harmful. Just don't use '-m 1' if\nyou don't want merges.\n\n-- Sergey\n"},{"id":"372194","messageId":"xmqqy357r6im.fsf@gitster-ct.c.googlers.com","threadId":"50785","inReplyTo":"87k1gsh1zs.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-22T02:24:01Z","receivedAt":"2019-03-22T02:24:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov <sorganov@gmail.com> writes:\n\n>> With it reverted, \"[alias] cp = cherry-pick -m1\" can be used to train\n>> the user to blindly pick a range that has a merge without thinking,\n>> which is what I meant by \"ship has already sailed\".\n>\n> Did you mean \"With it *not* reverted\" here?\n\nThanks for a correction.  Yes, if we do not revert it, then that\nwould allow people to follow a bad workflow we do not want to\nrecommend (and I think that is what Elijah does not want to do), and\nthat is why I said the ship has already sailed.\n\n> Those who don't like such alias are still free not to define or use it.\n\nThat's not the point.  Those who do want to be careful can learn to\nuse a new option --forbid-stupid-things, but why should they?  They\nshould be forbidden from doing stupid things by default, which is\nthe point of this exchange.\n"},{"id":"372231","messageId":"nycvar.QRO.7.76.6.1903221106001.41@tvgsbejvaqbjf.bet","threadId":"50785","inReplyTo":"CABPp-BEe56GFM_2g7EyXmSrULFwRAvSPBomQ66jEQmCs=HhWpg@mail.gmail.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-22T10:13:13Z","receivedAt":"2019-03-22T10:13:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 20 Mar 2019, Elijah Newren wrote:\n\n> On Wed, Mar 20, 2019 at 8:09 AM Sergey Organov <sorganov@gmail.com> wrote:\n> >\n> > Junio C Hamano <gitster@pobox.com> writes:\n> >\n> > [...]\n> >\n> > > But I do have a very strong opinion against adding yet another\n> > > option that takes an optional argument.  If we want to allow\n> > > cherry-picking a merge commit just as easy as cherrry-picking a\n> > > single-parent commit, \"git cherry-pick -m merge\" (assuming 'merge'\n> > > is the tip of a branch that is a merge commit) that still requires\n> > > the user to say \"-m\" is not a good improvement.  We should just\n> > > accept \"git cherry-pick merge\" without any \"-m\" if we want to move\n> > > in this direction, I would think.\n> >\n> > Let's just make '-m 1' the default option indeed. No need for further\n> > complexities.\n> >\n> > Exactly according to what Junio has already said before. Here:\n> >\n> > https://public-inbox.org/git/xmqqsh5gt9sm.fsf@gitster-ct.c.googlers.com\n> >\n> > Junio wrote:\n> >\n> > > Now, it appears, at least to me, that the world pretty much accepted\n> > > that the first-parent worldview is often very convenient and worth\n> > > supporting by the tool, so the next logical step might be to set\n> > > opts->mainline to 1 by default (and allow an explicit \"-m $n\" from\n> > > the command line to override it).  But that should happen after this\n> > > patch lands---it is logically a separate step, I would think.\n> >\n> > ... and as that patch already landed...\n>\n> This worries me that it'll lead to bad surprises.\n\nIndeed. Merge commits simply do not have the same semantics as regular\ncommits. They not only have more than one parent, they also have the\nfurther complication that they, unlike regular commits, do not introduce\nregular code changes, but need to reconcile diverging code changes\ninstead.\n\nAs such, cherry-picking a merge commit typically leads to *many, many*\nmore merge conflicts than cherry-picking regular commits.\n\nIt would appear to be a wise idea to keep that safety line in place, where\nusers cherry-picking the wrong commit would have to tell Git that they\nreally are sure that they want to cherry-pick a merge commit.\n\nAnd I know that I've been there myself, so it is not just some users you\nmight dismiss as less than smart (which is actually not a smart thing to\ndo, users are the essential audience of our software, and if we make it\nhard for users, it is not the users who failed).\n\nWe cannot just \"ignore away\" that merge commits are different from regular\ncommits: Neither in the data shape nor in their purpose are they the same.\n\nCiao,\nDscho\n"},{"id":"372232","messageId":"nycvar.QRO.7.76.6.1903221116530.41@tvgsbejvaqbjf.bet","threadId":"50785","inReplyTo":"xmqqd0mlt1h1.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-22T10:23:23Z","receivedAt":"2019-03-22T10:23:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 21 Mar 2019, Junio C Hamano wrote:\n\n> Elijah Newren <newren@gmail.com> writes:\n>\n> > This worries me that it'll lead to bad surprises.  Perhaps some folks\n> > cherry-pick merges around intentionally, but I would want that to be a\n> > rare occurrence at most.  There are lots of folks at $DAYJOB that\n> > cherry-pick things, not all of them are expert git-users, and I am\n> > certain several have erroneously attempted to cherry-pick merges\n> > before.\n>\n> It was a lot simpler back when \"git cherry-pick\" did not accept\n> ranges.  You are either knowingly cherry-picking a merge, or doing\n> so by mistake, and because the command rejected cherry-picking a\n> merge without being told with \"-m $n\" which parent the mainline is\n> by default, we are assured that the user knew that s/he was picking\n> a merge commit when we saw \"-m $n\".\n\nIndeed.\n\n> It's not so simple in the world after we started allowing picks of a\n> range.  \"cherry-pick -m1 A..B\" did not work when the range A..B is a\n> mixture of merges and non-merges (which is the case 100% of the\n> time), as the command used to error out when given the -m option for\n> a single-parent commit.  Earlier we said that \"as long as the $n\n> does not exceed the number of actual parents, let's allow '-m $n'\n> even for non-merge commits.\" to fix it.\n>\n> We can just reject this RFC patch and we'd be in a slightly safer\n> place.  You still need to tell us with \"-m 1\" on the command line\n> that you are picking a range with merges in it.  But then I am sure\n> that clueless people blindly would alias \"pick = cherry-pick -m1\" and\n> use \"git pick\" and blindly pick ranges here and there, so I am not\n> sure such a slightly-more safety buys us very much.\n\nIt gets even better.\n\nRecently, I found myself wishing for the equivalent of `--rebase-merges`\nin `git cherry-pick`. In other words, I wanted to transplant some commits\n(including merge commits) from a different branch onto the current branch.\n\nCurrently, I work around this by this kind of dance:\n\n\t$ git checkout B^0\n\t$ git rebase -ir --onto HEAD@{1} A\n\t$ git checkout @{-1}\n\t$ git merge --ff-only HEAD@{1}\n\nwhich does work, but it changes the worktree quite a bit (often causing\ncomplete rebuilds due to \"changed\" header files, when those rebuilds were\nnot actually necessary).\n\nIt also has the rather real downside of not mixing well with an\nalready-running rebase...\n\nAnd with that use case in mind, I think that making the `-m 1` option the\ndefault is even less desirable than I thought before.\n\nCiao,\nDscho\n"},{"id":"372254","messageId":"8736nfc4t9.fsf@javad.com","threadId":"50785","inReplyTo":"xmqqy357r6im.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-03-22T15:22:10Z","receivedAt":"2019-03-22T15:22:14Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Sergey Organov <sorganov@gmail.com> writes:\n>\n>>> With it reverted, \"[alias] cp = cherry-pick -m1\" can be used to train\n>>> the user to blindly pick a range that has a merge without thinking,\n>>> which is what I meant by \"ship has already sailed\".\n>>\n>> Did you mean \"With it *not* reverted\" here?\n>\n> Thanks for a correction.  Yes, if we do not revert it, then that\n> would allow people to follow a bad workflow we do not want to\n> recommend (and I think that is what Elijah does not want to do), and\n> that is why I said the ship has already sailed.\n\nI still don't think it makes sense to revert the patch (that fixed a\nreal-life issue) on the sole ground that, as a side-effect, it has\nprovided an opportunity that could potentially be abused, specifically\nby defining a random alias, and then shooting oneself in the foot with\nit. And even then no irreversible damage actually happens.\n\nMoreover, if somebody actually wants to \"follow a bad workflow\", he\nstill needs to ask for it explicitly, either by providing '-m 1', or by\ndefining and using an alias, so let him do it please, maybe he even does\nknow what he is doing, after all.\n\n>\n>> Those who don't like such alias are still free not to define or use it.\n>\n> That's not the point.  Those who do want to be careful can learn to\n> use a new option --forbid-stupid-things, but why should they?\n\nSure thing, who said they should? Fortunately, that's exactly the\ncurrent state, no need to invent and specify any --forbid-stupid-things\noption, and even if we pretend the option is already there and is\nactive by default, still no need to revert anything.\n\n> They should be forbidden from doing stupid things by default, which is\n> the point of this exchange.\n\nI already agreed before to assume this, and it seems that we now all\nagree this safety should be preserved, as there are those who actually\ncare. However, as merges are already forbidden right now with all the\ncurrent defaults, I fail to see how it could justify reverting of\nalready applied patch.\n\nTo me, the actual question here is: what's the option that overrides\nthat default? The current answer is: \"-m 1\", that admittedly is not very\nnice, but has not been introduced by any of the recent patches, so is\nnot solvable by reverting any of them.\n\nTo summarize, as it looks to me, it's mostly the current way of allowing\nmerges, that cryptically reads as \"-m 1\", that makes the OP unhappy.\nThis was already the case before the \"allow '-m 1' for non-merge\ncommits\" patch, so reverting it won't solve the problem in any suitable\nway.\n\nDue to all the above, may we please finally let alone the already\napplied patch and focus on finding (or denying) actual solution to the\noriginal issue of this thread?\n\nIf so, I'm still on the ground of providing new, say,\n\"--no-forbid-merges\" option, if anything. I'm with Duy Nguyen that the\nway suggested by RFC, making value optional for yet another short\noption, is to be avoided at all costs.\n\n-- Sergey\n"},{"id":"372266","messageId":"CALm+SV+mD0oKfQqZBDUixyB6ckHLchj83=0mh7wxuhC5G6kfgQ@mail.gmail.com","threadId":"50785","inReplyTo":"8736nfc4t9.fsf@javad.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"C.J. Jameson","fromEmail":"cjcjameson@gmail.com","sentAt":"2019-03-22T18:27:19Z","receivedAt":"2019-03-22T18:27:34Z","isPatch":true,"sender":{"key":"cjcjameson@gmail.com","avatar":"https://gravatar.com/avatar/2d485726dfe7eeb8bfc3de4527e0dbc8ce36d880cd4930f2c0b4321da8257c37?d=mp&s=160"},"body":"Thank you everyone for the comments!\n\nConfirming Sergey's summary: at this point, I think my only residual\nopinion is that requiring `-m 1` or `--mainline 1` is a little cryptic\nfor someone who's just cherry-picking a single commit, which happens\nto be a merge commit. `--no-forbid-merges` would be the thorough way\nof accommodating it, but it's even more verbose and you'd still need\nto discover it...\n\nI'd be fine abandon this -- thanks again!\n\nC.J.\n\nOn Fri, Mar 22, 2019 at 8:22 AM Sergey Organov <sorganov@gmail.com> wrote:\n>\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n> > Sergey Organov <sorganov@gmail.com> writes:\n> >\n> >>> With it reverted, \"[alias] cp = cherry-pick -m1\" can be used to train\n> >>> the user to blindly pick a range that has a merge without thinking,\n> >>> which is what I meant by \"ship has already sailed\".\n> >>\n> >> Did you mean \"With it *not* reverted\" here?\n> >\n> > Thanks for a correction.  Yes, if we do not revert it, then that\n> > would allow people to follow a bad workflow we do not want to\n> > recommend (and I think that is what Elijah does not want to do), and\n> > that is why I said the ship has already sailed.\n>\n> I still don't think it makes sense to revert the patch (that fixed a\n> real-life issue) on the sole ground that, as a side-effect, it has\n> provided an opportunity that could potentially be abused, specifically\n> by defining a random alias, and then shooting oneself in the foot with\n> it. And even then no irreversible damage actually happens.\n>\n> Moreover, if somebody actually wants to \"follow a bad workflow\", he\n> still needs to ask for it explicitly, either by providing '-m 1', or by\n> defining and using an alias, so let him do it please, maybe he even does\n> know what he is doing, after all.\n>\n> >\n> >> Those who don't like such alias are still free not to define or use it.\n> >\n> > That's not the point.  Those who do want to be careful can learn to\n> > use a new option --forbid-stupid-things, but why should they?\n>\n> Sure thing, who said they should? Fortunately, that's exactly the\n> current state, no need to invent and specify any --forbid-stupid-things\n> option, and even if we pretend the option is already there and is\n> active by default, still no need to revert anything.\n>\n> > They should be forbidden from doing stupid things by default, which is\n> > the point of this exchange.\n>\n> I already agreed before to assume this, and it seems that we now all\n> agree this safety should be preserved, as there are those who actually\n> care. However, as merges are already forbidden right now with all the\n> current defaults, I fail to see how it could justify reverting of\n> already applied patch.\n>\n> To me, the actual question here is: what's the option that overrides\n> that default? The current answer is: \"-m 1\", that admittedly is not very\n> nice, but has not been introduced by any of the recent patches, so is\n> not solvable by reverting any of them.\n>\n> To summarize, as it looks to me, it's mostly the current way of allowing\n> merges, that cryptically reads as \"-m 1\", that makes the OP unhappy.\n> This was already the case before the \"allow '-m 1' for non-merge\n> commits\" patch, so reverting it won't solve the problem in any suitable\n> way.\n>\n> Due to all the above, may we please finally let alone the already\n> applied patch and focus on finding (or denying) actual solution to the\n> original issue of this thread?\n>\n> If so, I'm still on the ground of providing new, say,\n> \"--no-forbid-merges\" option, if anything. I'm with Duy Nguyen that the\n> way suggested by RFC, making value optional for yet another short\n> option, is to be avoided at all costs.\n>\n> -- Sergey\n"},{"id":"372430","messageId":"nycvar.QRO.7.76.6.1903251544260.41@tvgsbejvaqbjf.bet","threadId":"50785","inReplyTo":"CALm+SV+mD0oKfQqZBDUixyB6ckHLchj83=0mh7wxuhC5G6kfgQ@mail.gmail.com","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-25T14:55:43Z","receivedAt":"2019-03-25T14:56:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi C.J.\n\nOn Fri, 22 Mar 2019, C.J. Jameson wrote:\n\n> Confirming Sergey's summary: at this point, I think my only residual\n> opinion is that requiring `-m 1` or `--mainline 1` is a little cryptic\n> for someone who's just cherry-picking a single commit, which happens\n> to be a merge commit.\n\nTwo points:\n\n- You do not happen to cherry-pick a merge commit. If you do that, and do\n  not realize that it is a merge commit, then you have no idea what you\n  actually wanted to pick (the changes relative to the first parent? or the\n  to the second parent?). The fact that Git does not let you do that gives\n  you the opportunity to pause, think, and then be sure what you actually\n  want.\n\n  So Git does the right thing there.\n\n- If it is a little cryptic that `-m 1` (or `-m 2` or `-m 3` or...) is\n  needed, then this needs to be made less cryptic. A fantastic idea would\n  be to teach Git to mention this hint in the exact circumstance when it\n  matters most: when the user just tried to cherry-pick a merge commit.\n\n  I actually do not know whether Git does the right thing there. If it\n  does not, that would be a good direction to focus your efforts on.\n\n> `--no-forbid-merges` would be the thorough way of accommodating it, but\n> it's even more verbose and you'd still need to discover it...\n\nBut it would definitely make sure that we do not pretend that merge\ncommits are anything like non-merge commits.\n\nAs I pointed out elsewhere in this thread: non-merge commits' purpose in\nlife is to introduce something new, in a relatively small, neat package.\nIf it does not fit into one small, neat package, you need to split it\namong multiple non-merge commits.\n\nA *merge commit*'s purpose in life is not to introduce something new, but\nto reconcile diverging changes. I.e. it is more of a moderator, a mediator\nbetween battling non-merge commits. Often, the only real changes a merge\ncommit introduces are changes that are necessary to make the two diverging\nbranches work well together.\n\nIt can be hard in practice to appreciate the difference, but it is very\nreal.\n\nFor example, when you `git cherry-pick -m 1 <merge>`, you will typically\nend up with a *ton* more merge conflicts than when you cherry-pick a\nnon-merge commit.\n\nAnd when you cherry-pick a merge commit, it is much more like performing a\nsquash merge than performing a cherry-pick, and resolving the merge\nconflicts looks a lot more like bringing peace to a shouting match, much\nlike when you resolve merge conflicts during a *merge* (as opposed to\nresolving merge conflicts during a cherry-pick, which feels a lot more\nlike helping a patch move into a new city it does not yet quite know).\n\n(This is at least what my experience is, from working with 70+ branches\nin Windows that I maintain on top of core Git.)\n\nCiao,\nDscho\n\n> I'd be fine abandon this -- thanks again!\n>\n> C.J.\n>\n> On Fri, Mar 22, 2019 at 8:22 AM Sergey Organov <sorganov@gmail.com> wrote:\n> >\n> > Junio C Hamano <gitster@pobox.com> writes:\n> >\n> > > Sergey Organov <sorganov@gmail.com> writes:\n> > >\n> > >>> With it reverted, \"[alias] cp = cherry-pick -m1\" can be used to train\n> > >>> the user to blindly pick a range that has a merge without thinking,\n> > >>> which is what I meant by \"ship has already sailed\".\n> > >>\n> > >> Did you mean \"With it *not* reverted\" here?\n> > >\n> > > Thanks for a correction.  Yes, if we do not revert it, then that\n> > > would allow people to follow a bad workflow we do not want to\n> > > recommend (and I think that is what Elijah does not want to do), and\n> > > that is why I said the ship has already sailed.\n> >\n> > I still don't think it makes sense to revert the patch (that fixed a\n> > real-life issue) on the sole ground that, as a side-effect, it has\n> > provided an opportunity that could potentially be abused, specifically\n> > by defining a random alias, and then shooting oneself in the foot with\n> > it. And even then no irreversible damage actually happens.\n> >\n> > Moreover, if somebody actually wants to \"follow a bad workflow\", he\n> > still needs to ask for it explicitly, either by providing '-m 1', or by\n> > defining and using an alias, so let him do it please, maybe he even does\n> > know what he is doing, after all.\n> >\n> > >\n> > >> Those who don't like such alias are still free not to define or use it.\n> > >\n> > > That's not the point.  Those who do want to be careful can learn to\n> > > use a new option --forbid-stupid-things, but why should they?\n> >\n> > Sure thing, who said they should? Fortunately, that's exactly the\n> > current state, no need to invent and specify any --forbid-stupid-things\n> > option, and even if we pretend the option is already there and is\n> > active by default, still no need to revert anything.\n> >\n> > > They should be forbidden from doing stupid things by default, which is\n> > > the point of this exchange.\n> >\n> > I already agreed before to assume this, and it seems that we now all\n> > agree this safety should be preserved, as there are those who actually\n> > care. However, as merges are already forbidden right now with all the\n> > current defaults, I fail to see how it could justify reverting of\n> > already applied patch.\n> >\n> > To me, the actual question here is: what's the option that overrides\n> > that default? The current answer is: \"-m 1\", that admittedly is not very\n> > nice, but has not been introduced by any of the recent patches, so is\n> > not solvable by reverting any of them.\n> >\n> > To summarize, as it looks to me, it's mostly the current way of allowing\n> > merges, that cryptically reads as \"-m 1\", that makes the OP unhappy.\n> > This was already the case before the \"allow '-m 1' for non-merge\n> > commits\" patch, so reverting it won't solve the problem in any suitable\n> > way.\n> >\n> > Due to all the above, may we please finally let alone the already\n> > applied patch and focus on finding (or denying) actual solution to the\n> > original issue of this thread?\n> >\n> > If so, I'm still on the ground of providing new, say,\n> > \"--no-forbid-merges\" option, if anything. I'm with Duy Nguyen that the\n> > way suggested by RFC, making value optional for yet another short\n> > option, is to be avoided at all costs.\n> >\n> > -- Sergey\n>\n"},{"id":"372435","messageId":"CALm+SVL6ZyVVPqLEiwEzXru8TOjP7JVfpH=THdT9nLvH84O_Mg@mail.gmail.com","threadId":"50785","inReplyTo":"nycvar.QRO.7.76.6.1903251544260.41@tvgsbejvaqbjf.bet","subject":"Re: [RFC PATCH] cherry-pick: set default `--mainline` parent to 1","fromName":"C.J. Jameson","fromEmail":"cjcjameson@gmail.com","sentAt":"2019-03-25T15:41:56Z","receivedAt":"2019-03-25T15:42:13Z","isPatch":true,"sender":{"key":"cjcjameson@gmail.com","avatar":"https://gravatar.com/avatar/2d485726dfe7eeb8bfc3de4527e0dbc8ce36d880cd4930f2c0b4321da8257c37?d=mp&s=160"},"body":"On Mon, Mar 25, 2019 at 7:56 AM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi C.J.\n>\n> On Fri, 22 Mar 2019, C.J. Jameson wrote:\n>\n> > Confirming Sergey's summary: at this point, I think my only residual\n> > opinion is that requiring `-m 1` or `--mainline 1` is a little cryptic\n> > for someone who's just cherry-picking a single commit, which happens\n> > to be a merge commit.\n>\n> Two points:\n>\n> - You do not happen to cherry-pick a merge commit. If you do that, and do\n>   not realize that it is a merge commit, then you have no idea what you\n>   actually wanted to pick (the changes relative to the first parent? or the\n>   to the second parent?). The fact that Git does not let you do that gives\n>   you the opportunity to pause, think, and then be sure what you actually\n>   want.\n>\n>   So Git does the right thing there.\n>\n> - If it is a little cryptic that `-m 1` (or `-m 2` or `-m 3` or...) is\n>   needed, then this needs to be made less cryptic. A fantastic idea would\n>   be to teach Git to mention this hint in the exact circumstance when it\n>   matters most: when the user just tried to cherry-pick a merge commit.\n>\n>   I actually do not know whether Git does the right thing there. If it\n>   does not, that would be a good direction to focus your efforts on.\n>\n> > `--no-forbid-merges` would be the thorough way of accommodating it, but\n> > it's even more verbose and you'd still need to discover it...\n>\n> But it would definitely make sure that we do not pretend that merge\n> commits are anything like non-merge commits.\n>\n> As I pointed out elsewhere in this thread: non-merge commits' purpose in\n> life is to introduce something new, in a relatively small, neat package.\n> If it does not fit into one small, neat package, you need to split it\n> among multiple non-merge commits.\n>\n> A *merge commit*'s purpose in life is not to introduce something new, but\n> to reconcile diverging changes. I.e. it is more of a moderator, a mediator\n> between battling non-merge commits. Often, the only real changes a merge\n> commit introduces are changes that are necessary to make the two diverging\n> branches work well together.\n>\n> It can be hard in practice to appreciate the difference, but it is very\n> real.\n>\nTotally. My context is that we have way too many low-value / no-value merge\ncommits in our repo, and we put our version tags on the merge commits rather\nthan the non-merge commits (the ones with the meaningful code changes).\nI wish it were different...\n\nIt's cool you phrase it this way, because it highlights how our repo's settings\nstray from the meaningful purpose-in-life of these git fundamentals. In Gitlab\n(same is possible in Github), we have our repo configured to:\n\n- always create a merge commit when merging to master, even if it's\n  fast-forwardable\n- auto-squash all Merge Requests / Pull Requests into one commit\n  when merging\n\nThank you all for helping me get a very deep understanding of the matter,\nbecause it enables me to keep pushing my team to select different settings.\n\n> For example, when you `git cherry-pick -m 1 <merge>`, you will typically\n> end up with a *ton* more merge conflicts than when you cherry-pick a\n> non-merge commit.\n>\n> And when you cherry-pick a merge commit, it is much more like performing a\n> squash merge than performing a cherry-pick, and resolving the merge\n> conflicts looks a lot more like bringing peace to a shouting match, much\n> like when you resolve merge conflicts during a *merge* (as opposed to\n> resolving merge conflicts during a cherry-pick, which feels a lot more\n> like helping a patch move into a new city it does not yet quite know).\n>\nYea... we don't feel the pain of squash-like merging at cherry-pick time,\nbecause pull-requestors' merge requests have already been squashed down\nwhether they like it or not...! :( uff da\n\n> (This is at least what my experience is, from working with 70+ branches\n> in Windows that I maintain on top of core Git.)\n>\n> Ciao,\n> Dscho\n>\n> > I'd be fine abandon this -- thanks again!\n> >\n> > C.J.\n> >\n> > On Fri, Mar 22, 2019 at 8:22 AM Sergey Organov <sorganov@gmail.com> wrote:\n> > >\n> > > Junio C Hamano <gitster@pobox.com> writes:\n> > >\n> > > > Sergey Organov <sorganov@gmail.com> writes:\n> > > >\n> > > >>> With it reverted, \"[alias] cp = cherry-pick -m1\" can be used to train\n> > > >>> the user to blindly pick a range that has a merge without thinking,\n> > > >>> which is what I meant by \"ship has already sailed\".\n> > > >>\n> > > >> Did you mean \"With it *not* reverted\" here?\n> > > >\n> > > > Thanks for a correction.  Yes, if we do not revert it, then that\n> > > > would allow people to follow a bad workflow we do not want to\n> > > > recommend (and I think that is what Elijah does not want to do), and\n> > > > that is why I said the ship has already sailed.\n> > >\n> > > I still don't think it makes sense to revert the patch (that fixed a\n> > > real-life issue) on the sole ground that, as a side-effect, it has\n> > > provided an opportunity that could potentially be abused, specifically\n> > > by defining a random alias, and then shooting oneself in the foot with\n> > > it. And even then no irreversible damage actually happens.\n> > >\n> > > Moreover, if somebody actually wants to \"follow a bad workflow\", he\n> > > still needs to ask for it explicitly, either by providing '-m 1', or by\n> > > defining and using an alias, so let him do it please, maybe he even does\n> > > know what he is doing, after all.\n> > >\n> > > >\n> > > >> Those who don't like such alias are still free not to define or use it.\n> > > >\n> > > > That's not the point.  Those who do want to be careful can learn to\n> > > > use a new option --forbid-stupid-things, but why should they?\n> > >\n> > > Sure thing, who said they should? Fortunately, that's exactly the\n> > > current state, no need to invent and specify any --forbid-stupid-things\n> > > option, and even if we pretend the option is already there and is\n> > > active by default, still no need to revert anything.\n> > >\n> > > > They should be forbidden from doing stupid things by default, which is\n> > > > the point of this exchange.\n> > >\n> > > I already agreed before to assume this, and it seems that we now all\n> > > agree this safety should be preserved, as there are those who actually\n> > > care. However, as merges are already forbidden right now with all the\n> > > current defaults, I fail to see how it could justify reverting of\n> > > already applied patch.\n> > >\n> > > To me, the actual question here is: what's the option that overrides\n> > > that default? The current answer is: \"-m 1\", that admittedly is not very\n> > > nice, but has not been introduced by any of the recent patches, so is\n> > > not solvable by reverting any of them.\n> > >\n> > > To summarize, as it looks to me, it's mostly the current way of allowing\n> > > merges, that cryptically reads as \"-m 1\", that makes the OP unhappy.\n> > > This was already the case before the \"allow '-m 1' for non-merge\n> > > commits\" patch, so reverting it won't solve the problem in any suitable\n> > > way.\n> > >\n> > > Due to all the above, may we please finally let alone the already\n> > > applied patch and focus on finding (or denying) actual solution to the\n> > > original issue of this thread?\n> > >\n> > > If so, I'm still on the ground of providing new, say,\n> > > \"--no-forbid-merges\" option, if anything. I'm with Duy Nguyen that the\n> > > way suggested by RFC, making value optional for yet another short\n> > > option, is to be avoided at all costs.\n> > >\n> > > -- Sergey\n> >\n"}]}