{"thread":{"id":"58932","subject":"[RFC PATCH 0/1] rebase --onto detection of already applied commits","startedAt":"2022-12-12T11:41:18Z","lastAt":"2022-12-15T17:09:35Z","messageCount":12,"participants":["Cristian Ciocaltea","Ævar Arnfjörð Bjarmason","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"468883","messageId":"20221212113516.27816-1-cristian.ciocaltea@collabora.com","threadId":"58932","inReplyTo":null,"subject":"[RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Cristian Ciocaltea","fromEmail":"cristian.ciocaltea@collabora.com","sentAt":"2022-12-12T11:35:15Z","receivedAt":"2022-12-12T11:41:18Z","isPatch":true,"sender":{"key":"cristian.ciocaltea@collabora.com","avatar":null},"body":"Let's consider the following operation:\n\n git rebase --onto new-base upstream feature\n\nwhere 'feature' contains a few commits on top of 'upstream' which need to be\nrebased onto 'new-base'.\n\nThe problem is that some of those commits have been already applied to\n'new-base' and we would like to skip them as in a regular rebase command that\ndoesn't use '--onto', i.e. expecting to see messages like:\n\n warning: skipped previously applied commit [...]\n\nHowever, that doesn't happen and we either get\n\n dropping [...] -- patch contents already upstream\n\nor a conflict if one of the rebased commits doesn't resolve to an empty patch\nanymore, e.g. due to additional changes applied on the target branch. \n\nThis seems to be similar to the behavior of '--reapply-cherry-picks' and cannot\nbe disabled via '--no-reapply-cherry-picks' or by any other means.\n\nThe proposed patch is just a quick workaround, as I'm not sure what would be the\nproper or recommended approach to handle the scenario described. Any advice\nwould be highly appreciated!\n\nCristian Ciocaltea (1):\n rebase --onto: Skip previously applied commits\n\n builtin/rebase.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\n-- \n2.38.1\n\n"},{"id":"468884","messageId":"20221212113516.27816-2-cristian.ciocaltea@collabora.com","threadId":"58932","inReplyTo":"20221212113516.27816-1-cristian.ciocaltea@collabora.com","subject":"[RFC PATCH 1/1] rebase --onto: Skip previously applied commits","fromName":"Cristian Ciocaltea","fromEmail":"cristian.ciocaltea@collabora.com","sentAt":"2022-12-12T11:35:16Z","receivedAt":"2022-12-12T11:41:18Z","isPatch":true,"sender":{"key":"cristian.ciocaltea@collabora.com","avatar":null},"body":"When rebase is used with '--onto ', the patches that might have\nbeen already applied on are not detected, unless they resolve\nto an empty commit. When the related files contain additional changes\nmerged, the rebase operation fails due to conflicts that require manual\nintervention.\n\nEnsure the '--onto' variant behaviour is consistent with the common\nrebase by dropping the already applied commits on the target branch.\n\nNote the current behavior is still reachable by using the\n'--reapply-cherry-picks' flag.\n\nSigned-off-by: Cristian Ciocaltea \n---\n builtin/rebase.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex b22768ca5b9f..2907c6db5cce 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -1659,8 +1659,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_addstr(&buf, \"...\");\n \t\tstrbuf_addstr(&buf, branch_name);\n \t\toptions.onto_name = xstrdup(buf.buf);\n-\t} else if (!options.onto_name)\n+\t} else if (!options.onto_name) {\n \t\toptions.onto_name = options.upstream_name;\n+\t} else if (options.upstream) {\n+\t\toptions.restrict_revision = options.upstream;\n+\t\toptions.upstream = NULL;\n+\t}\n \tif (strstr(options.onto_name, \"...\")) {\n \t\tif (get_oid_mb(options.onto_name, &branch_base) < 0) {\n \t\t\tif (keep_base)\n-- \n2.38.1\n\n"},{"id":"468886","messageId":"221212.86cz8o6bwp.gmgdl@evledraar.gmail.com","threadId":"58932","inReplyTo":"20221212113516.27816-2-cristian.ciocaltea@collabora.com","subject":"Re: [RFC PATCH 1/1] rebase --onto: Skip previously applied commits","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-12T12:27:35Z","receivedAt":"2022-12-12T12:32:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Dec 12 2022, Cristian Ciocaltea wrote:\n\n> When rebase is used with '--onto ', the patches that might have\n> been already applied on are not detected, unless they resolve\n> to an empty commit. When the related files contain additional changes\n> merged, the rebase operation fails due to conflicts that require manual\n> intervention.\n>\n> Ensure the '--onto' variant behaviour is consistent with the common\n> rebase by dropping the already applied commits on the target branch.\n>\n> Note the current behavior is still reachable by using the\n> '--reapply-cherry-picks' flag.\n>\n> Signed-off-by: Cristian Ciocaltea \n> ---\n> builtin/rebase.c | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/rebase.c b/builtin/rebase.c\n> index b22768ca5b9f..2907c6db5cce 100644\n> --- a/builtin/rebase.c\n> +++ b/builtin/rebase.c\n> @@ -1659,8 +1659,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n> \t\tstrbuf_addstr(&buf, \"...\");\n> \t\tstrbuf_addstr(&buf, branch_name);\n> \t\toptions.onto_name = xstrdup(buf.buf);\n> -\t} else if (!options.onto_name)\n> +\t} else if (!options.onto_name) {\n> \t\toptions.onto_name = options.upstream_name;\n> +\t} else if (options.upstream) {\n> +\t\toptions.restrict_revision = options.upstream;\n> +\t\toptions.upstream = NULL;\n> +\t}\n> \tif (strstr(options.onto_name, \"...\")) {\n> \t\tif (get_oid_mb(options.onto_name, &branch_base) < 0) {\n> \t\t\tif (keep_base)\n\nWhen I apply this & run the tests e.g. t3418 will segfault, and t3403\nseems to fail due to the new behavior not having adjusted the test.\n\nWhich would be my \"C\" for the \"RFC\" :)\n\nI.e. try to get the tests working, and not segfaulting.\n\nIf this is a good idea UX wise (I haven't formed an opinion) the main\nthing that should inform that is having to decide on the various cases\nthat the tests are checking for already.\n\nDo I understand this correctly that we'll currently stop and requise a\n\"git rebase --continue\" if we have an empty patch with \"--onto\", but\nwithout \"--onto\" we'll just print a warning, and that you'd like\n\"--onto\" to be consistent with the non-onto case?\n"},{"id":"468897","messageId":"0a5808eb-666e-26ed-edfa-69cc7752ea41@collabora.com","threadId":"58932","inReplyTo":"221212.86cz8o6bwp.gmgdl@evledraar.gmail.com","subject":"Re: [RFC PATCH 1/1] rebase --onto: Skip previously applied commits","fromName":"Cristian Ciocaltea","fromEmail":"cristian.ciocaltea@collabora.com","sentAt":"2022-12-12T15:37:48Z","receivedAt":"2022-12-12T15:38:02Z","isPatch":true,"sender":{"key":"cristian.ciocaltea@collabora.com","avatar":null},"body":"\nOn 12/12/22 14:27, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, Dec 12 2022, Cristian Ciocaltea wrote:\n> \n>> When rebase is used with '--onto ', the patches that might have\n>> been already applied on are not detected, unless they resolve\n>> to an empty commit. When the related files contain additional changes\n>> merged, the rebase operation fails due to conflicts that require manual\n>> intervention.\n>>\n>> Ensure the '--onto' variant behaviour is consistent with the common\n>> rebase by dropping the already applied commits on the target branch.\n>>\n>> Note the current behavior is still reachable by using the\n>> '--reapply-cherry-picks' flag.\n>>\n>> Signed-off-by: Cristian Ciocaltea \n>> ---\n>> builtin/rebase.c | 6 +++++-\n>> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/rebase.c b/builtin/rebase.c\n>> index b22768ca5b9f..2907c6db5cce 100644\n>> --- a/builtin/rebase.c\n>> +++ b/builtin/rebase.c\n>> @@ -1659,8 +1659,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n>> \t\tstrbuf_addstr(&buf, \"...\");\n>> \t\tstrbuf_addstr(&buf, branch_name);\n>> \t\toptions.onto_name = xstrdup(buf.buf);\n>> -\t} else if (!options.onto_name)\n>> +\t} else if (!options.onto_name) {\n>> \t\toptions.onto_name = options.upstream_name;\n>> +\t} else if (options.upstream) {\n>> +\t\toptions.restrict_revision = options.upstream;\n>> +\t\toptions.upstream = NULL;\n>> +\t}\n>> \tif (strstr(options.onto_name, \"...\")) {\n>> \t\tif (get_oid_mb(options.onto_name, &branch_base) < 0) {\n>> \t\t\tif (keep_base)\n> \n> When I apply this & run the tests e.g. t3418 will segfault, and t3403\n> seems to fail due to the new behavior not having adjusted the test.\n\nRight, a bunch of the merge related tests are now failing, which is one \nof the reasons I submitted the patch as RFC. If this is not a (totally) \nwrong solution to the problem, I will try to get all the tests working \nin a subsequent patch revision.\n\n> Which would be my \"C\" for the \"RFC\" :)\n> \n> I.e. try to get the tests working, and not segfaulting.\n> \n> If this is a good idea UX wise (I haven't formed an opinion) the main\n> thing that should inform that is having to decide on the various cases\n> that the tests are checking for already.\n> \n> Do I understand this correctly that we'll currently stop and requise a\n> \"git rebase --continue\" if we have an empty patch with \"--onto\", but\n> without \"--onto\" we'll just print a warning, and that you'd like\n> \"--onto\" to be consistent with the non-onto case?\n\nThe main use case I'm trying to address is the one described in the \ncover letter, mainly to avoid getting conflicts for the already applied \ncommits when the rebase will not result in an empty patch anymore. This \nwould be, indeed, consistent with the non-onto rebase variant.\n\nCurrently, \"rebase --onto\" behaves like \"--reapply-cherry-picks\" is \nbeing enforced, without having the possibility to disable it, which I \nfound rather confusing and not expected.\n\nThanks for the quick feedback,\nCristian\n"},{"id":"468966","messageId":"xmqqlencqhxy.fsf@gitster.g","threadId":"58932","inReplyTo":"20221212113516.27816-1-cristian.ciocaltea@collabora.com","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-13T00:13:29Z","receivedAt":"2022-12-13T00:13:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Cristian Ciocaltea writes:\n\n> Let's consider the following operation:\n>\n> git rebase --onto new-base upstream feature\n>\n> where 'feature' contains a few commits on top of 'upstream' which need to be\n> rebased onto 'new-base'.\n\nIsn't it what \"git rebase new-base feature\" for? \"My 'feature'\nforked from where 'new-base' came from but they updated 'new-base'\nso I do not know if some of what I had in my 'feature' is in\ntheirs. Please forward port what is still left in 'feature' on top\nof updated 'new-base' that I just got from them\".\n\nThe primary reason why we have an explicit \"--onto\" is so that\n\"rebase\" is used just like\n\n\tgit checkout --detach new-base\n\tgit cherry-pick upstream..feature\n\tgit checkout -B feature\n\nto deal with a different situation, i.e. \"My 'feature' forked from\n'upstream', and I have a commit 'new-base'. Just transplant the\nwhole thing on top of it\", without having to worry about \"what is\nalready in new-base?\" at all. After all, 'new-base' may not have\nANY ancestry relationship with the 'upstream', so \"inspect commits\nin the range upstream..new-base to exclude those that are the same\nas the ones in upstream..feature\" is not a well defined operation.\n\n"},{"id":"468976","messageId":"f4b485c8-3bc4-fb0c-3f7a-084daac7c14d@collabora.com","threadId":"58932","inReplyTo":"xmqqlencqhxy.fsf@gitster.g","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Cristian Ciocaltea","fromEmail":"cristian.ciocaltea@collabora.com","sentAt":"2022-12-13T10:37:29Z","receivedAt":"2022-12-13T10:37:42Z","isPatch":true,"sender":{"key":"cristian.ciocaltea@collabora.com","avatar":null},"body":"\nOn 12/13/22 02:13, Junio C Hamano wrote:\n> Cristian Ciocaltea writes:\n> \n>> Let's consider the following operation:\n>>\n>> git rebase --onto new-base upstream feature\n>>\n>> where 'feature' contains a few commits on top of 'upstream' which need to be\n>> rebased onto 'new-base'.\n> \n> Isn't it what \"git rebase new-base feature\" for? \"My 'feature'\n> forked from where 'new-base' came from but they updated 'new-base'\n> so I do not know if some of what I had in my 'feature' is in\n> theirs. Please forward port what is still left in 'feature' on top\n> of updated 'new-base' that I just got from them\".\n\nI should have highlighted that 'feature' contains a bunch of commits \nthat must not be rebased on top of 'new-base', but only the ones in the \n'upstream..feature' range need to be considered.\n\n> The primary reason why we have an explicit \"--onto\" is so that\n> \"rebase\" is used just like\n> \n> \tgit checkout --detach new-base\n> \tgit cherry-pick upstream..feature\n> \tgit checkout -B feature\n> \n> to deal with a different situation, i.e. \"My 'feature' forked from\n> 'upstream', and I have a commit 'new-base'. Just transplant the\n> whole thing on top of it\", without having to worry about \"what is\n> already in new-base?\" at all. After all, 'new-base' may not have\n> ANY ancestry relationship with the 'upstream', so \"inspect commits\n> in the range upstream..new-base to exclude those that are the same\n> as the ones in upstream..feature\" is not a well defined operation.\n> \n\nThanks for clarifying the intended use case of '--onto'. I have wrongly \nassumed it could be used as a more flexible rebase alternative, allowing \none to limit the range of commits to be considered for rebasing on top \nof a given base, without losing the feature of detecting the already \napplied commits.\n\nCurrently '--onto' works as if the user provided the \n'--reapply-cherry-picks' flag, so maybe a possible improvement of this \npatch could be to handle '--no-reapply-cherry-picks' to explicitly \nenable the detection.\n\nWould this be an acceptable extension?\n\nThanks,\nCristian\n"},{"id":"468989","messageId":"xmqqedt3zdg7.fsf@gitster.g","threadId":"58932","inReplyTo":"f4b485c8-3bc4-fb0c-3f7a-084daac7c14d@collabora.com","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-13T12:38:00Z","receivedAt":"2022-12-13T12:41:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Cristian Ciocaltea writes:\n\n> Currently '--onto' works as if the user provided the\n> '--reapply-cherry-picks' flag, so maybe a possible improvement of this\n> patch could be to handle '--no-reapply-cherry-picks' to explicitly\n> enable the detection.\n\nThat sounds like a workable approach that breaks no existing users.\n"},{"id":"468990","messageId":"51209c56-c82e-c393-6983-2a8c1a7388d3@dunelm.org.uk","threadId":"58932","inReplyTo":"f4b485c8-3bc4-fb0c-3f7a-084daac7c14d@collabora.com","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-13T13:04:11Z","receivedAt":"2022-12-13T13:04:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Christian\n\nOn 13/12/2022 10:37, Cristian Ciocaltea wrote:\n> Currently '--onto' works as if the user provided the \n> '--reapply-cherry-picks' flag,\n\n--onto does not affect the cherry-pick detection. When running\n\n\tgit rebase --onto new-base upstream feature\n\nany commits in upstream have been cherry-picked from feature they will \nnot be rebased. What it does not do is look for cherry-picks in \nonto...feature. It would be nice to add that but I'm not sure it is \nstraight forward to do so and still exclude commits that have been \ncherry-picked from feature to upstream.\n\nBest Wishes\n\nPhillip\n"},{"id":"468993","messageId":"9e06bfd1-3e63-2414-e18c-6901fbb290e3@collabora.com","threadId":"58932","inReplyTo":"51209c56-c82e-c393-6983-2a8c1a7388d3@dunelm.org.uk","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Cristian Ciocaltea","fromEmail":"cristian.ciocaltea@collabora.com","sentAt":"2022-12-13T15:34:14Z","receivedAt":"2022-12-13T15:34:23Z","isPatch":true,"sender":{"key":"cristian.ciocaltea@collabora.com","avatar":null},"body":"Hi Phillip,\n\nOn 12/13/22 15:04, Phillip Wood wrote:\n> Hi Christian\n> \n> On 13/12/2022 10:37, Cristian Ciocaltea wrote:\n>> Currently '--onto' works as if the user provided the \n>> '--reapply-cherry-picks' flag,\n> \n> --onto does not affect the cherry-pick detection. When running\n> \n>     git rebase --onto new-base upstream feature\n> \n> any commits in upstream have been cherry-picked from feature they will \n> not be rebased. What it does not do is look for cherry-picks in \n> onto...feature. It would be nice to add that but I'm not sure it is \n> straight forward to do so and still exclude commits that have been \n> cherry-picked from feature to upstream.\n\nThe proposed patch enables looking for commits into new-base..feature \nrange and excluding the ones reachable from upstream. Since this is a \nchange in the existing behavior, we might need to introduce a new flag \nto enable it. I previously suggested to use '--no-reapply-cherry-picks' \nfor this purpose, but now it's pretty obvious this will be a source of \nconfusion, since the \"cherry-picks\" term refers to the commits picked \nfrom feature to upstream instead of new-base, as you already mentioned.\n\nI agree it would be nice to support both exclusion ranges, but I'm not \nsure how complicated the implementation would be, since I don't have any \nprevious experience with the Git internals. Could this be added as a \nseparate feature at a later point?\n\nThanks,\nCristian\n\n> Best Wishes\n> \n> Phillip\n"},{"id":"469130","messageId":"3c5cb346-e6b4-7b85-6d3e-4b625cc21a94@dunelm.org.uk","threadId":"58932","inReplyTo":"9e06bfd1-3e63-2414-e18c-6901fbb290e3@collabora.com","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-15T15:40:13Z","receivedAt":"2022-12-15T15:40:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Cristian\n\nOn 13/12/2022 15:34, Cristian Ciocaltea wrote:\n> Hi Phillip,\n> \n> On 12/13/22 15:04, Phillip Wood wrote:\n>> Hi Christian\n>>\n>> On 13/12/2022 10:37, Cristian Ciocaltea wrote:\n>>> Currently '--onto' works as if the user provided the \n>>> '--reapply-cherry-picks' flag,\n>>\n>> --onto does not affect the cherry-pick detection. When running\n>>\n>>      git rebase --onto new-base upstream feature\n>>\n>> any commits in upstream have been cherry-picked from feature they will \n>> not be rebased. What it does not do is look for cherry-picks in \n>> onto...feature. It would be nice to add that but I'm not sure it is \n>> straight forward to do so and still exclude commits that have been \n>> cherry-picked from feature to upstream.\n> \n> The proposed patch enables looking for commits into new-base..feature \n> range and excluding the ones reachable from upstream. Since this is a \n> change in the existing behavior, we might need to introduce a new flag \n> to enable it. I previously suggested to use '--no-reapply-cherry-picks' \n> for this purpose, but now it's pretty obvious this will be a source of \n> confusion, since the \"cherry-picks\" term refers to the commits picked \n> from feature to upstream instead of new-base, as you already mentioned.\n> \n> I agree it would be nice to support both exclusion ranges, but I'm not \n> sure how complicated the implementation would be, since I don't have any \n> previous experience with the Git internals. Could this be added as a \n> separate feature at a later point?\n\nIf we can I'd rather add code that excludes cherry-pick both ranges. To \nremove the cherry-picks that are in upstream and new-base you could \nrework the todo list generation as follows\n\n1. Calculate the merge-base $mb of feature and upstream\n2. Store the list of commits $mb..feature in an array and in a hash\n table indexed their patch-id.\n3. Walk $mb..upstream calculating the patch-id for each commit and\n removing any commit in the list from step 2 that matches.\n4. If onto is equal to upstream skip to step 7\n5. Calculate the merge-base $mb of feature and onto.\n6. Walk $mb..new-base calculating the patch-id for each commit and\n removing any commit in the list from step 2 that matches.\n7. Generate the todo list using the modified list of commits from step\n 2.\n\nI don't have much time at the moment but I can try and help a bit more \nin the New Year if you want.\n\nBest Wishes\n\nPhillip\n"},{"id":"469132","messageId":"ee59112a-bd07-53cc-dfe3-6ee6f2342287@collabora.com","threadId":"58932","inReplyTo":"3c5cb346-e6b4-7b85-6d3e-4b625cc21a94@dunelm.org.uk","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Cristian Ciocaltea","fromEmail":"cristian.ciocaltea@collabora.com","sentAt":"2022-12-15T16:02:02Z","receivedAt":"2022-12-15T16:02:18Z","isPatch":true,"sender":{"key":"cristian.ciocaltea@collabora.com","avatar":null},"body":"Hi Phillip,\n\nOn 12/15/22 17:40, Phillip Wood wrote:\n> Hi Cristian\n> \n> On 13/12/2022 15:34, Cristian Ciocaltea wrote:\n>> Hi Phillip,\n>>\n>> On 12/13/22 15:04, Phillip Wood wrote:\n>>> Hi Christian\n>>>\n>>> On 13/12/2022 10:37, Cristian Ciocaltea wrote:\n>>>> Currently '--onto' works as if the user provided the \n>>>> '--reapply-cherry-picks' flag,\n>>>\n>>> --onto does not affect the cherry-pick detection. When running\n>>>\n>>>      git rebase --onto new-base upstream feature\n>>>\n>>> any commits in upstream have been cherry-picked from feature they \n>>> will not be rebased. What it does not do is look for cherry-picks in \n>>> onto...feature. It would be nice to add that but I'm not sure it is \n>>> straight forward to do so and still exclude commits that have been \n>>> cherry-picked from feature to upstream.\n>>\n>> The proposed patch enables looking for commits into new-base..feature \n>> range and excluding the ones reachable from upstream. Since this is a \n>> change in the existing behavior, we might need to introduce a new flag \n>> to enable it. I previously suggested to use \n>> '--no-reapply-cherry-picks' for this purpose, but now it's pretty \n>> obvious this will be a source of confusion, since the \"cherry-picks\" \n>> term refers to the commits picked from feature to upstream instead of \n>> new-base, as you already mentioned.\n>>\n>> I agree it would be nice to support both exclusion ranges, but I'm not \n>> sure how complicated the implementation would be, since I don't have \n>> any previous experience with the Git internals. Could this be added as \n>> a separate feature at a later point?\n> \n> If we can I'd rather add code that excludes cherry-pick both ranges. To \n> remove the cherry-picks that are in upstream and new-base you could \n> rework the todo list generation as follows\n> \n> 1. Calculate the merge-base $mb of feature and upstream\n> 2. Store the list of commits $mb..feature in an array and in a hash\n>    table indexed their patch-id.\n> 3. Walk $mb..upstream calculating the patch-id for each commit and\n>    removing any commit in the list from step 2 that matches.\n> 4. If onto is equal to upstream skip to step 7\n> 5. Calculate the merge-base $mb of feature and onto.\n> 6. Walk $mb..new-base calculating the patch-id for each commit and\n>    removing any commit in the list from step 2 that matches.\n> 7. Generate the todo list using the modified list of commits from step\n>    2.\n> \n> I don't have much time at the moment but I can try and help a bit more \n> in the New Year if you want.\n\nThank you for the implementation hints and your availability to help \nfurther! I will try to put this in practice and let you know as soon as \nI get something working.\n\nKind regards,\nCristian\n\n> Best Wishes\n> \n> Phillip\n"},{"id":"469137","messageId":"e174b4a6-f5c2-6856-23bf-216cbab86fbf@dunelm.org.uk","threadId":"58932","inReplyTo":"ee59112a-bd07-53cc-dfe3-6ee6f2342287@collabora.com","subject":"Re: [RFC PATCH 0/1] rebase --onto detection of already applied commits","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-15T17:07:03Z","receivedAt":"2022-12-15T17:09:35Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 15/12/2022 16:02, Cristian Ciocaltea wrote:\n> Hi Phillip,\n> \n> On 12/15/22 17:40, Phillip Wood wrote:\n>> Hi Cristian\n>>\n>> On 13/12/2022 15:34, Cristian Ciocaltea wrote:\n>>> Hi Phillip,\n>>>\n>>> On 12/13/22 15:04, Phillip Wood wrote:\n>>>> Hi Christian\n>>>>\n>>>> On 13/12/2022 10:37, Cristian Ciocaltea wrote:\n>>>>> Currently '--onto' works as if the user provided the \n>>>>> '--reapply-cherry-picks' flag,\n>>>>\n>>>> --onto does not affect the cherry-pick detection. When running\n>>>>\n>>>>      git rebase --onto new-base upstream feature\n>>>>\n>>>> any commits in upstream have been cherry-picked from feature they \n>>>> will not be rebased. What it does not do is look for cherry-picks in \n>>>> onto...feature. It would be nice to add that but I'm not sure it is \n>>>> straight forward to do so and still exclude commits that have been \n>>>> cherry-picked from feature to upstream.\n>>>\n>>> The proposed patch enables looking for commits into new-base..feature \n>>> range and excluding the ones reachable from upstream. Since this is a \n>>> change in the existing behavior, we might need to introduce a new \n>>> flag to enable it. I previously suggested to use \n>>> '--no-reapply-cherry-picks' for this purpose, but now it's pretty \n>>> obvious this will be a source of confusion, since the \"cherry-picks\" \n>>> term refers to the commits picked from feature to upstream instead of \n>>> new-base, as you already mentioned.\n>>>\n>>> I agree it would be nice to support both exclusion ranges, but I'm \n>>> not sure how complicated the implementation would be, since I don't \n>>> have any previous experience with the Git internals. Could this be \n>>> added as a separate feature at a later point?\n>>\n>> If we can I'd rather add code that excludes cherry-pick both ranges. \n>> To remove the cherry-picks that are in upstream and new-base you could \n>> rework the todo list generation as follows\n>>\n>> 1. Calculate the merge-base $mb of feature and upstream\n>> 2. Store the list of commits $mb..feature in an array and in a hash\n>>     table indexed their patch-id.\n>> 3. Walk $mb..upstream calculating the patch-id for each commit and\n>>     removing any commit in the list from step 2 that matches.\n>> 4. If onto is equal to upstream skip to step 7\n>> 5. Calculate the merge-base $mb of feature and onto.\n>> 6. Walk $mb..new-base calculating the patch-id for each commit and\n>>     removing any commit in the list from step 2 that matches.\n>> 7. Generate the todo list using the modified list of commits from step\n>>     2.\n>>\n>> I don't have much time at the moment but I can try and help a bit more \n>> in the New Year if you want.\n> \n> Thank you for the implementation hints and your availability to help \n> further! I will try to put this in practice and let you know as soon as \n> I get something working.\n\nI'd start by looking at the existing todo list generation in \nsequencer.c:sequencer_make_script()\n\nBest Wishes\n\nPhillip\n"}]}