{"thread":{"id":"48096","subject":"[PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","startedAt":"2018-03-20T10:11:30Z","lastAt":"2018-03-22T16:49:10Z","messageCount":9,"participants":["Phillip Wood","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"342308","messageId":"20180320101114.17663-1-phillip.wood@talktalk.net","threadId":"48096","inReplyTo":null,"subject":"[PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T10:11:12Z","receivedAt":"2018-03-20T10:11:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThese patches apply on top of js/rebase-recreate-merge. They extend\nthe --keep-empty fix from maint [1] to work with --recreate-merges.\n\n[1] https://public-inbox.org/git/20180320100315.15261-3-phillip.wood@talktalk.net/T/#u\n\nPhillip Wood (2):\n add failing test for rebase --recreate-merges --keep-empty\n rebase --recreate-merges --keep-empty: don't prune empty commits\n\n sequencer.c | 30 ++++++++++++++++--------------\n t/t3421-rebase-topology-linear.sh | 3 ++-\n 2 files changed, 18 insertions(+), 15 deletions(-)\n\n-- \n2.16.2\n\n"},{"id":"342309","messageId":"20180320101114.17663-3-phillip.wood@talktalk.net","threadId":"48096","inReplyTo":"20180320101114.17663-1-phillip.wood@talktalk.net","subject":"[PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T10:11:14Z","receivedAt":"2018-03-20T10:11:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are empty commits on the left hand side of $upstream...HEAD\nthen the empty commits on the right hand side that we want to keep are\npruned because they are marked as cherry-picks. Fix this by keeping\nthe commits that are empty or are not marked as cherry-picks.\n\nSigned-off-by: Phillip Wood \n---\n sequencer.c | 30 ++++++++++++++++--------------\n t/t3421-rebase-topology-linear.sh | 4 ++--\n 2 files changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex d8cc63dbe4..da87c390ed 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2975,14 +2975,15 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t */\n \twhile ((commit = get_revision(revs))) {\n \t\tstruct commit_list *to_merge;\n-\t\tint is_octopus;\n+\t\tint is_octopus, is_empty;\n \t\tconst char *p1, *p2;\n \t\tstruct object_id *oid;\n \n \t\ttail = &commit_list_insert(commit, tail)->next;\n \t\toidset_insert(&interesting, &commit->object.oid);\n \n-\t\tif ((commit->object.flags & PATCHSAME))\n+\t\tis_empty = is_original_commit_empty(commit);\n+\t\tif (!is_empty && (commit->object.flags & PATCHSAME))\n \t\t\tcontinue;\n \n \t\tstrbuf_reset(&oneline);\n@@ -2992,7 +2993,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\tif (!to_merge) {\n \t\t\t/* non-merge commit: easy case */\n \t\t\tstrbuf_reset(&buf);\n-\t\t\tif (!keep_empty && is_original_commit_empty(commit))\n+\t\t\tif (!keep_empty && is_empty)\n \t\t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n \t\t\tstrbuf_addf(&buf, \"%s %s %s\", cmd_pick,\n \t\t\t\t oid_to_hex(&commit->object.oid),\n@@ -3172,12 +3173,9 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \n \tinit_revisions(&revs, NULL);\n \trevs.verbose_header = 1;\n-\tif (recreate_merges)\n-\t\trevs.cherry_mark = 1;\n-\telse {\n+\tif (!recreate_merges)\n \t\trevs.max_parents = 1;\n-\t\trevs.cherry_pick = 1;\n-\t}\n+\trevs.cherry_mark = 1;\n \trevs.limited = 1;\n \trevs.reverse = 1;\n \trevs.right_only = 1;\n@@ -3205,14 +3203,18 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n \t\treturn make_script_with_merges(&pp, &revs, out, flags);\n \n \twhile ((commit = get_revision(&revs))) {\n+\t\tint is_empty = is_original_commit_empty(commit);\n+\n \t\tstrbuf_reset(&buf);\n-\t\tif (!keep_empty && is_original_commit_empty(commit))\n+\t\tif (!keep_empty && is_empty)\n \t\t\tstrbuf_addf(&buf, \"%c \", comment_line_char);\n-\t\tstrbuf_addf(&buf, \"%s %s \", insn,\n-\t\t\t oid_to_hex(&commit->object.oid));\n-\t\tpretty_print_commit(&pp, commit, &buf);\n-\t\tstrbuf_addch(&buf, '\\n');\n-\t\tfputs(buf.buf, out);\n+\t\tif (is_empty || !(commit->object.flags & PATCHSAME)) {\n+\t\t\tstrbuf_addf(&buf, \"%s %s \", insn,\n+\t\t\t\t oid_to_hex(&commit->object.oid));\n+\t\t\tpretty_print_commit(&pp, commit, &buf);\n+\t\t\tstrbuf_addch(&buf, '\\n');\n+\t\t\tfputs(buf.buf, out);\n+\t\t}\n \t}\n \tstrbuf_release(&buf);\n \treturn 0;\ndiff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh\nindex cb7f176f1d..7384010075 100755\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -215,9 +215,9 @@ test_run_rebase () {\n }\n test_run_rebase success ''\n test_run_rebase failure -m\n-test_run_rebase failure -i\n+test_run_rebase success -i\n test_run_rebase failure -p\n-test_run_rebase failure --recreate-merges\n+test_run_rebase success --recreate-merges\n \n # m\n # /\n-- \n2.16.2\n\n"},{"id":"342310","messageId":"20180320101114.17663-2-phillip.wood@talktalk.net","threadId":"48096","inReplyTo":"20180320101114.17663-1-phillip.wood@talktalk.net","subject":"[PATCH 1/2] add failing test for rebase --recreate-merges --keep-empty","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T10:11:13Z","receivedAt":"2018-03-20T10:11:35Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are empty commits on the left hand side of $upstream...HEAD\nthen the empty commits on the right hand side that we want to keep are\nbeing pruned. This will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n t/t3421-rebase-topology-linear.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh\nindex 68fe2003ef..cb7f176f1d 100755\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -217,6 +217,7 @@ test_run_rebase success ''\n test_run_rebase failure -m\n test_run_rebase failure -i\n test_run_rebase failure -p\n+test_run_rebase failure --recreate-merges\n \n # m\n # /\n-- \n2.16.2\n\n"},{"id":"342324","messageId":"nycvar.QRO.7.76.6.1803201638010.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48096","inReplyTo":"20180320101114.17663-3-phillip.wood@talktalk.net","subject":"Re: [PATCH 2/2] rebase --recreate-merges --keep-empty: don't prune empty commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-20T15:38:53Z","receivedAt":"2018-03-20T15:39:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Tue, 20 Mar 2018, Phillip Wood wrote:\n\n> From: Phillip Wood \n> \n> If there are empty commits on the left hand side of $upstream...HEAD\n> then the empty commits on the right hand side that we want to keep are\n> pruned because they are marked as cherry-picks. Fix this by keeping\n> the commits that are empty or are not marked as cherry-picks.\n\nThank you!\n\n> @@ -3172,12 +3173,9 @@ int sequencer_make_script(FILE *out, int argc, const char **argv,\n> \n> \tinit_revisions(&revs, NULL);\n> \trevs.verbose_header = 1;\n> -\tif (recreate_merges)\n> -\t\trevs.cherry_mark = 1;\n> -\telse {\n> +\tif (!recreate_merges)\n> \t\trevs.max_parents = 1;\n> -\t\trevs.cherry_pick = 1;\n> -\t}\n> +\trevs.cherry_mark = 1;\n> \trevs.limited = 1;\n> \trevs.reverse = 1;\n> \trevs.right_only = 1;\n\nYeah, this looks ugly. I'd rather have your patch series applied first and\nthen rebase my --recreate-merges patch series on top.\n\nCiao,\nDscho\n"},{"id":"342325","messageId":"nycvar.QRO.7.76.6.1803201639540.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48096","inReplyTo":"20180320101114.17663-1-phillip.wood@talktalk.net","subject":"Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-20T15:42:05Z","receivedAt":"2018-03-20T15:42:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Tue, 20 Mar 2018, Phillip Wood wrote:\n\n> From: Phillip Wood \n> \n> These patches apply on top of js/rebase-recreate-merge. They extend\n> the --keep-empty fix from maint [1] to work with --recreate-merges.\n\nAs indicated in another reply, I'd rather rebase the --recreate-merges\npatches on top of your --keep-empty patch series. This obviously means\nthat I would fold essentially all of your 2/2 changes into my\n\"rebase-helper --make-script: introduce a flag to recreate merges\"\n\nThe 1/2 (with s/failure/success/g) would then be added to the\n--recreate-merges patch series at the end.\n\nWould that be okay with you?\n\nCiao,\nDscho\n"},{"id":"342353","messageId":"e306f33a-3cd9-b753-ed61-66b7ff614087@talktalk.net","threadId":"48096","inReplyTo":"nycvar.QRO.7.76.6.1803201639540.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T18:40:45Z","receivedAt":"2018-03-20T18:40:52Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/03/18 15:42, Johannes Schindelin wrote:\n> Hi Phillip,\n> \n> On Tue, 20 Mar 2018, Phillip Wood wrote:\n> \n>> From: Phillip Wood \n>>\n>> These patches apply on top of js/rebase-recreate-merge. They extend\n>> the --keep-empty fix from maint [1] to work with --recreate-merges.\n> \n> As indicated in another reply, I'd rather rebase the --recreate-merges\n> patches on top of your --keep-empty patch series. This obviously means\n> that I would fold essentially all of your 2/2 changes into my\n> \"rebase-helper --make-script: introduce a flag to recreate merges\"\n> \n> The 1/2 (with s/failure/success/g) would then be added to the\n> --recreate-merges patch series at the end.\n> \n> Would that be okay with you?\n\nYes, that's fine, it would give a clearer history\n\nBest Wishes\n\nPhillip\n"},{"id":"342356","messageId":"xmqqr2oer19v.fsf@gitster-ct.c.googlers.com","threadId":"48096","inReplyTo":"e306f33a-3cd9-b753-ed61-66b7ff614087@talktalk.net","subject":"Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-20T19:32:28Z","receivedAt":"2018-03-20T19:32:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 20/03/18 15:42, Johannes Schindelin wrote:\n> ...\n>> As indicated in another reply, I'd rather rebase the --recreate-merges\n>> patches on top of your --keep-empty patch series. This obviously means\n>> that I would fold essentially all of your 2/2 changes into my\n>> \"rebase-helper --make-script: introduce a flag to recreate merges\"\n>> \n>> The 1/2 (with s/failure/success/g) would then be added to the\n>> --recreate-merges patch series at the end.\n>> \n>> Would that be okay with you?\n>\n> Yes, that's fine, it would give a clearer history\n\nWith or without the above plan, what we saw from you were a bit\nmessy to queue. The --keep-empty fix series is based on 'maint',\nwhile the --signoff series depends on changes that happened to\nsequencer between 'maint' and 'master', but yet depends on the\nformer.\n\nIn what I'll be pushing out at the end of today's integration run,\nI'll have two topics organized this way:\n\n - pw/rebase-keep-empty-fixes: built by applying the three\n '--keep-empty' patches on top of 'maint'.\n\n - pw/rebase-signoff: built by first merging the above to 0f57f731\n (\"Merge branch 'pw/sequencer-in-process-commit'\", 2018-02-13) and\n then applying \"rebase --signoff\" series.\n\nAlso, I'll revert merge of Dscho's recreate-merges topic to 'next';\ndoing so would probably have to invalidate a few evil merges I've\nbeen carrying to resolve conflicts between it and bc/object-id\ntopic, so today's integration cycle may take a bit longer than\nusual.\n"},{"id":"342499","messageId":"1f7194bb-f48b-e229-94ae-44c58636172f@talktalk.net","threadId":"48096","inReplyTo":"xmqqr2oer19v.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-22T11:03:59Z","receivedAt":"2018-03-22T11:04:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/03/18 19:32, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> On 20/03/18 15:42, Johannes Schindelin wrote:\n>> ...\n>>> As indicated in another reply, I'd rather rebase the --recreate-merges\n>>> patches on top of your --keep-empty patch series. This obviously means\n>>> that I would fold essentially all of your 2/2 changes into my\n>>> \"rebase-helper --make-script: introduce a flag to recreate merges\"\n>>>\n>>> The 1/2 (with s/failure/success/g) would then be added to the\n>>> --recreate-merges patch series at the end.\n>>>\n>>> Would that be okay with you?\n>>\n>> Yes, that's fine, it would give a clearer history\n> \n> With or without the above plan, what we saw from you were a bit\n> messy to queue. The --keep-empty fix series is based on 'maint',\n> while the --signoff series depends on changes that happened to\n> sequencer between 'maint' and 'master', but yet depends on the\n> former.\n\nYes, that is awkward and unfortunate but the idea behind splitting them\ninto two separate series was to have a single set of bug fixes in the\nhistory. The feature needed to be based on master, so if I'd had the bug\nfixes in the same series you'd of had to cherry-pick them to maint which\nwould break branch/tag --contains. I'm not sure if that is a better option.\n\nBest Wishes\n\nPhillip\n\n> In what I'll be pushing out at the end of today's integration run,\n> I'll have two topics organized this way:\n> \n> - pw/rebase-keep-empty-fixes: built by applying the three\n> '--keep-empty' patches on top of 'maint'.\n> \n> - pw/rebase-signoff: built by first merging the above to 0f57f731\n> (\"Merge branch 'pw/sequencer-in-process-commit'\", 2018-02-13) and\n> then applying \"rebase --signoff\" series.\n> \n> Also, I'll revert merge of Dscho's recreate-merges topic to 'next';\n> doing so would probably have to invalidate a few evil merges I've\n> been carrying to resolve conflicts between it and bc/object-id\n> topic, so today's integration cycle may take a bit longer than\n> usual.\n> \n\n"},{"id":"342509","messageId":"xmqq8takm4y4.fsf@gitster-ct.c.googlers.com","threadId":"48096","inReplyTo":"1f7194bb-f48b-e229-94ae-44c58636172f@talktalk.net","subject":"Re: [PATCH 0/2] rebase --recreate-merges --keep-empty: don't prune empty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-22T16:48:51Z","receivedAt":"2018-03-22T16:49:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 20/03/18 19:32, Junio C Hamano wrote:\n>\n>> With or without the above plan, what we saw from you were a bit\n>> messy to queue. The --keep-empty fix series is based on 'maint',\n>> while the --signoff series depends on changes that happened to\n>> sequencer between 'maint' and 'master', but yet depends on the\n>> former.\n>\n> Yes, that is awkward and unfortunate but the idea behind splitting them\n> into two separate series was to have a single set of bug fixes in the\n> history. The feature needed to be based on master, so if I'd had the bug\n> fixes in the same series you'd of had to cherry-pick them to maint which\n> would break branch/tag --contains. I'm not sure if that is a better option.\n\nI said \"a bit messy\" but that was a statement of a fact, not a\ncomplaint. Sometimes, we cannot avoid that necessary solutions to\nreal-life problems must be messy.\n\nI still think what you sent was the best organization, given the\nconstraints ;-).\n\nThanks.\n"}]}