{"thread":{"id":"56126","subject":"why \"git rebase\" searching the duplicate patches in rather than in ?","startedAt":"2021-07-19T17:51:08Z","lastAt":"2021-08-01T08:56:51Z","messageCount":7,"participants":["Andy Zhang","Felipe Contreras","Junio C Hamano","Sergey Organov"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"430524","messageId":"CAJcwCMPHFNHi5i=xRg=GAJL5HiUfKu_KUPwYwELofLLtOAK1bg@mail.gmail.com","threadId":"56126","inReplyTo":"CAJcwCMPU9EhRkqeei_LnYyTJRZUQgHCvomrBbW0Qn+Jp1yhQfQ@mail.gmail.com","subject":"why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Andy Zhang","fromEmail":"zhgdrx@gmail.com","sentAt":"2021-07-19T17:45:01Z","receivedAt":"2021-07-19T17:51:08Z","isPatch":false,"sender":{"key":"zhgdrx@gmail.com","avatar":null},"body":"why \"git rebase\" searching the duplicate patches in \nrather than in ?\n\nhi, all:\n\n I am reading the help of \"git rebase\", it says:\n \"If the upstream branch already contains a change you have made\n(e.g., because you mailed a patch which was applied upstream), then\nthat commit will be skipped. \"\n\n But, because we are applying commits to rather than\nto , I really don't understand why we are searching\nthe duplicate patches in rather than in ?\n\n In the following example, the git command is as:\n git rebase --onto master next topic\n\n I think it should be reasonable to search the duplicate patches in\n(i.e, master) instead of .\n\n If possible, an improvement may be an option for enabling searching\nin as well?\n\n Thank you in advance for any enlightenment you can provide!\n\n\nJintao Zhu\n\n//----\nexample:\n\nOld tree is:\n\no---o---o---o---o master\n \\\n o---o---o---o---o next\n \\\n o---o---o topic\n\n\nWe want our tree to look like this:\no---o---o---o---o master\n | \\\n | o'--o'--o' topic\n \\\n o---o---o---o---o next\n\nWe can get this using the following command:\n\n git rebase --onto master next topic\n\n//----\n"},{"id":"430534","messageId":"60f5c1a5c8422_13f62b208e@natae.notmuch","threadId":"56126","inReplyTo":"CAJcwCMPHFNHi5i=xRg=GAJL5HiUfKu_KUPwYwELofLLtOAK1bg@mail.gmail.com","subject":"RE: why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-19T18:17:09Z","receivedAt":"2021-07-19T19:02:44Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Andy Zhang wrote:\n> why \"git rebase\" searching the duplicate patches in \n> rather than in ?\n> \n> hi, all:\n> \n> I am reading the help of \"git rebase\", it says:\n> \"If the upstream branch already contains a change you have made\n> (e.g., because you mailed a patch which was applied upstream), then\n> that commit will be skipped. \"\n> \n> But, because we are applying commits to rather than\n> to , I really don't understand why we are searching\n> the duplicate patches in rather than in branch>?\n> \n> In the following example, the git command is as:\n> git rebase --onto master next topic\n> \n> I think it should be reasonable to search the duplicate patches in\n> (i.e, master) instead of .\n\nMy understanding is that only applies when --onto is not specified:\n\n git rebase master topic\n\nIn this case \"master\" is both the new base, and upstream, and it's in\nthis case where duplicates are skipped.\n\nIf you specify --onto master, then this isn't done.\n\n-- \nFelipe Contreras\n"},{"id":"430552","messageId":"xmqqmtqij63t.fsf@gitster.g","threadId":"56126","inReplyTo":"CAJcwCMPHFNHi5i=xRg=GAJL5HiUfKu_KUPwYwELofLLtOAK1bg@mail.gmail.com","subject":"Re: why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-19T22:23:18Z","receivedAt":"2021-07-19T23:11:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Zhang writes:\n\n> why \"git rebase\" searching the duplicate patches in branch> rather than in ?\n>\n> hi, all:\n>\n> I am reading the help of \"git rebase\", it says:\n> \"If the upstream branch already contains a change you have made\n> (e.g., because you mailed a patch which was applied upstream), then\n> that commit will be skipped. \"\n>\n> But, because we are applying commits to rather than\n> to , I really don't understand why we are searching\n> the duplicate patches in rather than in branch>?\n\nIt is either a design bug or a documentation bug, or both ;-)\n\nI do think it makes sense to skip commits from the branch we are\nrebasing that have equivalent commits in the upstream, as it is\nexpected that upstream might have already applied/cherry-picked some\nof the changes you are rebasing, and you do not want to use the same\nchange twice.\n\nWhen we are transplanting a series of commits from an old base to\ntotally unrelated base using the --onto option, e.g. when replaying\nthe contents of 'topic' relative to 'next' down to 'master' in your\ntopology, however,\n\n> Old tree is:\n>\n> o---o---o---o---o master\n> \\\n> o---o---o---o---o next\n> \\\n> o---o---o topic\n\nit is not necessarily obvious where to stop digging back at. In the\nabove picture where 'master' and 'next' have ancestry relationship,\nwe could try to see if the three commits on 'topic' branch being\nreplayed match any of the commits in next..master range, but when\nusing the --onto option, there does not have to be any relationship\nbetween the and (they do not have to share a\nroot commit). So from that point of view, it probably makes sense\nto default to --no-reapply-cherry-picks when --onto is used, while\ndefaulting --reapply-cherry-picks when --onto is not used.\n\n\n"},{"id":"430574","messageId":"87a6mhgxv4.fsf@osv.gnss.ru","threadId":"56126","inReplyTo":"xmqqmtqij63t.fsf@gitster.g","subject":"Re: why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-07-20T09:04:15Z","receivedAt":"2021-07-20T09:08:34Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano writes:\n\n> Andy Zhang writes:\n>\n>> why \"git rebase\" searching the duplicate patches in > branch> rather than in ?\n>>\n>> hi, all:\n>>\n>> I am reading the help of \"git rebase\", it says:\n>> \"If the upstream branch already contains a change you have made\n>> (e.g., because you mailed a patch which was applied upstream), then\n>> that commit will be skipped. \"\n>>\n>> But, because we are applying commits to rather than\n>> to , I really don't understand why we are searching\n>> the duplicate patches in rather than in > branch>?\n>\n> It is either a design bug or a documentation bug, or both ;-)\n\nIt's definitely /at least/ a documentation bug, as description of the\nfeature is not precise enough. For example, it's unclear if such a\ncommit will appear in the todo list of --interactive. Will it?\n\nIt looks like documentation of \"git rebase\" should be revised to make\nclearer distinction between , , and .\n\n>\n> I do think it makes sense to skip commits from the branch we are\n> rebasing that have equivalent commits in the upstream, as it is\n> expected that upstream might have already applied/cherry-picked some\n> of the changes you are rebasing, and you do not want to use the same\n> change twice.\n\nTo me this only makes sense for the branch we rebase /onto/, and thus it\nactually makes sense for , and for it only happens\nto make sense by default as = in this case.\n\nIf Git currently indeed searches for duplicates in , then it\nlooks like implementation bug, or misfeature. I think the \nshould rather be used.\n\n>\n> When we are transplanting a series of commits from an old base to\n> totally unrelated base using the --onto option, e.g. when replaying\n> the contents of 'topic' relative to 'next' down to 'master' in your\n> topology, however,\n>\n>> Old tree is:\n>>\n>> o---o---o---o---o master\n>> \\\n>> o---o---o---o---o next\n>> \\\n>> o---o---o topic\n>\n> it is not necessarily obvious where to stop digging back at.\n\nSimilar problem should exist for explicitly specified that\nmight happen to have little in common with the current , right?\nIf so, then it's already somehow being solved, even if simply by\nignoring the issue, so adding to the picture doesn't actually\nbring anything significantly new.\n\n> In the\n> above picture where 'master' and 'next' have ancestry relationship,\n> we could try to see if the three commits on 'topic' branch being\n> replayed match any of the commits in next..master range, but when\n> using the --onto option, there does not have to be any relationship\n> between the and (they do not have to share a\n> root commit). So from that point of view, it probably makes sense\n> to default to --no-reapply-cherry-picks when --onto is used, while\n> defaulting --reapply-cherry-picks when --onto is not used.\n\nI don't actually like this.\n\nFirst, in general, changing default of another option is not to be taken\nlightly. For example, defaulting to --fork-point when no is\nspecified is already a point of confusion.\n\nSecond, changing the default is not backward compatible, so there should\nbe very sound reason to change it.\n\nFinally, if user does specify /both/ --onto and\n--no-reapply-cherry-picks, where would Git supposedly stop digging for\nmatching cherry-picks? Provided this is to be solved anyway, the\nrationale to change the default does not sound strong enough.\n\nOverall, it seems that we should take the rather than\n (that is still when --onto is not specified), and\napply the skipping logic from there, to whatever depth the merge-base\nwill give us. If it's already implemented this way, then only the manual\npage needs to be fixed.\n\nThanks,\n-- \nSergey Organov\n"},{"id":"430633","messageId":"xmqqczrdhu9q.fsf@gitster.g","threadId":"56126","inReplyTo":"87a6mhgxv4.fsf@osv.gnss.ru","subject":"Re: why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-20T15:36:33Z","receivedAt":"2021-07-20T15:47:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sergey Organov writes:\n\n> Similar problem should exist for explicitly specified that\n> might happen to have little in common with the current , right?\n\nI do not think so. Plain-vanilla rebase is to carry forward our\nchanges on top of updated upstream, which means that there is\n\n x--x--x (side)\n /\n ---o---o---o---o---o---o (upstream)\n ^\n (old upstream)\n\ninherently ancestry relationship between the old upstream and the\ncurrent upstream when rebasing 'side' to 'upstream'.\n\n> I don't actually like this.\n\nYou do not have to ;-) because I was not suggesting to change any\nexisting behaviour. It was merely me thinking aloud, how I might\ndo the feature if I were designing it from scratch now.\n\n> Overall, it seems that we should take the rather than\n> (that is still when --onto is not specified), and\n> apply the skipping logic from there, to whatever depth the merge-base\n> will give us. If it's already implemented this way, then only the manual\n> page needs to be fixed.\n\nSounds sensible. I didn't check what the actual code does ;-)\n"},{"id":"430637","messageId":"87y2a19bp9.fsf@osv.gnss.ru","threadId":"56126","inReplyTo":"xmqqczrdhu9q.fsf@gitster.g","subject":"Re: why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-07-20T16:44:50Z","receivedAt":"2021-07-20T16:45:57Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Junio C Hamano writes:\n\n> Sergey Organov writes:\n>\n\n[...]\n\n>\n>> I don't actually like this.\n>\n> You do not have to ;-) because I was not suggesting to change any\n> existing behaviour. It was merely me thinking aloud, how I might\n> do the feature if I were designing it from scratch now.\n\nYeah, I've noticed the absence of [PATCH] ;-) and was just thinking\naloud myself as well.\n\nThanks,\n-- \nSergey Organov\n"},{"id":"431649","messageId":"CAJcwCMPaX4qTBfxG6UwswFXCAkGE1meX8GOFNQueKXmfHPMJ2Q@mail.gmail.com","threadId":"56126","inReplyTo":"xmqqmtqij63t.fsf@gitster.g","subject":"Re: why \"git rebase\" searching the duplicate patches in rather than in ?","fromName":"Andy Zhang","fromEmail":"zhgdrx@gmail.com","sentAt":"2021-08-01T08:59:27Z","receivedAt":"2021-08-01T08:56:51Z","isPatch":false,"sender":{"key":"zhgdrx@gmail.com","avatar":null},"body":"On Tue, Jul 20, 2021 at 6:23 AM Junio C Hamano wrote:\n>\n> Andy Zhang writes:\n>\n> > why \"git rebase\" searching the duplicate patches in > branch> rather than in ?\n> >\n> > hi, all:\n> >\n> > I am reading the help of \"git rebase\", it says:\n> > \"If the upstream branch already contains a change you have made\n> > (e.g., because you mailed a patch which was applied upstream), then\n> > that commit will be skipped. \"\n> >\n> > But, because we are applying commits to rather than\n> > to , I really don't understand why we are searching\n> > the duplicate patches in rather than in > branch>?\n>\n> It is either a design bug or a documentation bug, or both ;-)\n>\nIt should NOT be a documentation bug, because, in my experiment, I\nobserve that, \"git\"'s behavior is exactly the same as documentation.\nSo, it should be \"work as design\".\n\n> I do think it makes sense to skip commits from the branch we are\n> rebasing that have equivalent commits in the upstream, as it is\n> expected that upstream might have already applied/cherry-picked some\n> of the changes you are rebasing, and you do not want to use the same\n> change twice.\n>\n> When we are transplanting a series of commits from an old base to\n> totally unrelated base using the --onto option, e.g. when replaying\n> the contents of 'topic' relative to 'next' down to 'master' in your\n> topology, however,\n>\n> > Old tree is:\n> >\n> > o---o---o---o---o master\n> > \\\n> > o---o---o---o---o next\n> > \\\n> > o---o---o topic\n>\n> it is not necessarily obvious where to stop digging back at. In the\n> above picture where 'master' and 'next' have ancestry relationship,\n> we could try to see if the three commits on 'topic' branch being\n> replayed match any of the commits in next..master range, but when\n> using the --onto option, there does not have to be any relationship\n> between the and (they do not have to share a\n> root commit). So from that point of view, it probably makes sense\n> to default to --no-reapply-cherry-picks when --onto is used, while\n> defaulting --reapply-cherry-picks when --onto is not used.\n>\n>\n"}]}