{"thread":{"id":"50036","subject":"[PATCH v2 4/4] t3506: validate '-m 1 -ff' is now accepted for non-merge commits","startedAt":"2018-12-14T13:05:57Z","lastAt":"2019-01-06T14:41:24Z","messageCount":10,"participants":["Sergey Organov","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"365348","messageId":"5baa84089fbfdb16a2887a0144800354297aa0b5.1544764226.git.sorganov@gmail.com","threadId":"50036","inReplyTo":"cover.1544762343.git.sorganov@gmail.com","subject":"[PATCH v2 4/4] t3506: validate '-m 1 -ff' is now accepted for non-merge commits","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-14T04:53:51Z","receivedAt":"2018-12-14T13:05:57Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Signed-off-by: Sergey Organov \n---\n t/t3506-cherry-pick-ff.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\nindex fb889ac..127dd00 100755\n--- a/t/t3506-cherry-pick-ff.sh\n+++ b/t/t3506-cherry-pick-ff.sh\n@@ -64,10 +64,10 @@ test_expect_success 'merge setup' '\n \tgit checkout -b new A\n '\n \n-test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '\n+test_expect_success 'cherry-pick explicit first parent of a non-merge with --ff' '\n \tgit reset --hard A -- &&\n-\ttest_must_fail git cherry-pick --ff -m 1 B &&\n-\tgit diff --exit-code A --\n+\tgit cherry-pick --ff -m 1 B &&\n+\tgit diff --exit-code C --\n '\n \n test_expect_success 'cherry pick a merge with --ff but without -m should fail' '\n-- \n2.10.0.1.g57b01a3\n\n"},{"id":"365349","messageId":"47bde24fd077a7f2a5a6db4c9bb9411c7255b89a.1544764226.git.sorganov@gmail.com","threadId":"50036","inReplyTo":"cover.1544762343.git.sorganov@gmail.com","subject":"[PATCH v2 1/4] t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-14T04:53:51Z","receivedAt":"2018-12-14T13:05:58Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"We are going to allow 'git cherry-pick -m 1' for non-merge commits, so\nthis method to force failure will stop to work.\n\nUse '-m 4' instead as it's very unlikely we will ever have such an\noctopus in this test setup.\n\nSigned-off-by: Sergey Organov \n---\n t/t3510-cherry-pick-sequence.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex c84eeef..941d502 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -61,7 +61,11 @@ test_expect_success 'cherry-pick mid-cherry-pick-sequence' '\n \n test_expect_success 'cherry-pick persists opts correctly' '\n \tpristine_detach initial &&\n-\ttest_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours initial..anotherpick &&\n+\t# to make sure that the session to cherry-pick a sequence\n+\t# gets interrupted, use a high-enough number that is larger\n+\t# than the number of parents of any commit we have created\n+\tmainline=4 &&\n+\ttest_expect_code 128 git cherry-pick -s -m $mainline --strategy=recursive -X patience -X ours initial..anotherpick &&\n \ttest_path_is_dir .git/sequencer &&\n \ttest_path_is_file .git/sequencer/head &&\n \ttest_path_is_file .git/sequencer/todo &&\n@@ -69,7 +73,7 @@ test_expect_success 'cherry-pick persists opts correctly' '\n \techo \"true\" >expect &&\n \tgit config --file=.git/sequencer/opts --get-all options.signoff >actual &&\n \ttest_cmp expect actual &&\n-\techo \"1\" >expect &&\n+\techo \"$mainline\" >expect &&\n \tgit config --file=.git/sequencer/opts --get-all options.mainline >actual &&\n \ttest_cmp expect actual &&\n \techo \"recursive\" >expect &&\n-- \n2.10.0.1.g57b01a3\n\n"},{"id":"365350","messageId":"a8b86bc307141963d0ce6b8f8710b38bd802e302.1544764226.git.sorganov@gmail.com","threadId":"50036","inReplyTo":"cover.1544762343.git.sorganov@gmail.com","subject":"[PATCH v2 2/4] cherry-pick: do not error on non-merge commits when '-m 1' is specified","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-14T04:53:51Z","receivedAt":"2018-12-14T13:06:00Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"When cherry-picking multiple commits, it's impossible to have both\nmerge- and non-merge commits on the same command-line. Not specifying\n'-m 1' results in cherry-pick refusing to handle merge commits, while\nspecifying '-m 1' fails on non-merge commits.\n\nThis patch allows '-m 1' for non-merge commits. As mainline is always\nthe only parent for a non-merge commit, it makes little sense to\ndisable it.\n\nSigned-off-by: Sergey Organov \n---\n sequencer.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e1a4dd1..d0fd61b 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1766,9 +1766,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n \t\t\treturn error(_(\"commit %s does not have parent %d\"),\n \t\t\t\toid_to_hex(&commit->object.oid), opts->mainline);\n \t\tparent = p->item;\n-\t} else if (0 < opts->mainline)\n-\t\treturn error(_(\"mainline was specified but commit %s is not a merge.\"),\n-\t\t\toid_to_hex(&commit->object.oid));\n+\t} else if (1 < opts->mainline)\n+\t\t/*\n+\t\t * Non-first parent explicitly specified as mainline for\n+\t\t * non-merge commit\n+\t\t */\n+\t\treturn error(_(\"commit %s does not have parent %d\"),\n+\t\t\t oid_to_hex(&commit->object.oid), opts->mainline);\n \telse\n \t\tparent = commit->parents->item;\n \n-- \n2.10.0.1.g57b01a3\n\n"},{"id":"365351","messageId":"ccfe8ae38301b6ee1b0924fbf00eb5d20242ea5d.1544764226.git.sorganov@gmail.com","threadId":"50036","inReplyTo":"cover.1544762343.git.sorganov@gmail.com","subject":"[PATCH v2 3/4] t3502: validate '-m 1' argument is now accepted for non-merge commits","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-14T04:53:51Z","receivedAt":"2018-12-14T13:06:01Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Signed-off-by: Sergey Organov \n---\n t/t3502-cherry-pick-merge.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh\nindex b160271..3259bd5 100755\n--- a/t/t3502-cherry-pick-merge.sh\n+++ b/t/t3502-cherry-pick-merge.sh\n@@ -40,12 +40,12 @@ test_expect_success 'cherry-pick -m complains of bogus numbers' '\n \ttest_expect_code 129 git cherry-pick -m 0 b\n '\n \n-test_expect_success 'cherry-pick a non-merge with -m should fail' '\n+test_expect_success 'cherry-pick explicit first parent of a non-merge' '\n \n \tgit reset --hard &&\n \tgit checkout a^0 &&\n-\ttest_expect_code 128 git cherry-pick -m 1 b &&\n-\tgit diff --exit-code a --\n+\tgit cherry-pick -m 1 b &&\n+\tgit diff --exit-code c --\n \n '\n \n@@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f\n \n '\n \n-test_expect_success 'revert a non-merge with -m should fail' '\n+test_expect_success 'revert explicit first parent of a non-merge' '\n \n \tgit reset --hard &&\n \tgit checkout c^0 &&\n-\ttest_must_fail git revert -m 1 b &&\n-\tgit diff --exit-code c\n+\tgit revert -m 1 b &&\n+\tgit diff --exit-code a\n \n '\n \n-- \n2.10.0.1.g57b01a3\n\n"},{"id":"365352","messageId":"cover.1544762343.git.sorganov@gmail.com","threadId":"50036","inReplyTo":"xmqq5zvwesvz.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 0/4] Allow 'cherry-pick -m 1' for non-merge commits","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-14T04:39:03Z","receivedAt":"2018-12-14T13:06:02Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"When cherry-picking multiple commits, it's impossible to have both\nmerge- and non-merge commits on the same command-line. Not specifying\n'-m 1' results in cherry-pick refusing to handle merge commits, while\nspecifying '-m 1' fails on non-merge commits.\n\nThis patch series allow '-m 1' for non-merge commits as well as fixes\nrelevant tests in accordance.\n\nIt also opens the way to making '-m 1' the default, that would be\ninline with the trends to assume first parent to be the mainline in\nmost workflows.\n\nSergey Organov (4):\n t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks\n cherry-pick: do not error on non-merge commits when '-m 1' is\n specified\n t3502: validate '-m 1' argument is now accepted for non-merge commits\n t3506: validate '-m 1 -ff' is now accepted for non-merge commits\n\n sequencer.c | 10 +++++++---\n t/t3502-cherry-pick-merge.sh | 12 ++++++------\n t/t3506-cherry-pick-ff.sh | 6 +++---\n t/t3510-cherry-pick-sequence.sh | 8 ++++++--\n 4 files changed, 22 insertions(+), 14 deletions(-)\n\n-- \n2.10.0.1.g57b01a3\n\n"},{"id":"365800","messageId":"87tvj1ok4k.fsf@javad.com","threadId":"50036","inReplyTo":"cover.1544762343.git.sorganov@gmail.com","subject":"Re: [PATCH v2 0/4] Allow 'cherry-pick -m 1' for non-merge commits","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-25T12:39:55Z","receivedAt":"2018-12-25T12:40:00Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Hi Junio,\n\nWhat's the status of these patches?\n\nThanks,\n\n-- Sergey\n\nSergey Organov writes:\n> When cherry-picking multiple commits, it's impossible to have both\n> merge- and non-merge commits on the same command-line. Not specifying\n> '-m 1' results in cherry-pick refusing to handle merge commits, while\n> specifying '-m 1' fails on non-merge commits.\n>\n> This patch series allow '-m 1' for non-merge commits as well as fixes\n> relevant tests in accordance.\n>\n> It also opens the way to making '-m 1' the default, that would be\n> inline with the trends to assume first parent to be the mainline in\n> most workflows.\n>\n> Sergey Organov (4):\n> t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks\n> cherry-pick: do not error on non-merge commits when '-m 1' is\n> specified\n> t3502: validate '-m 1' argument is now accepted for non-merge commits\n> t3506: validate '-m 1 -ff' is now accepted for non-merge commits\n>\n> sequencer.c | 10 +++++++---\n> t/t3502-cherry-pick-merge.sh | 12 ++++++------\n> t/t3506-cherry-pick-ff.sh | 6 +++---\n> t/t3510-cherry-pick-sequence.sh | 8 ++++++--\n> 4 files changed, 22 insertions(+), 14 deletions(-)\n"},{"id":"365923","messageId":"xmqqr2e1mmuu.fsf@gitster-ct.c.googlers.com","threadId":"50036","inReplyTo":"87tvj1ok4k.fsf@javad.com","subject":"Re: [PATCH v2 0/4] Allow 'cherry-pick -m 1' for non-merge commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-12-26T22:52:07Z","receivedAt":"2018-12-28T20:13:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov , Sergey Organov\n writes:\n\n> Hi Junio,\n>\n> What's the status of these patches?\n\nThe status of these patches is \"Just updated on the list\", as far as\nI am concerned, and its cover letter would have described what's\nimproved relative to the previous round better than whatever answer\nI could give here ;-)\n\nI checked the overall diff between the previous round and the result\nof applying all four patches to find out that the updates are only\nto the tests. The changes in the code (still) looked correct.\n\n\n\n\ndiff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh\nindex b1602718f8..3259bd59eb 100755\n--- a/t/t3502-cherry-pick-merge.sh\n+++ b/t/t3502-cherry-pick-merge.sh\n@@ -40,12 +40,12 @@ test_expect_success 'cherry-pick -m complains of bogus numbers' '\n \ttest_expect_code 129 git cherry-pick -m 0 b\n '\n \n-test_expect_success 'cherry-pick a non-merge with -m should fail' '\n+test_expect_success 'cherry-pick explicit first parent of a non-merge' '\n \n \tgit reset --hard &&\n \tgit checkout a^0 &&\n-\ttest_expect_code 128 git cherry-pick -m 1 b &&\n-\tgit diff --exit-code a --\n+\tgit cherry-pick -m 1 b &&\n+\tgit diff --exit-code c --\n \n '\n \n@@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f\n \n '\n \n-test_expect_success 'revert a non-merge with -m should fail' '\n+test_expect_success 'revert explicit first parent of a non-merge' '\n \n \tgit reset --hard &&\n \tgit checkout c^0 &&\n-\ttest_must_fail git revert -m 1 b &&\n-\tgit diff --exit-code c\n+\tgit revert -m 1 b &&\n+\tgit diff --exit-code a\n \n '\n \ndiff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh\nindex fb889ac6f0..127dd0082f 100755\n--- a/t/t3506-cherry-pick-ff.sh\n+++ b/t/t3506-cherry-pick-ff.sh\n@@ -64,10 +64,10 @@ test_expect_success 'merge setup' '\n \tgit checkout -b new A\n '\n \n-test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '\n+test_expect_success 'cherry-pick explicit first parent of a non-merge with --ff' '\n \tgit reset --hard A -- &&\n-\ttest_must_fail git cherry-pick --ff -m 1 B &&\n-\tgit diff --exit-code A --\n+\tgit cherry-pick --ff -m 1 B &&\n+\tgit diff --exit-code C --\n '\n \n test_expect_success 'cherry pick a merge with --ff but without -m should fail' '\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex c84eeefdc9..941d5026da 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -61,7 +61,11 @@ test_expect_success 'cherry-pick mid-cherry-pick-sequence' '\n \n test_expect_success 'cherry-pick persists opts correctly' '\n \tpristine_detach initial &&\n-\ttest_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours initial..anotherpick &&\n+\t# to make sure that the session to cherry-pick a sequence\n+\t# gets interrupted, use a high-enough number that is larger\n+\t# than the number of parents of any commit we have created\n+\tmainline=4 &&\n+\ttest_expect_code 128 git cherry-pick -s -m $mainline --strategy=recursive -X patience -X ours initial..anotherpick &&\n \ttest_path_is_dir .git/sequencer &&\n \ttest_path_is_file .git/sequencer/head &&\n \ttest_path_is_file .git/sequencer/todo &&\n@@ -69,7 +73,7 @@ test_expect_success 'cherry-pick persists opts correctly' '\n \techo \"true\" >expect &&\n \tgit config --file=.git/sequencer/opts --get-all options.signoff >actual &&\n \ttest_cmp expect actual &&\n-\techo \"1\" >expect &&\n+\techo \"$mainline\" >expect &&\n \tgit config --file=.git/sequencer/opts --get-all options.mainline >actual &&\n \ttest_cmp expect actual &&\n \techo \"recursive\" >expect &&\n"},{"id":"365943","messageId":"87sgygem1a.fsf@javad.com","threadId":"50036","inReplyTo":"xmqqr2e1mmuu.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 0/4] Allow 'cherry-pick -m 1' for non-merge commits","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2018-12-29T09:10:09Z","receivedAt":"2018-12-29T09:10:13Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano writes:\n\n> Sergey Organov , Sergey Organov\n> writes:\n>\n>> Hi Junio,\n>>\n>> What's the status of these patches?\n>\n> The status of these patches is \"Just updated on the list\", as far as\n> I am concerned, and its cover letter would have described what's\n> improved relative to the previous round better than whatever answer\n> I could give here ;-)\n\nYeah, I need to be more thorough, thanks!\n\n-- Sergey\n"},{"id":"366094","messageId":"20190103172221.GB4673@szeder.dev","threadId":"50036","inReplyTo":"ccfe8ae38301b6ee1b0924fbf00eb5d20242ea5d.1544764226.git.sorganov@gmail.com","subject":"Re: [PATCH v2 3/4] t3502: validate '-m 1' argument is now accepted for non-merge commits","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-01-03T17:22:21Z","receivedAt":"2019-01-03T17:22:28Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Dec 14, 2018 at 07:53:51AM +0300, Sergey Organov wrote:\n> Signed-off-by: Sergey Organov \n> ---\n> t/t3502-cherry-pick-merge.sh | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/t/t3502-cherry-pick-merge.sh b/t/t3502-cherry-pick-merge.sh\n> index b160271..3259bd5 100755\n> --- a/t/t3502-cherry-pick-merge.sh\n> +++ b/t/t3502-cherry-pick-merge.sh\n> @@ -40,12 +40,12 @@ test_expect_success 'cherry-pick -m complains of bogus numbers' '\n> \ttest_expect_code 129 git cherry-pick -m 0 b\n> '\n> \n> -test_expect_success 'cherry-pick a non-merge with -m should fail' '\n> +test_expect_success 'cherry-pick explicit first parent of a non-merge' '\n> \n> \tgit reset --hard &&\n> \tgit checkout a^0 &&\n> -\ttest_expect_code 128 git cherry-pick -m 1 b &&\n> -\tgit diff --exit-code a --\n> +\tgit cherry-pick -m 1 b &&\n> +\tgit diff --exit-code c --\n> \n> '\n> \n> @@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f\n> \n> '\n> \n> -test_expect_success 'revert a non-merge with -m should fail' '\n> +test_expect_success 'revert explicit first parent of a non-merge' '\n> \n> \tgit reset --hard &&\n> \tgit checkout c^0 &&\n> -\ttest_must_fail git revert -m 1 b &&\n> -\tgit diff --exit-code c\n> +\tgit revert -m 1 b &&\n> +\tgit diff --exit-code a\n\nYou need disambiguaion here, otherwise this test fails on\ncase-insensitive file systems:\n\n ++git diff --exit-code a\n fatal: ambiguous argument 'a': both revision and filename\n Use '--' to separate paths from revisions, like this:\n 'git [...] -- [...]'\n error: last command exited with $?=128\n not ok 8 - revert explicit first parent of a non-merge\n\n> \n> '\n> \n> -- \n> 2.10.0.1.g57b01a3\n> \n"},{"id":"366206","messageId":"87ftu5etm7.fsf@javad.com","threadId":"50036","inReplyTo":"20190103172221.GB4673@szeder.dev","subject":"Re: [PATCH v2 3/4] t3502: validate '-m 1' argument is now accepted for non-merge commits","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2019-01-06T14:41:20Z","receivedAt":"2019-01-06T14:41:24Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"SZEDER Gábor writes:\n\n> On Fri, Dec 14, 2018 at 07:53:51AM +0300, Sergey Organov wrote:\n>> Signed-off-by: Sergey Organov \n\n[...]\n\n>> \n>> @@ -84,12 +84,12 @@ test_expect_success 'cherry pick a merge relative to nonexistent parent should f\n>> \n>> '\n>> \n>> -test_expect_success 'revert a non-merge with -m should fail' '\n>> +test_expect_success 'revert explicit first parent of a non-merge' '\n>> \n>> \tgit reset --hard &&\n>> \tgit checkout c^0 &&\n>> -\ttest_must_fail git revert -m 1 b &&\n>> -\tgit diff --exit-code c\n>> +\tgit revert -m 1 b &&\n>> +\tgit diff --exit-code a\n>\n> You need disambiguaion here, otherwise this test fails on\n> case-insensitive file systems:\n>\n> ++git diff --exit-code a\n> fatal: ambiguous argument 'a': both revision and filename\n> Use '--' to separate paths from revisions, like this:\n> 'git [...] -- [...]'\n> error: last command exited with $?=128\n> not ok 8 - revert explicit first parent of a non-merge\n\nGood catch, -- thanks a lot!\n\n-- Sergey\n"}]}