Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Sergey, > [...] > In the parlance of your RFC v2, where you start with this history (which I > translated into the left-to-right notation that is used in pretty much all > of Git's own documentation about interactive rebases, which you apparently > either did not read, or chose *not* to imitate, creating yet another > unnecessary diversion): > > > - B1 > \ > - B2 - M First, it should rather be: - B1 \ M / - B2 as RFC presents essentially symmetric approach and I'd like it to be explicit. Representation in RFC simply saves some vertical space: M / \ B1 B2 Another reason to use it is that I liked to somehow indicate that this is about abstract DAG representation of Git history, not to be confused with some actual practical Git history. So that, for example, the reader can't be tempted to even try to assume that M has been necessarily created by "git merge" operation in the first place. That said, I'm sorry if it upsets you. I'll stick to your preferred notation below. > > You now insert U1 and U2 with trees identical to M: > > - B1 - U1 > \ > - B2 - U2 - M - B1 - U1 \ UM / - B2 - U2 _YES_. You've slightly screwed RFC as UM is not M anymore, having different parents, but otherwise it's still right. > So U1 is essentially B2 cherry-picked on top of B1, and U2 is essentially > B1 cherry-picked on top of B2. _NO_. No any cherry-picking has been involved, and I see absolutely no reason to pretend there has, except to intentionally make otherwise simple thing look tricky. U1 tree is still M tree, and U2 tree is still M tree, and UM tree is still M tree. That's what actually matters from RFC POV. > These U1/U2 commits are now to be cherry-picked on top of the rebased B1' > and B2'. I spare you more diagrams, you get the idea. _YES_. Exactly 2 cherry-picks. > Now, the changes in U1/U2 *are* the changes of the merge parents, that's > how they were constructed. Either _YES_, or _NO_, depending on the exact meaning of the term "the changes of the merge parents" you've used, but I suspect it's _NO_, taking into account your further inferences. The U1/U2 are constructed by simply duplicating the tree of the original merge commit M and thus they represent the changes _to_ the merge parents B1/B2 introduced by M, and not the changes "_of_ the merge parents" B1/B2, provided the latter meant to have some relation to the changes introduced by the merge parents B1/B2 themselves. > > Since they repeat what B1 and B2 are about, _NO_, they do not repeat what B1 and B2 are about at all. They rather represent what M is about. In other words, whatever B1 and B2 are about, the RFC method doesn't care. And as this is fundamental misinterpretation of the RFC on your side, it starts to be big _NO_ from now on... > and since B1'/B2' means they are rebased, and since U1'/U2' are *also* > rebased, but independently... > > ... you essentially have to rebase *every parent's changes > twice*. _NO_. U1' is rebase of U1 (on top of B1'), and U2' is rebase of U2 (on top of B2'). Each of U1/U2 is rebased only once. > The answer "No" to this is... astonishing. It's still _NO_, sorry. In fact, I could have said _NO_ the first time you started to assign some arbitrary "meaning" to the commits, as RFC is about somewhat formal proof of the method, using already well-known operations on the DAG, and to criticize the RFC, you need to either find and show a _formal_ mistake somewhere in the proof logic, or to show a use-case where it fails, as you did for RFC v1. Assigning arbitrary "meaning" to the DAG nodes and operations on them won't do the trick, sorry. I'd like to reach some agreement on formal correctness of the RFC first, and then discuss the meanings, the implementations, and other consequences based on well-established formal base. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Sergey, > > On Wed, 28 Mar 2018, Sergey Organov wrote: > >> Johannes Schindelin writes: >> >> > I use rebase every day. I use the Git garden shears every week. If you >> > do not trust my experience with these things, nothing will convince >> > you. >> >> Unfortunately you have exactly zero experience with rebasing merges as >> you've never actually rebased them till now, and it's rebasing merges >> that matters in this particular discussion. > > Who says that I have 0 experience with that? Oh yes, you do. Like, as if > you know. I just didn't see even single symptom of it in the discussion, still I said nothing about it until you started to use your presumed experience in place of true arguments. > Guess what I do with those Git garden shears' merges? Can you guess? Of > course you can. But you'll never know until I tell you. It is a little > silly to try to tell me that I do not have any experience with rebasing > merges when you have no idea what strategies I tried in the past. Please notice that I never even started to discuss your 'merge' directive, exactly because I believe you have huge experience both implementing and using it that could be relied upon. Just don't mix-in rebasing merge commits into it, as that is fundamentally different operation. And the other unspoken strategies you tried are irrelevant here, as you declined the whole idea of replaying merge-the-commit instead of replaying merge-the-operation until recently, and it seems you still do, at least to some level, by attempting to use 'merge' operation to replay "the (merge) _commit_". I'm afraid that whatever you've tried in the past likely suffered from the same conceptual confusion, and thus did not work indeed. That said, negative experience is still an experience, often helpful, but what is relevant here is that it likely was not about rebasing merge commits at all, so trying to use this irrelevant experience in the discussion to support your arguments, or rather lack of them, seems even more unfair to me then using relevant experience for that purpose. > Now, Phillip's strategy is clearly the best strategy I ever heard > about, For 'pick' vs 'merge', it's not about strategy, it's about concept. It looks like you still believe that you somehow "merge" a merge commit when you actually rebase it (with Phillip's strategy if you wish). You don't merge it anywhere, period. > and I am in the process of doing Actual Work to Put It To The Test. That's the best outcome of the discussion I ever hoped for, seriously, and I'll be all ears to listen to the outcomes of the experience you will gain with it. BTW, when you have it working, use the strategy you've implemented for non-merge commits as well, as a good one should still work fine. Moreover, it should better bring exactly the same results as the default existing strategy being used for non-merge commits, even in conflicting situations. Hopefully /that/ experience will finally push you strong enough to get the concept right, and you will finally understand that what you've implemented is nothing else but a _cherry-pick_ of a _merge commit_, that reads simply _pick_, for brevity. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Sergey, > > On Fri, 30 Mar 2018, Sergey Organov wrote: > >> Could we please agree to stop using backward compatibility as an >> objection in the discussion of the --recreate-merges feature? > > No. > > The expectation of users as to what a `pick` is has not changed just > because you wish it would. As if I ever suggested to change user expectations. Could you please stop putting words into my mouth? I _am_ a user, and I expect 'pick' to pick commits, no matter how many parents they might have. And no, --preserve-merges did not ever pick commits with number of parents more than one, it rather threw them away and re-merged the heads. Calling it 'pick' was a huge mistake indeed! Fixing that mistake is what I expect, as a user. Just teach the 'pick' to correctly pick any commit, please! > > That is a matter of backwards-compatibility. OK, fine, at least its only about user expectations and not about some scripting incompatibility. > You see, if you are driving a car for a hundred years already, and then > switch to a different car, and it has a lever in the same place as your > previous car's windshield wiper, but in the new car it has a button that > activates the emergency driver seat ejection OMG *it has a seat ejection > like in the James Bond movies! Where can I get that car?* Sorry for > disgressing. Except it's irrelevant as the 'pick' will still pick commits. > I am really concerned about that willingness to put an innocuous button, > so to speak, onto something users got really used to, over the course of a > decade or so, when that button should really be made red and blinking and > OMG where can I get that car? It's irrelevant as the 'pick' will still pick commits. > So to reiterate, I am really interested in a practical solution that won't > cause nasty surprises. I rather don't see how it possibly could cause any surprises, especially compared to using 'merge' to pick commits. > Meaning: `pick` != merge. Exactly! Use 'merge' when you merge, as you are already doing. Use 'pick' when you are picking. You don't merge "merge commit" when you are picking it! > That was a mistake in preserve-merges, as I have only mentioned like a > hundred times, and we won't repeat it. The mistake was that it used 'pick' to denote re-merge. You already fixed that mistake by introducing 'merge' to re-merge, thanks God. Please don't commit yet another mistake by now using 'merge' to pick! -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On Wed, 28 Mar 2018, Sergey Organov wrote: > Johannes Schindelinwrites: > > > I use rebase every day. I use the Git garden shears every week. If you > > do not trust my experience with these things, nothing will convince > > you. > > Unfortunately you have exactly zero experience with rebasing merges as > you've never actually rebased them till now, and it's rebasing merges > that matters in this particular discussion. Who says that I have 0 experience with that? Oh yes, you do. Like, as if you know. Guess what I do with those Git garden shears' merges? Can you guess? Of course you can. But you'll never know until I tell you. It is a little silly to try to tell me that I do not have any experience with rebasing merges when you have no idea what strategies I tried in the past. Now, Phillip's strategy is clearly the best strategy I ever heard about, and I am in the process of doing Actual Work to Put It To The Test. > > You are just stuck with your pre-existing opinion. > > I'm afraid that it's rather your huge experience with re-creating merges > that makes you stuck to your pre-existing opinion and carefully shields > you from experiencing actual paradigm shift. You know what? Whatevs. Ciao, Johannes
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On Fri, 30 Mar 2018, Sergey Organov wrote: > Could we please agree to stop using backward compatibility as an > objection in the discussion of the --recreate-merges feature? No. The expectation of users as to what a `pick` is has not changed just because you wish it would. That is a matter of backwards-compatibility. You see, if you are driving a car for a hundred years already, and then switch to a different car, and it has a lever in the same place as your previous car's windshield wiper, but in the new car it has a button that activates the emergency driver seat ejection OMG *it has a seat ejection like in the James Bond movies! Where can I get that car?* Sorry for disgressing. I am really concerned about that willingness to put an innocuous button, so to speak, onto something users got really used to, over the course of a decade or so, when that button should really be made red and blinking and OMG where can I get that car? So to reiterate, I am really interested in a practical solution that won't cause nasty surprises. Meaning: `pick` != merge. That was a mistake in preserve-merges, as I have only mentioned like a hundred times, and we won't repeat it. Now back to that important question: where can I get such a James Bond car? Ideally also with Turbo Boost. Oh wait, that was somebody else's car. Ciao, Johannes
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi, > > On Thu, 29 Mar 2018, Sergey Organov wrote: > >> Jacob Keller writes: >> >> > I care about the general compatibility of the rebase todo list >> > regardless of which options you enabled on the command line to >> > generate it. >> >> It's a good thing in general, yes. However, I recall I was told by the >> author that --recreate-merges was introduced exactly to break backward >> compatibility of the todo list. If so, could we please agree to stop >> using backward compatibility as an objection in the discussion of this >> particular feature? > > That is a serious misrepresentation of what I said. > > If I had changed --preserve-merges to the new format, *that* would have > broken backwards-compatibility. > > So the entire reason of introducing --recreate-merges was to *not have to > break backwards-compatibility*. > > I definitely did not say the *exact opposite*. I'm sorry I committed ambiguity in my wording that allowed it to be misinterpreted. I actually intended to say roughly the same thing you are saying, as what matters for the discussion is that new todo list format does not need to be (backward-)compatible to that of --preserve-merges. > Hopefully this clarifies your confusion, There was actually no confusion on my side, and I like your wording better. Except that you've managed to clarify your intentions without actually addressing the primary concern: Could we please agree to stop using backward compatibility as an objection in the discussion of the --recreate-merges feature? Could we? I understand you are still resistant to change 'pick' syntax, but it's not because of backward-compatibility, right? -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi, On Thu, 29 Mar 2018, Sergey Organov wrote: > Jacob Kellerwrites: > > > I care about the general compatibility of the rebase todo list > > regardless of which options you enabled on the command line to > > generate it. > > It's a good thing in general, yes. However, I recall I was told by the > author that --recreate-merges was introduced exactly to break backward > compatibility of the todo list. If so, could we please agree to stop > using backward compatibility as an objection in the discussion of this > particular feature? That is a serious misrepresentation of what I said. If I had changed --preserve-merges to the new format, *that* would have broken backwards-compatibility. So the entire reason of introducing --recreate-merges was to *not have to break backwards-compatibility*. I definitely did not say the *exact opposite*. Hopefully this clarifies your confusion, Johannes
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Jacob Kellerwrites: > On Wed, Mar 28, 2018 at 4:29 AM, Sergey Organov wrote: > >> Jacob Keller writes: >> >> > On Tue, Mar 27, 2018 at 10:57 PM, Sergey Organov >> wrote: >> >> >> >> Hi Johannes, >> >> >> >> Johannes Schindelin writes: >> >> > Hi Sergey, >> >> > >> >> >> >> [...] >> >> >> >> >> >> Reusing existing concepts where possible doesn`t have this >> problem. >> >> >> > >> >> >> > Existing concepts are great. As long as they fit the requirements >> of >> >> >> > the new scenarios. In this case, `pick` does *not* fit the >> > requirement >> >> >> > of "rebase a merge commit". >> >> >> >> >> >> It does, provided you use suitable syntax. >> >> > >> >> > You know what `pick` would also do, provided you use suitable syntax? >> > Pick >> >> > your nose. >> >> > >> >> > Don't blame me for this ridiculous turn the discussion took. >> >> > >> >> > Of course, using the suitable syntax you can do anything. Unless there >> > is >> >> > *already* a syntax and you cannot break it for backwards-compatibility >> >> > reasons, as is the case here. >> >> >> >> Backward compatibility to what? To a broken '--preserve-merges'? I had a >> >> feel you've invented '--recreate-merges' exactly to break that >> >> compatibility. No? >> >> >> >> Or is it "Backwards compatibility of a feature that existed only as a >> >> topic branch in `next` before being worked on more?", as you say >> >> yourself below? >> >> >> > >> > I'm pretty sure he meant that changing the meaning and behavior of "pick" >> > is incompatible, as people use scripts which check the edit lists, and >> > these scripts would expect pick to behave in a certain way. >> >> Are we still speaking about that new --recreate-merges feature? You >> already care for compatibility for it? You expect there are already >> scripts that use it? >> >> Once again, it seems like you care and don't care about backward >> compatibility at the same time, here is your phrase below: >> >> "He absolutely cares about compatibility, but in this case, the feature >> has not yet been merged into an official release." >> >> Are we still speaking about that new --recreate-merges feature? >> >> Do you guys care for compatibility for this particular --recreate-merges >> feature or not? I'm lost. "Yes" or "No" answer, if you please! >> >> -- Sergey >> > > I care about the general compatibility of the rebase todo list regardless > of which options you enabled on the command line to generate it. It's a good thing in general, yes. However, I recall I was told by the author that --recreate-merges was introduced exactly to break backward compatibility of the todo list. If so, could we please agree to stop using backward compatibility as an objection in the discussion of this particular feature? > Yes this has a bit of problem because *any* new todo command will > break the todo list, but it's better to only add new commands rather > than change semantics of existing ones. I'm not against new commands in general. I'm against inventing new entities without necessity, so, provided we did agree not to care about compatibility, there should be some other necessity to invent yet another command. I don't see such a necessity. Do you? The main principle I stand for in this discussion though is that all the commits should be treated equally as much as possible, new command or no new command. Doing otherwise will lead to all kinds of troubles and confusion, both in implementation and in user experience. Overall, what I think is needed is extending the syntax of existing todo commands to handle merge commits. This will give a provision to get it right this time. Otherwise it will likely end up being yet another subject of deprecation in the future. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Jacob Kellerwrites: > On Tue, Mar 27, 2018 at 10:57 PM, Sergey Organov wrote: >> >> Hi Johannes, >> >> Johannes Schindelin writes: [...] > I'm pretty sure the fact has already been accepted, as he did indeed > implement and develop a strategy for rebasing the merges (Phillip's > strategy). He hasn't chosen to re-write all the code such that it was > "always" this method, but rather kept it as an incremental patch on top as > it makes it easier to review the changes since we've already spent time > looking at and reviewing the --recreate-merges patches. That's perfectly OK with me, except that he apparently still can't accept the fact that rebasing a non-merge is not fundamentally different from rebasing a merge. "Rebase non-merge" is just a special case of generic "rebase commit", provided we do have generic method that is capable to rebase any commit, and we do have it, Phillip's or not. > Having watched from the sidelines, I've been unable to completely > understand and parse the strategies completely, but I've also found > Phillip's method to be easier to understand. It doesn't matter at all for this particular discussion. Let's call the method "rebase a commit", a black-box, that is capable to rebase any commit. I don't care what implementation is inside. Rebasing a commit is still rebasing a commit, and it should not be called "merge" in the todo list. > As someone who's read the discussion on the sidelines, it certainly > does feel like there is some misunderstanding on both sides. Neither > of you have been able to get the other to see what you clearly both > believe strongly. Calling "rebase" operation "merge" is wrong no matter what method is used to rebase a commit. Isn't it obvious? It's currently called "pick" in the todo and it seems natural to continue to use that name for picking a commit, whatever number of parents it happens to have. > Unfortunately I do not have any suggestion as to how to resolve the > misunderstanding. This sub-thread is not about method at all, so no resolution on that matter is required here. This sub-thread is about todo format only. > Sergey's method appears to me to be more complex, and I agree that the > extra steps could cause more merge conflicts, at least in how it was > originally conceptualized and implemented. It is possible that we are > mis-understanding the terminology for U1 and U2? It sure seems like it > introduces more changes for merge conflicts than the strategy proposed by > Phillip. However, the latest editions also sound a lot closer to Phillip's > strategy in general, so maybe I have mis-understood how it works and what > is fundamentally different about the two strategies. There is nothing fundamentally different between them and thus I don't care in this discussion what exact method is being used. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Jacob Kellerwrites: > On Tue, Mar 27, 2018 at 10:57 PM, Sergey Organov wrote: >> >> Hi Johannes, >> >> Johannes Schindelin writes: >> > Hi Sergey, >> > >> >> [...] >> >> >> >> Reusing existing concepts where possible doesn`t have this problem. >> >> > >> >> > Existing concepts are great. As long as they fit the requirements of >> >> > the new scenarios. In this case, `pick` does *not* fit the > requirement >> >> > of "rebase a merge commit". >> >> >> >> It does, provided you use suitable syntax. >> > >> > You know what `pick` would also do, provided you use suitable syntax? > Pick >> > your nose. >> > >> > Don't blame me for this ridiculous turn the discussion took. >> > >> > Of course, using the suitable syntax you can do anything. Unless there > is >> > *already* a syntax and you cannot break it for backwards-compatibility >> > reasons, as is the case here. >> >> Backward compatibility to what? To a broken '--preserve-merges'? I had a >> feel you've invented '--recreate-merges' exactly to break that >> compatibility. No? >> >> Or is it "Backwards compatibility of a feature that existed only as a >> topic branch in `next` before being worked on more?", as you say >> yourself below? >> > > I'm pretty sure he meant that changing the meaning and behavior of "pick" > is incompatible, as people use scripts which check the edit lists, and > these scripts would expect pick to behave in a certain way. Are we still speaking about that new --recreate-merges feature? You already care for compatibility for it? You expect there are already scripts that use it? Once again, it seems like you care and don't care about backward compatibility at the same time, here is your phrase below: "He absolutely cares about compatibility, but in this case, the feature has not yet been merged into an official release." Are we still speaking about that new --recreate-merges feature? Do you guys care for compatibility for this particular --recreate-merges feature or not? I'm lost. "Yes" or "No" answer, if you please! -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Sergey, > [...] >> >> Reusing existing concepts where possible doesn`t have this problem. >> > >> > Existing concepts are great. As long as they fit the requirements of >> > the new scenarios. In this case, `pick` does *not* fit the requirement >> > of "rebase a merge commit". >> >> It does, provided you use suitable syntax. > > You know what `pick` would also do, provided you use suitable syntax? Pick > your nose. > > Don't blame me for this ridiculous turn the discussion took. > > Of course, using the suitable syntax you can do anything. Unless there is > *already* a syntax and you cannot break it for backwards-compatibility > reasons, as is the case here. Backward compatibility to what? To a broken '--preserve-merges'? I had a feel you've invented '--recreate-merges' exactly to break that compatibility. No? Or is it "Backwards compatibility of a feature that existed only as a topic branch in `next` before being worked on more?", as you say yourself below? [...] >> > The implementation detail is, of course, that I will introduce this with >> > the technically-simpler strategy: always recreating merge commits with the >> > recursive strategy. A follow-up patch series will add support for rebasing >> > merge commits, and then use it by default. >> >> Switching to use it by default would be backward incompatible again? Yet >> another option to obsolete? Sigh. > > Oh wow. > > Backwards compatibility of a feature that existed only as a topic branch > in `next` before being worked on more? Any other splendid ideas? Either you care about compatibility or not. You can't have it both ways, sorry. And "technically-simpler strategy: always recreating merge commits with the recursive strategy" vs. "rebasing merge commits" is not just a minor strategy change, it's entire paradigm shift in handling merge commits while rebasing. I'm afraid you will still come up with a wrong design unless you finally accept this fact. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Sergey, [...] > But I'll stop here. Even my account how there are conceptual differences > between the changes in merge vs non-merge commits (the non-merge commit > *introduces* changes, the merge commit *reconciles existing* changes) > seems to fly by without convincing you. Good for you, but Git should keep caring about content, it should care not about meaning. Please leave it to the user to assign meaning to their content. If you rather want a SCM that focuses on meaning, I'd suggest to look at Bzr and see how it goes. > I use rebase every day. I use the Git garden shears every week. If you do > not trust my experience with these things, nothing will convince you. Unfortunately you have exactly zero experience with rebasing merges as you've never actually rebased them till now, and it's rebasing merges that matters in this particular discussion. > You are just stuck with your pre-existing opinion. I'm afraid that it's rather your huge experience with re-creating merges that makes you stuck to your pre-existing opinion and carefully shields you from experiencing actual paradigm shift. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On Tue, 27 Mar 2018, Sergey Organov wrote: > Johannes Schindelinwrites: > > > On Tue, 13 Mar 2018, Igor Djordjevic wrote: > > > >> On 12/03/2018 11:46, Johannes Schindelin wrote: > >> > > >> > > Sometimes one just needs to read the manual, and I don`t really > >> > > think this is a ton complicated, but just something we didn`t > >> > > really have before (real merge rebasing), so it requires a moment > >> > > to grasp the concept. > >> > > >> > If that were the case, we would not keep getting bug reports about > >> > --preserve-merges failing to reorder patches. > >> > >> Not sure where that is heading to, but what I`m arguing about is that > >> introducing new commands and concepts (`merge`, and with `-R`) just > >> makes the situation even worse (more stuff to grasp). > > > > The problem with re-using `pick` is that its concept does not apply to > > merges. The cherry-pick of a non-merge commit is well-defined: the > > current HEAD is implicitly chosen as the cherry-picked commit's > > (single) parent commit. There is no ambiguity here. > > > > But for merge commits, we need to specify the parent commits (apart > > from the first one) *explicitly*. There was no need for that in the > > `pick` command, nor in the concept of a cherry-pick. > > > >> Reusing existing concepts where possible doesn`t have this problem. > > > > Existing concepts are great. As long as they fit the requirements of > > the new scenarios. In this case, `pick` does *not* fit the requirement > > of "rebase a merge commit". > > It does, provided you use suitable syntax. You know what `pick` would also do, provided you use suitable syntax? Pick your nose. Don't blame me for this ridiculous turn the discussion took. Of course, using the suitable syntax you can do anything. Unless there is *already* a syntax and you cannot break it for backwards-compatibility reasons, as is the case here. But I'll stop here. Even my account how there are conceptual differences between the changes in merge vs non-merge commits (the non-merge commit *introduces* changes, the merge commit *reconciles existing* changes) seems to fly by without convincing you. I use rebase every day. I use the Git garden shears every week. If you do not trust my experience with these things, nothing will convince you. You are just stuck with your pre-existing opinion. > > If you really want to force the `pick` concept onto the use case where > > you need to "reapply" merges, then the closest you get really is > > Sergey's idea, which I came to reject when considering its practical > > implications. > > Which one, and what are the implications that are bad, I wonder? The strategy described in RFC v2, which does too much work, forces the user to potentially address the same merge conflicts multiple times, and worst of all: risks merge conflicts with changes the user *already* dropped. > > Even so, you would have to make the `pick` command more complicated to > > support merge commits. And whatever you would do to extend the `pick` > > command would *not make any sense* to the current use case of the `pick` > > command. > > It would rather make a lot of sense. Please don't use 'merge' to pick > commits, merge ones or not! It would rather make a lot of sense. If you completely ignored everything I said about preserve-merges. If you ignored what I said about problems moving regular `pick` lines across merge commits. If you ignored all the experience I have with Git garden shears and that I tried really patiently for an impatient man to impart on you. > > The real problem, of course, is that a non-merge commit, when viewed > > from the perspective of the changes it introduced, is a very different > > beast than a merge commit: it does not need to reconcile changes, > > ever, because there is really only one "patch" to one revision. That > > is very different from a merge commit, whose changes can even disagree > > with one another (and in fact be resolved with changes disagreeing > > *yet again*)! > > You'd still 'pick' it though, not 'merge'. You don't merge "merge > commit", it makes no sense. It only makes perfect sense when you get rid > of original "merge commit" and re-merge from scratch, as you were doing > till now. No, you merge "merge head". And you use "merge commit"'s commit message. *That* makes sense. Picking a merge commit? Not so. What do you merge? The original merge commit's second parent? Or a rebased version thereof? What if that commit has been `pick`ed *twice*? No, you can repeat it all you want, it still does not make sense. Now that I think of the possiblity of picking the original parents multiple times, it does not even make theoretical sense. > > The implementation detail is, of course, that I will introduce this with > > the technically-simpler strategy: always recreating merge commits with the > > recursive strategy. A follow-up patch series will add support for rebasing > > merge
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On Tue, 27 Mar 2018, Sergey Organov wrote: > Johannes Schindelinwrites: > > > > On Tue, 13 Mar 2018, Igor Djordjevic wrote: > > > >> On 12/03/2018 13:56, Sergey Organov wrote: > >> > > >> > > > I agree with both of you that `pick ` is inflexible > >> > > > (not to say just plain wrong), but I never thought about it like > >> > > > that. > >> > > > > >> > > > If we are to extract further mentioned explicit old:new merge > >> > > > parameter mapping to a separate discussion point, what we`re > >> > > > eventually left with is just replacing this: > >> > > > > >> > > > merge -R -C > >> > > > > >> > > > ... with this: > >> > > > > >> > > > pick > >> > > > >> > > I see where you are coming from. > >> > > > >> > > I also see where users will be coming from. Reading a todo list in > >> > > the editor is as much documentation as it is a "program to execute". > >> > > And I am afraid that reading a command without even mentioning the > >> > > term "merge" once is pretty misleading in this setting. > >> > > > >> > > And even from the theoretical point of view: cherry-picking > >> > > non-merge commits is *so much different* from "rebasing merge > >> > > commits" as discussed here, so much so that using the same command > >> > > would be even more misleading. > >> > > >> > This last statement is plain wrong when applied to the method in the > >> > [RFC] you are replying to. > > > > That is only because the RFC seems to go out of its way to break down a > > single merge commit into as many commits as there are merge commit > > parents. > > Complex entity is being split for ease of reasoning. People tend to use > this often. Sure. Divide and conquer. Not duplicate and complicate, though. > > This is a pretty convoluted way to think about it: if you have three > > parent commits, for example, that way of thinking would introduce three > > intermediate commits, one with the changes of parent 2 & 3 combined, one > > with the changes of parent 1 & 3 combined, and one with the changes of > > parent 1 & 2 combined. > > No. Sorry. This is unacceptable. If you disagree, sure, you are free to do that. If you want to contribute to a fruitful discussion, just saying "No" without explaining why you *think* that my statement is wrong is just... unconstructive. > > To rebase those commits, you essentially have to rebase *every > > parent's changes twice*. > > No. Same here. > > It gets worse with merge commits that have 4 parents. In that case, you > > have to rebase every parent's changes *three times*. > > Sorry, the [RFC] has nothing of the above. Once again, it's still just > as simple is: rebase every side of the merge then merge the results > using the original merge commit as a merge base. > > And if you can't or don't want to grok the explanation in the RFC, just > forget the explanation, no problem. Your RFC talks about U1 and U2, for the two merge parents. Obviously this strategy can be generalized to n parents. I thought you had thought of that and simply did not bother to talk about it. Sorry, my mistake. I should not assume so much. > > And so on. > > > >> > Using the method in [RFC], "cherry-pick non-merge" is nothing more or > >> > less than reduced version of generic "cherry-pick merge", exactly as > >> > it should be. > > > > I really get the impression that you reject Phillip's proposal on the > > ground of not being yours. In other words, the purpose of this here > > argument is to praise one proposal because of its heritage, rather than > > trying to come up with the best solution. > > No. As the discussion evolved, I inclined to conclusion that modified > Phillip's algorithm is actually better suited for the implementation > [1]. Again a link. If that's what you are looking for, I will throw a hundred links your way and see how constructive a discussion you find that. > > On that basis, I will go with the proposal that is clearly the simplest > > and does the job and gets away with avoiding unnecessary work. > > These algorithms are actually the same one, as has already been shown > elsewhere in the discussion. I disproved that already. My example showed that instead of reconciling the diverging changes starting from the original merge parents, RFC v2 tries to rebase those parents first, and then use the original merge commit as base of "diverging changes" that never started from that original merge commit. Essentially, where Phillip's strategy imitates a cherry-pick's 3-way merge, your strategy tries to rebase the merge tips independently from the user (who already rebased them, thank you very much), and then runs a *revert*: while a cherry-pick uses the picked commit's parent as merge base, a revert uses the to-be-reverted commit itself as merge base. In short: Phillip's strategy is only equivalent to yours if you ignore the fact that you perform unnecessary work only to undo it in the end. > Asymmetric incremental nature of
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Buga, > > On Tue, 13 Mar 2018, Igor Djordjevic wrote: > >> On 12/03/2018 11:46, Johannes Schindelin wrote: >> > >> > > Sometimes one just needs to read the manual, and I don`t really >> > > think this is a ton complicated, but just something we didn`t really >> > > have before (real merge rebasing), so it requires a moment to grasp >> > > the concept. >> > >> > If that were the case, we would not keep getting bug reports about >> > --preserve-merges failing to reorder patches. >> >> Not sure where that is heading to, but what I`m arguing about is that >> introducing new commands and concepts (`merge`, and with `-R`) just >> makes the situation even worse (more stuff to grasp). > > The problem with re-using `pick` is that its concept does not apply to > merges. The cherry-pick of a non-merge commit is well-defined: the current > HEAD is implicitly chosen as the cherry-picked commit's (single) parent > commit. There is no ambiguity here. > > But for merge commits, we need to specify the parent commits (apart from > the first one) *explicitly*. There was no need for that in the `pick` > command, nor in the concept of a cherry-pick. > >> Reusing existing concepts where possible doesn`t have this problem. > > Existing concepts are great. As long as they fit the requirements of the > new scenarios. In this case, `pick` does *not* fit the requirement of > "rebase a merge commit". It does, provided you use suitable syntax. > If you really want to force the `pick` concept onto the use case where > you need to "reapply" merges, then the closest you get really is > Sergey's idea, which I came to reject when considering its practical > implications. Which one, and what are the implications that are bad, I wonder? > Even so, you would have to make the `pick` command more complicated to > support merge commits. And whatever you would do to extend the `pick` > command would *not make any sense* to the current use case of the `pick` > command. It would rather make a lot of sense. Please don't use 'merge' to pick commits, merge ones or not! > The real problem, of course, is that a non-merge commit, when viewed from > the perspective of the changes it introduced, is a very different beast > than a merge commit: it does not need to reconcile changes, ever, because > there is really only one "patch" to one revision. That is very different > from a merge commit, whose changes can even disagree with one another (and > in fact be resolved with changes disagreeing *yet again*)! You'd still 'pick' it though, not 'merge'. You don't merge "merge commit", it makes no sense. It only makes perfect sense when you get rid of original "merge commit" and re-merge from scratch, as you were doing till now. >> > > Saying in favor of `--rebase-merges`, you mean as a separate option, >> > > alongside `--recreate-merges` (once that series lands)? >> > >> > No. I am against yet another option. The only reason I pollute the >> > option name space further with --recreate-merges is that it would be >> > confusing to users if the new mode was called --preserve-merges=v2 >> > (but work *totally differently*). >> >> I see. So I take you`re thinking about renaming `--recreate-merges` to >> `--rebase-merges` instead? > > Thinking about it. Nothing will happen before v2.17.0 on that front, > though, because -- unlike you gentle people -- I have to focus on > stabilizing Git's code base now. > >> That would seem sensible, too, I think, being the default usage mode in >> the first place. Being able to actually (re)create merges, too, once >> user goes interactive, would be "just" an additional (nice and powerful) >> feature on top of it. > > The implementation detail is, of course, that I will introduce this with > the technically-simpler strategy: always recreating merge commits with the > recursive strategy. A follow-up patch series will add support for rebasing > merge commits, and then use it by default. Switching to use it by default would be backward incompatible again? Yet another option to obsolete? Sigh. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Buga, > > On Tue, 13 Mar 2018, Igor Djordjevic wrote: > >> On 12/03/2018 13:56, Sergey Organov wrote: >> > >> > > > I agree with both of you that `pick ` is inflexible >> > > > (not to say just plain wrong), but I never thought about it like >> > > > that. >> > > > >> > > > If we are to extract further mentioned explicit old:new merge >> > > > parameter mapping to a separate discussion point, what we`re >> > > > eventually left with is just replacing this: >> > > > >> > > >merge -R -C >> > > > >> > > > ... with this: >> > > > >> > > >pick >> > > >> > > I see where you are coming from. >> > > >> > > I also see where users will be coming from. Reading a todo list in >> > > the editor is as much documentation as it is a "program to execute". >> > > And I am afraid that reading a command without even mentioning the >> > > term "merge" once is pretty misleading in this setting. >> > > >> > > And even from the theoretical point of view: cherry-picking >> > > non-merge commits is *so much different* from "rebasing merge >> > > commits" as discussed here, so much so that using the same command >> > > would be even more misleading. >> > >> > This last statement is plain wrong when applied to the method in the >> > [RFC] you are replying to. > > That is only because the RFC seems to go out of its way to break down a > single merge commit into as many commits as there are merge commit > parents. Complex entity is being split for ease of reasoning. People tend to use this often. > This is a pretty convoluted way to think about it: if you have three > parent commits, for example, that way of thinking would introduce three > intermediate commits, one with the changes of parent 2 & 3 combined, one > with the changes of parent 1 & 3 combined, and one with the changes of > parent 1 & 2 combined. No. > To rebase those commits, you essentially have to rebase *every parent's > changes twice*. No. > It gets worse with merge commits that have 4 parents. In that case, you > have to rebase every parent's changes *three times*. Sorry, the [RFC] has nothing of the above. Once again, it's still just as simple is: rebase every side of the merge then merge the results using the original merge commit as a merge base. And if you can't or don't want to grok the explanation in the RFC, just forget the explanation, no problem. > And so on. > >> > Using the method in [RFC], "cherry-pick non-merge" is nothing more or >> > less than reduced version of generic "cherry-pick merge", exactly as >> > it should be. > > I really get the impression that you reject Phillip's proposal on the > ground of not being yours. In other words, the purpose of this here > argument is to praise one proposal because of its heritage, rather than > trying to come up with the best solution. No. As the discussion evolved, I inclined to conclusion that modified Phillip's algorithm is actually better suited for the implementation [1]. > On that basis, I will go with the proposal that is clearly the simplest > and does the job and gets away with avoiding unnecessary work. These algorithms are actually the same one, as has already been shown elsewhere in the discussion. Asymmetric incremental nature of the Phillip's one is apparently better suited for naturally asymmetrical way Git already handles merging. FYI, here is the latest proposal that came out of discussion [1]: git-rebase-first-parent --onto A' M tree_U1'=$(git write-tree) git merge-recursive B -- $tree_U1' B' tree=$(git write-tree) M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB') [ $conflicted_last_merge = "yes" ] || trees-match $tree_U1' $tree || stop-for-user-amendment where 'git-rebase-first-parent' denotes whatever machinery is currently being used to rebase simple non-merge commit. > >> > Or, in other words, "cherry-pick merge" is generalization of >> > "cherry-pick non-merge" to multiple parents. >> >> I think Sergey does have a point here, his approach showing it. > > His approach is showing that he wants to shoehorn the "rebase a merge > commit" idea into a form where you can cherry-pick *something*. > > It does not have to make sense. And to me, it really does not. Except that Phillip's one does exactly this as well, only in incremental manner, as shown in [1]. > >> Phillip`s simplification might be further from it, though, but we`re >> talking implementation again - important mental model should just be >> "rebasing a commit" (merge or non-merge), how we`re doing it is >> irrelevant for the user, the point (goal) is the same. > > Except that Phillip's simplification is not a simplification. It comes > from a different point of view: trying to reconcile the diverging > changes. They are essentially the same as one easily converts to another and back [1]. They will only bring different user experience in case of conflicts. > Phillip's is a true generalization of the
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On Wed, 14 Mar 2018, Sergey Organov wrote: > Igor Djordjevicwrites: > > > On 07/03/2018 08:26, Johannes Schindelin wrote: > > [...] > > >> Second side note: if we can fast-forward, currently we prefer that, > >> and I think we should keep that behavior with -R, too. > > > > I agree. > > I'm admittedly somewhat lost in the discussion, ... and cutting so much context does not help... The "fast-forward" here refers to the thing `git rebase` does unless you call it with `--force-rebase`: if we are about to `pick` a commit, and HEAD already points to its parent commit, we do not evey cherry-pick: we just fast-forward to the original commit. Likewise, `merge -C ` will simply fast-forward to the specified commit if HEAD is pointing to its first parent and the specified merge heads are also identical to the respective original parent commits. What I said with my comment was that `merge -R -C ` will behave in exactly the same way. > but are you talking fast-forward on _rebasing_ existing merge? Where > would it go in any of the suggested algorithms of rebasing and why? And this is something I did not talk about yet, but it does come up in practice: the main use case of rebasing branches is to follow an "upstream", of course. And guess what? From time to time, some of the branches get merged upstream (or, in Git's own source code, applied). In this case, the Git garden shears *do* skip the merge (as the new merge head is already an ancestor of HEAD). In --recreate-merges, it *will* create a new merge commit, though, which is a bit annoying. The best way to handle this would *probably* look similar to how "empty" commits (i.e. commits whose patch is empty) are handled by rebase. But it is not yet clear to me how that would look in practice, as `pick ` is a single operation that can be easily commented out in the todo list depending on `--allow-empty`, while the `merge -C ` command is not the entire operation, as there may be `pick` commands in the merge head that could potentially be skipped due to merge conflicts with already-applied versions of the same patches. If this sounds unclear to you, please do ask for clarification. Although this is currently not my highest priority in the `sequencer-shears` branch thicket (where `--recreate-merges` is the first part). > I readily see how it can break merges. E.g., any "git merge --ff-only > --no-ff" merge will magically disappear. Did you mean `git merge --no-ff`? Combining `--ff-only` with `--no-ff` does not make sense. > So, even if somehow supported, fast-forward should not be performed by > default during _rebasing_ of a merge. That statement is too general to be correct. *If* we detect that the original merge could have been fast-forwarded instead (and it is very easy to detect that in --make-list), we would have to handle that similar to afore-mentioned empty commits in conjunction with `--allow-empty`. If the original merge could not have been fast-forwarded, but during the rebase it *can*, we should skip it, as the reason for the merge commit is clearly no longer there. This, by the way, is an insight you can really only win by using --recreate-merges (or the Git garden shears). And using it a *lot*. Because otherwise, you will not even guess correctly what will, and what won't, come up in practice. > >> If the user wants to force a new merge, they simply remove that -R > >> flag. > > Alternatively, they'd replace 'pick' with 'merge', as they already do > for other actions. "A plurality is not to be posited without necessity". No, no, no and no again! We learned how *wrong* the `pick` approach was with --preserve-merges! Have you *ever* used --preserve-merges in any real way? If so, you *have* encountered the problems, and probably directed several lengthy curses my way for that lousy design. A pick is a pick is a pick. Of a single patch with metadata such as the author, commit message and the date. A merge is not a patch. While a pick introduces a specific change on top of a single revision, the changes introduced by a merge are ideally already there. It is conceptually *very* different. Sure, a merge commit sometimes needs to introduce extra changes on top of the merged changes, sure, that happens (and is called "evil merge" in Git parlance). Those *additional* changes should be kept as minimal as possible, in particular there should only be changes *necessitated* by the reconciled changes. So no, a `pick` is not a `merge`. Not at all. > Please, _please_, don't use 'merge' command to 'pick' merge commits! > It's utterly confusing! Please, please, please *work* with branch thickets for a while. And you will see that it makes a *huge* difference! For example, within a block of `pick` lines, it is relatively safe to reorder. You see more or less what changes interact with one another. Be *very* careful when reordering `merge` lines, in particular when you have a real-world branch thicket,
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Tue, 13 Mar 2018, Igor Djordjevic wrote: > On 12/03/2018 11:46, Johannes Schindelin wrote: > > > > > Sometimes one just needs to read the manual, and I don`t really > > > think this is a ton complicated, but just something we didn`t really > > > have before (real merge rebasing), so it requires a moment to grasp > > > the concept. > > > > If that were the case, we would not keep getting bug reports about > > --preserve-merges failing to reorder patches. > > Not sure where that is heading to, but what I`m arguing about is that > introducing new commands and concepts (`merge`, and with `-R`) just > makes the situation even worse (more stuff to grasp). The problem with re-using `pick` is that its concept does not apply to merges. The cherry-pick of a non-merge commit is well-defined: the current HEAD is implicitly chosen as the cherry-picked commit's (single) parent commit. There is no ambiguity here. But for merge commits, we need to specify the parent commits (apart from the first one) *explicitly*. There was no need for that in the `pick` command, nor in the concept of a cherry-pick. > Reusing existing concepts where possible doesn`t have this problem. Existing concepts are great. As long as they fit the requirements of the new scenarios. In this case, `pick` does *not* fit the requirement of "rebase a merge commit". If you really want to force the `pick` concept onto the use case where you need to "reapply" merges, then the closest you get really is Sergey's idea, which I came to reject when considering its practical implications. Even so, you would have to make the `pick` command more complicated to support merge commits. And whatever you would do to extend the `pick` command would *not make any sense* to the current use case of the `pick` command. The real problem, of course, is that a non-merge commit, when viewed from the perspective of the changes it introduced, is a very different beast than a merge commit: it does not need to reconcile changes, ever, because there is really only one "patch" to one revision. That is very different from a merge commit, whose changes can even disagree with one another (and in fact be resolved with changes disagreeing *yet again*)! > > > Saying in favor of `--rebase-merges`, you mean as a separate option, > > > alongside `--recreate-merges` (once that series lands)? > > > > No. I am against yet another option. The only reason I pollute the > > option name space further with --recreate-merges is that it would be > > confusing to users if the new mode was called --preserve-merges=v2 > > (but work *totally differently*). > > I see. So I take you`re thinking about renaming `--recreate-merges` to > `--rebase-merges` instead? Thinking about it. Nothing will happen before v2.17.0 on that front, though, because -- unlike you gentle people -- I have to focus on stabilizing Git's code base now. > That would seem sensible, too, I think, being the default usage mode in > the first place. Being able to actually (re)create merges, too, once > user goes interactive, would be "just" an additional (nice and powerful) > feature on top of it. The implementation detail is, of course, that I will introduce this with the technically-simpler strategy: always recreating merge commits with the recursive strategy. A follow-up patch series will add support for rebasing merge commits, and then use it by default. This latter part will need a lot of experimentation, though. That's why I want the --recreate-merges patch series cooking in `next` first. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Tue, 13 Mar 2018, Igor Djordjevic wrote: > On 12/03/2018 13:56, Sergey Organov wrote: > > > > > > I agree with both of you that `pick ` is inflexible > > > > (not to say just plain wrong), but I never thought about it like > > > > that. > > > > > > > > If we are to extract further mentioned explicit old:new merge > > > > parameter mapping to a separate discussion point, what we`re > > > > eventually left with is just replacing this: > > > > > > > > merge -R -C > > > > > > > > ... with this: > > > > > > > > pick > > > > > > I see where you are coming from. > > > > > > I also see where users will be coming from. Reading a todo list in > > > the editor is as much documentation as it is a "program to execute". > > > And I am afraid that reading a command without even mentioning the > > > term "merge" once is pretty misleading in this setting. > > > > > > And even from the theoretical point of view: cherry-picking > > > non-merge commits is *so much different* from "rebasing merge > > > commits" as discussed here, so much so that using the same command > > > would be even more misleading. > > > > This last statement is plain wrong when applied to the method in the > > [RFC] you are replying to. That is only because the RFC seems to go out of its way to break down a single merge commit into as many commits as there are merge commit parents. This is a pretty convoluted way to think about it: if you have three parent commits, for example, that way of thinking would introduce three intermediate commits, one with the changes of parent 2 & 3 combined, one with the changes of parent 1 & 3 combined, and one with the changes of parent 1 & 2 combined. To rebase those commits, you essentially have to rebase *every parent's changes twice*. It gets worse with merge commits that have 4 parents. In that case, you have to rebase every parent's changes *three times*. And so on. > > Using the method in [RFC], "cherry-pick non-merge" is nothing more or > > less than reduced version of generic "cherry-pick merge", exactly as > > it should be. I really get the impression that you reject Phillip's proposal on the ground of not being yours. In other words, the purpose of this here argument is to praise one proposal because of its heritage, rather than trying to come up with the best solution. On that basis, I will go with the proposal that is clearly the simplest and does the job and gets away with avoiding unnecessary work. > > Or, in other words, "cherry-pick merge" is generalization of > > "cherry-pick non-merge" to multiple parents. > > I think Sergey does have a point here, his approach showing it. His approach is showing that he wants to shoehorn the "rebase a merge commit" idea into a form where you can cherry-pick *something*. It does not have to make sense. And to me, it really does not. > Phillip`s simplification might be further from it, though, but we`re > talking implementation again - important mental model should just be > "rebasing a commit" (merge or non-merge), how we`re doing it is > irrelevant for the user, the point (goal) is the same. Except that Phillip's simplification is not a simplification. It comes from a different point of view: trying to reconcile the diverging changes. Phillip's is a true generalization of the "rebase vs merge" story: it is no longer about merging, or about rebasing, but about reconciling divergent commit histories, with whatever tool is appropriate. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Igor Djordjevicwrites: > Hi Sergey, > > On 19/03/2018 06:44, Sergey Organov wrote: >> >> > > > > > Second side note: if we can fast-forward, currently we prefer >> > > > > > that, and I think we should keep that behavior with -R, too. >> > > > > >> > > > > I agree. >> > > > >> > > > I'm admittedly somewhat lost in the discussion, but are you >> > > > talking fast-forward on _rebasing_ existing merge? Where would it >> > > > go in any of the suggested algorithms of rebasing and why? >> > > > >> > > > I readily see how it can break merges. E.g., any "git merge >> > > > --ff-only --no-ff" merge will magically disappear. So, even if >> > > > somehow supported, fast-forward should not be performed by default >> > > > during _rebasing_ of a merge. >> > > >> > > Hmm, now that you brought this up, I can only agree, of course. >> > > >> > > What I had in my mind was more similar to "no-rebase-cousins", like >> > > if we can get away without actually rebasing the merge but still >> > > using the original one, do it. But I guess that`s not what Johannes >> > > originally asked about. >> > > >> > > This is another definitive difference between rebasing (`pick`?) and >> > > recreating (`merge`) a merge commit - in the case where we`re rebasing, >> > > of course it doesn`t make sense to drop commit this time (due to >> > > fast-forward). This does make sense in recreating the merge (only). >> > >> > Eh, I might take this back. I think my original interpretation (and >> > agreement) to fast-forwarding is correct. >> > >> > But the confusion here comes from `--no-ff` as used for merging, as >> > opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant >> > the latter one. >> > >> > In rebasing, `--no-ff` means that even if a commit inside todo list >> > isn`t to be changed, do not reuse it but create a new one. Here`s >> > excerpt from the docs[1]: >> > >> > --no-ff >> > With --interactive, cherry-pick all rebased commits instead of >> > fast-forwarding over the unchanged ones. This ensures that the >> > entire history of the rebased branch is composed of new commits. >> > >> > Without --interactive, this is a synonym for --force-rebase. >> > >> > >> > So fast-forwarding in case of rebasing (merge commits as well) is >> > something you would want by default, as it wouldn`t drop/lose >> > anything, but merely reuse existing commit (if unchanged), instead of >> > cherry-picking (rebasing) it into a new (merge) commit anyway. >> >> This sounds like breakage. E.g., it seems to be breaking every "-x ours" >> merge out there. > > Either you are not understanding how rebase fast-forward works, or > I`m missing what you are pointing to... Mind explaining how can > something that`s left unchanged suddenly become a breakage? It was misunderstanding on my side indeed, sorry. > >> Fast-forwarding existing merge, one way or another, still seems to be >> wrong idea to me, as merge commit is not only about content change, but >> also about joint point at particular place in the DAG. > > Not sure what this has to do with rebase fast-forwarding, either - > nothing changes for fast-forwarded (merge or non-merge) commit in > question, both content, joint point and everything else stays exactly > the same. If anything changed, then it can`t/won`t be fast-forwarded, > being unchanged is a prerequisite. > > Let me elaborate a bit. Here`s a starting diagram: [... detailed explanation skipped for brevity ...] > Does this settle your concerns, or I`m missing something? Yes, it does, thank you! Leaving as many leading commits as possible unchanged during rebase is what fast-forward mean in this case then, and it's pretty OK with me. >> As for fast-forwarding re-merge, explicitly requested, I'm not sure. On >> one hand, it's inline with the default "git merge" behavior, on the >> other hand, it still feels wrong, somehow. > > Regarding fast-forwarding in context of merging, in case where we are > recreating merges (not rebasing them), following existing `git merge` > logic might make sense, where I would expect rebasing todo list `merge` > command to pick-up tricks from `git merge` as needed, like learning > to accept `--no-ff` option, for example, thus not fast-forwarding > merges (on request) even when possible. > > Though, I do agree that in case you want to recreate an existing merge > (instead of just rebasing it), `merge` command fast-forwarding might > probably not be what you want for the most of the time, but I`m afraid > having rebase todo list `merge` command default behavior different than > `git merge` default one (in regards to fast-forwarding) would be > confusing... or not? > > From what I could grasp so far, usually Git commands` default > behavior is (explained to be) chosen per "most common use case", so > might be non fast-forwarding would be fine as default for rebase todo > list `merge` command, even though different than
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On 19/03/2018 06:44, Sergey Organov wrote: > > > > > > > Second side note: if we can fast-forward, currently we prefer > > > > > > that, and I think we should keep that behavior with -R, too. > > > > > > > > > > I agree. > > > > > > > > I'm admittedly somewhat lost in the discussion, but are you > > > > talking fast-forward on _rebasing_ existing merge? Where would it > > > > go in any of the suggested algorithms of rebasing and why? > > > > > > > > I readily see how it can break merges. E.g., any "git merge > > > > --ff-only --no-ff" merge will magically disappear. So, even if > > > > somehow supported, fast-forward should not be performed by default > > > > during _rebasing_ of a merge. > > > > > > Hmm, now that you brought this up, I can only agree, of course. > > > > > > What I had in my mind was more similar to "no-rebase-cousins", like > > > if we can get away without actually rebasing the merge but still > > > using the original one, do it. But I guess that`s not what Johannes > > > originally asked about. > > > > > > This is another definitive difference between rebasing (`pick`?) and > > > recreating (`merge`) a merge commit - in the case where we`re rebasing, > > > of course it doesn`t make sense to drop commit this time (due to > > > fast-forward). This does make sense in recreating the merge (only). > > > > Eh, I might take this back. I think my original interpretation (and > > agreement) to fast-forwarding is correct. > > > > But the confusion here comes from `--no-ff` as used for merging, as > > opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant > > the latter one. > > > > In rebasing, `--no-ff` means that even if a commit inside todo list > > isn`t to be changed, do not reuse it but create a new one. Here`s > > excerpt from the docs[1]: > > > > --no-ff > > With --interactive, cherry-pick all rebased commits instead of > > fast-forwarding over the unchanged ones. This ensures that the > > entire history of the rebased branch is composed of new commits. > > > > Without --interactive, this is a synonym for --force-rebase. > > > > > > So fast-forwarding in case of rebasing (merge commits as well) is > > something you would want by default, as it wouldn`t drop/lose > > anything, but merely reuse existing commit (if unchanged), instead of > > cherry-picking (rebasing) it into a new (merge) commit anyway. > > This sounds like breakage. E.g., it seems to be breaking every "-x ours" > merge out there. Either you are not understanding how rebase fast-forward works, or I`m missing what you are pointing to... Mind explaining how can something that`s left unchanged suddenly become a breakage? > Fast-forwarding existing merge, one way or another, still seems to be > wrong idea to me, as merge commit is not only about content change, but > also about joint point at particular place in the DAG. Not sure what this has to do with rebase fast-forwarding, either - nothing changes for fast-forwarded (merge or non-merge) commit in question, both content, joint point and everything else stays exactly the same. If anything changed, then it can`t/won`t be fast-forwarded, being unchanged is a prerequisite. Let me elaborate a bit. Here`s a starting diagram: (1) ---X1---X2---X3 (master) |\ | A1---A2---A3 |\ | M---C1---C2 (topic) |/ \-B1---B2---B3 With "topic" being active branch, we start interactive rebase with `git rebase -i master`. Generated todo list will hold commits A1 to A3, B1 to B3, M and C1 to C2. Now, if we decide to `edit` commit C1, leaving everything else the same, fast-forward logic will make the new situation look like this: (2) ---X1---X2---X3 (master) |\ | A1---A2---A3 |\ | M---C1'--C2' (topic) |/ \-B1---B2---B3 Notice how only C1 and C2 changed to C1' and C2'? That`s rebase fast-forwarding, noticing earlier commits left unchanged, thus reusing original ones. No matter what, no breakage can happen to M in this case, as it`s left (reused) exactly as it was - it`s fast-forward rebased. If we `edit`-ed commit A2, we would have ended in a situation like this: (3) ---X1---X2---X3 (master) |\ | A1---A2'--A3' |\ | M'--C1'--C2' (topic) |/ \-B1---B2---B3 This time we have new commits A2', A3', M', C1' and C2' - so everything influenced by the change that happened will be changed (merge commit as well), where all the rest can still be reused (fast-forwarded). If we had started rebasing with `git rebase -i --no-ff master`, no matter which commits we `edit` (or none, even), we would end up with this instead:
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Igor Djordjevicwrites: > On 15/03/2018 00:11, Igor Djordjevic wrote: >> >> > > > Second side note: if we can fast-forward, currently we prefer >> > > > that, and I think we should keep that behavior with -R, too. >> > > >> > > I agree. >> > >> > I'm admittedly somewhat lost in the discussion, but are you >> > talking fast-forward on _rebasing_ existing merge? Where would it >> > go in any of the suggested algorithms of rebasing and why? >> > >> > I readily see how it can break merges. E.g., any "git merge >> > --ff-only --no-ff" merge will magically disappear. So, even if >> > somehow supported, fast-forward should not be performed by default >> > during _rebasing_ of a merge. >> >> Hmm, now that you brought this up, I can only agree, of course. >> >> What I had in my mind was more similar to "no-rebase-cousins", like >> if we can get away without actually rebasing the merge but still >> using the original one, do it. But I guess that`s not what Johannes >> originally asked about. >> >> This is another definitive difference between rebasing (`pick`?) and >> recreating (`merge`) a merge commit - in the case where we`re rebasing, >> of course it doesn`t make sense to drop commit this time (due to >> fast-forward). This does make sense in recreating the merge (only). > > Eh, I might take this back. I think my original interpretation (and > agreement) to fast-forwarding is correct. > > But the confusion here comes from `--no-ff` as used for merging, as > opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant > the latter one. > > In rebasing, `--no-ff` means that even if a commit inside todo list > isn`t to be changed, do not reuse it but create a new one. Here`s > excerpt from the docs[1]: > > --no-ff > With --interactive, cherry-pick all rebased commits instead of > fast-forwarding over the unchanged ones. This ensures that the > entire history of the rebased branch is composed of new commits. > > Without --interactive, this is a synonym for --force-rebase. > > > So fast-forwarding in case of rebasing (merge commits as well) is > something you would want by default, as it wouldn`t drop/lose > anything, but merely reuse existing commit (if unchanged), instead of > cherry-picking (rebasing) it into a new (merge) commit anyway. This sounds like breakage. E.g., it seems to be breaking every "-x ours" merge out there. Fast-forwarding existing merge, one way or another, still seems to be wrong idea to me, as merge commit is not only about content change, but also about joint point at particular place in the DAG. As for fast-forwarding re-merge, explicitly requested, I'm not sure. On one hand, it's inline with the default "git merge" behavior, on the other hand, it still feels wrong, somehow. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 15/03/2018 00:11, Igor Djordjevic wrote: > > > > > Second side note: if we can fast-forward, currently we prefer > > > > that, and I think we should keep that behavior with -R, too. > > > > > > I agree. > > > > I'm admittedly somewhat lost in the discussion, but are you > > talking fast-forward on _rebasing_ existing merge? Where would it > > go in any of the suggested algorithms of rebasing and why? > > > > I readily see how it can break merges. E.g., any "git merge > > --ff-only --no-ff" merge will magically disappear. So, even if > > somehow supported, fast-forward should not be performed by default > > during _rebasing_ of a merge. > > Hmm, now that you brought this up, I can only agree, of course. > > What I had in my mind was more similar to "no-rebase-cousins", like > if we can get away without actually rebasing the merge but still > using the original one, do it. But I guess that`s not what Johannes > originally asked about. > > This is another definitive difference between rebasing (`pick`?) and > recreating (`merge`) a merge commit - in the case where we`re rebasing, > of course it doesn`t make sense to drop commit this time (due to > fast-forward). This does make sense in recreating the merge (only). Eh, I might take this back. I think my original interpretation (and agreement) to fast-forwarding is correct. But the confusion here comes from `--no-ff` as used for merging, as opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant the latter one. In rebasing, `--no-ff` means that even if a commit inside todo list isn`t to be changed, do not reuse it but create a new one. Here`s excerpt from the docs[1]: --no-ff With --interactive, cherry-pick all rebased commits instead of fast-forwarding over the unchanged ones. This ensures that the entire history of the rebased branch is composed of new commits. Without --interactive, this is a synonym for --force-rebase. So fast-forwarding in case of rebasing (merge commits as well) is something you would want by default, as it wouldn`t drop/lose anything, but merely reuse existing commit (if unchanged), instead of cherry-picking (rebasing) it into a new (merge) commit anyway. The same goes for this part: > > > > If the user wants to force a new merge, they simply remove that > > > > -R flag. This means that using `-R` flag is sensitive to `--no-ff` rebase option, original merge commit _reused_ (fast-forwarded) when possible (unchanged, and `--no-ff` not provided), or original merge commit _rebased_ (when changed, or `--no-ff` provided). If `-R` flag is removed, then merge commit is always _recreated_, no matter if `--no-ff` option is used or not. p.s. I`m still a bit opposed to `-R` flag in the first place, as discussed elsewhere[2][3], but that`s unrelated to this fast-forward discussion. Related to it, though, if `pick` command would be used instead of `merge -R` to signal merge commit _rebasing_, it would fit into existing logic nicely, where `pick` is already sensitive to `--no-ff` option (for rebasing regular commits). Then `merge` alone could be naturally (and only) used for _recreating_ merge commits, as originally intended (and intuitively expected). Regards, Buga [1] https://git-scm.com/docs/git-rebase#git-rebase---no-ff [2] https://public-inbox.org/git/77b695d0-7564-80d7-d9e6-70a531e66...@gmail.com/ [3] https://public-inbox.org/git/a3d40dca-f508-5853-89bc-1f9ab3934...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Sergey, On 15/03/2018 07:00, Sergey Organov wrote: > > > > Thinking about it I've got an idea that what we actually need is > > > --no-flatten flag that, when used alone, will just tell "git rebase" to > > > stop flattening history, and which will be implicitly imposed by > > > --recreate-merges (and --preserve-merges). > > > > > > Then the only thing the --recreate-merges will tune is to put 'merge' > > > directives into the todo list for merge commits, exactly according to > > > what its name suggests, while the default behavior will be to put 'pick' > > > with suitable syntax into the todo. And arguments to the > > > --recreate-merge will specify additional options for the 'merge' > > > directive, obviously. > > > > This seem to basically boil down to what I mentioned previously[2] > > through use of new `--rebase-merges` alongside `--recreate-merges`, just > > that you named it `--no-flatten` here, but the point is the same - and > > not something Johannes liked, "polluting" rebase option space further. > > Not quite so. The problem with --XXX-merges flags is that they do two > things at once: they say _what_ to do and _how_ to do it. Clean UI > designs usually have these things separate, and that's what I propose. > > The --[no-]flatten says _what_ (not) to do, and --recreate-merges says > _how_ exactly it will be performed. In this model --no-flatten could > have been called, say --preserve-shape, but not --rebase-merges. > > To minimize pollution, the _how_ part could rather be made option value: > > --no-flatten[=] > > where is 'rebase', 'remerge', etc. > > In this case we will need separate option to specify strategy options, > if required, that will lead us to something similar to the set of merge > strategies options. > > > I would agree with him, and settling onto `--rebase-merges` _instead_ of > > `--recreate-merges` seems as a more appropriate name, indeed, now that > > default behavior is actually merge commit rebasing and not recreating > > (recreating still being possible through user editing the todo list). > > I hope he'd be pleased to be able to say --no-flatten=remerge and get > back his current mode of operation, that he obviously has a good use > for. Makes sense, I like it, thanks for elaborating. [ Especially that you used "(no) flatten" phrasing, where original `--preserve-merges` documentation says it`s used "not to flatten the history", nice touch ;) ] Not sure if I would prefer original `recreate` instead of `remerge` as that particular strategy name, though, but might be I`m just more used to the former one at the moment. But if I think about it more, "recreate" seems straightforward enough - create something (again), kind of making it more obvious that it is a new thing (that we are now creating, again, based on some old thing). On the other hand, "remerge" communicates "merge something again", which doesn`t necessarily mean creating a new thing (based on, but not attached to old thing), and could also be interpreted as "merge existing thing again" (and leaves me wondering if it would better suite some other strategy possible in the future). Not sure if that explanation suffices, but it comes to "recreate a merge" having more clear meaning than "remerge a merge", being somewhat ambiguous, thus confusing (to me, at least). I don`t know, thinking too much? Regards, Buga
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, Igor Djordjevicwrites: > On 14/03/2018 15:24, Sergey Organov wrote: [...] >> Thinking about it I've got an idea that what we actually need is >> --no-flatten flag that, when used alone, will just tell "git rebase" to >> stop flattening history, and which will be implicitly imposed by >> --recreate-merges (and --preserve-merges). >> >> Then the only thing the --recreate-merges will tune is to put 'merge' >> directives into the todo list for merge commits, exactly according to >> what its name suggests, while the default behavior will be to put 'pick' >> with suitable syntax into the todo. And arguments to the >> --recreate-merge will specify additional options for the 'merge' >> directive, obviously. > > This seem to basically boil down to what I mentioned previously[2] > through use of new `--rebase-merges` alongside `--recreate-merges`, just > that you named it `--no-flatten` here, but the point is the same - and > not something Johannes liked, "polluting" rebase option space further. Not quite so. The problem with --XXX-merges flags is that they do two things at once: they say _what_ to do and _how_ to do it. Clean UI designs usually have these things separate, and that's what I propose. The --[no-]flatten says _what_ (not) to do, and --recreate-merges says _how_ exactly it will be performed. In this model --no-flatten could have been called, say --preserve-shape, but not --rebase-merges. To minimize pollution, the _how_ part could rather be made option value: --no-flatten[=] where is 'rebase', 'remerge', etc. In this case we will need separate option to specify strategy options, if required, that will lead us to something similar to the set of merge strategies options. > I would agree with him, and settling onto `--rebase-merges` _instead_ of > `--recreate-merges` seems as a more appropriate name, indeed, now that > default behavior is actually merge commit rebasing and not recreating > (recreating still being possible through user editing the todo list). I hope he'd be pleased to be able to say --no-flatten=remerge and get back his current mode of operation, that he obviously has a good use for. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 14/03/2018 15:24, Sergey Organov wrote: > > > > Second side note: if we can fast-forward, currently we prefer that, and I > > > think we should keep that behavior with -R, too. > > > > I agree. > > I'm admittedly somewhat lost in the discussion, but are you talking > fast-forward on _rebasing_ existing merge? Where would it go in any of > the suggested algorithms of rebasing and why? > > I readily see how it can break merges. E.g., any "git merge --ff-only > --no-ff" merge will magically disappear. So, even if somehow supported, > fast-forward should not be performed by default during _rebasing_ of a > merge. Hmm, now that you brought this up, I can only agree, of course. What I had in my mind was more similar to "no-rebase-cousins", like if we can get away without actually rebasing the merge but still using the original one, do it. But I guess that`s not what Johannes originally asked about. This is another definitive difference between rebasing (`pick`?) and recreating (`merge`) a merge commit - in the case where we`re rebasing, of course it doesn`t make sense to drop commit this time (due to fast-forward). This does make sense in recreating the merge (only). > > > If the user wants to force a new merge, they simply remove that -R > > > flag. And this sounds wrong now, too, because we actually have _three_ possible behaviors here - (1) rebase merge commit, which should always do what its told (so no fast-forwarding, otherwise the whole concept of rebasing a merge commit doesn`t make sense), and recreate merge commit, which should (2) by default use fast-forward where possible (or whatever the settings say), but (3) also be possible to force a new merge as well (through standard `--no-ff`, I guess, or something). > Alternatively, they'd replace 'pick' with 'merge', as they already do > for other actions. "A plurality is not to be posited without necessity". > > Please, _please_, don't use 'merge' command to 'pick' merge commits! > It's utterly confusing! I agree here, as previously discussed[1], but let`s hear Johannes. > Thinking about it I've got an idea that what we actually need is > --no-flatten flag that, when used alone, will just tell "git rebase" to > stop flattening history, and which will be implicitly imposed by > --recreate-merges (and --preserve-merges). > > Then the only thing the --recreate-merges will tune is to put 'merge' > directives into the todo list for merge commits, exactly according to > what its name suggests, while the default behavior will be to put 'pick' > with suitable syntax into the todo. And arguments to the > --recreate-merge will specify additional options for the 'merge' > directive, obviously. This seem to basically boil down to what I mentioned previously[2] through use of new `--rebase-merges` alongside `--recreate-merges`, just that you named it `--no-flatten` here, but the point is the same - and not something Johannes liked, "polluting" rebase option space further. I would agree with him, and settling onto `--rebase-merges` _instead_ of `--recreate-merges` seems as a more appropriate name, indeed, now that default behavior is actually merge commit rebasing and not recreating (recreating still being possible through user editing the todo list). Now, the only thing left seems to be agreeing on actual command to use to rebase the merge commit, to `pick` it, so to say... ;) Regards, Buga [1] https://public-inbox.org/git/77b695d0-7564-80d7-d9e6-70a531e66...@gmail.com/ [2] https://public-inbox.org/git/b329bb98-f9d6-3d51-2513-465aad2fa...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Igor Djordjevicwrites: > Hi Dscho, > > On 07/03/2018 08:26, Johannes Schindelin wrote: [...] >> Second side note: if we can fast-forward, currently we prefer that, and I >> think we should keep that behavior with -R, too. > > I agree. I'm admittedly somewhat lost in the discussion, but are you talking fast-forward on _rebasing_ existing merge? Where would it go in any of the suggested algorithms of rebasing and why? I readily see how it can break merges. E.g., any "git merge --ff-only --no-ff" merge will magically disappear. So, even if somehow supported, fast-forward should not be performed by default during _rebasing_ of a merge. >> If the user wants to force a new merge, they simply remove that -R >> flag. Alternatively, they'd replace 'pick' with 'merge', as they already do for other actions. "A plurality is not to be posited without necessity". Please, _please_, don't use 'merge' command to 'pick' merge commits! It's utterly confusing! Thinking about it I've got an idea that what we actually need is --no-flatten flag that, when used alone, will just tell "git rebase" to stop flattening history, and which will be implicitly imposed by --recreate-merges (and --preserve-merges). Then the only thing the --recreate-merges will tune is to put 'merge' directives into the todo list for merge commits, exactly according to what its name suggests, while the default behavior will be to put 'pick' with suitable syntax into the todo. And arguments to the --recreate-merge will specify additional options for the 'merge' directive, obviously. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, Igor Djordjevicwrites: [...] > I don`t know, I`m thinking if we are looking at todo list from > different perspectives - to you, it seems to be a sequence of > commands to create something new (yes, from something that already > exists, but that`s implementation detail). In that context, using > `merge` might have sense (even if still being a very special merge). > > But to me, as we already have `pick` and not `commit` to rebase > commits, it means we are not creating but rather reusing what we have > (being an important concept to reason about), thus `pick` would still > fit in for picking a merge commit just nicely, and more naturally, > too. Exactly. Fundamentally, a good editing tool should first be able to just safely reproduce back what it got, only then everything else goes. >From this follows that git history-editing tool should start from sound history representation, i.e., some text representation of the DAG that allows to re-create the DAG back. The simplest way is to just list all the nodes in some topological order, along with references to the parents. Then, to simplify the list, first parent, unless explicitly specified, could be assumed to be the preceding item in the list. Next, we actually need _to do_ something with this, so we convert this to a _todo_ list by prepending action to each element of the list (isn't it Lisp once again?). Let the action be called 'pick'. Then, e.g., the piece of history: B1---B2 / \ S--M0---M1 M2 M3 -- M4 from M0 to M4 will be represented like this: skip S # Just to have a handy reference pick M0 # Implicit first parent (S) pick M1 # Implicit first parent (M0) pick M2 # Implicit first parent (M1) pick B1 M1 # Explicit first parent pick B2 # Implicit first parent (B1) pick M3 M2 B2 # Explicit first and second parents pick M4 Which basically gets us back to what you are advocating. Here is another variant, using command options to specify parents (I also exchanged order of branch and mainline): skip S # To have the base reference handy pick M0 # Implicit first parent (S) pick B1 # Implicit first parent (M0) pick B2 # Implicit first parent (B1) pick M1 -1 M0 # Explicit first parent M0 pick M2 # Implicit first parent (M1) pick M3 -2 B2 # Implicit first parent (M2) and # explicit second parent B2 pick M4 # Implicit first parent (M3) I like this one even better. IMHO, this is indeed a good starting point. No special treatment for merges is needed so far. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Dscho, On 12/03/2018 11:46, Johannes Schindelin wrote: > > > Sometimes one just needs to read the manual, and I don`t really think > > this is a ton complicated, but just something we didn`t really have > > before (real merge rebasing), so it requires a moment to grasp the > > concept. > > If that were the case, we would not keep getting bug reports about > --preserve-merges failing to reorder patches. Not sure where that is heading to, but what I`m arguing about is that introducing new commands and concepts (`merge`, and with `-R`) just makes the situation even worse (more stuff to grasp). Reusing existing concepts where possible doesn`t have this problem. > > Saying in favor of `--rebase-merges`, you mean as a separate option, > > alongside `--recreate-merges` (once that series lands)? > > No. I am against yet another option. The only reason I pollute the option > name space further with --recreate-merges is that it would be confusing to > users if the new mode was called --preserve-merges=v2 (but work *totally > differently*). I see. So I take you`re thinking about renaming `--recreate-merges` to `--rebase-merges` instead? That would seem sensible, too, I think, being the default usage mode in the first place. Being able to actually (re)create merges, too, once user goes interactive, would be "just" an additional (nice and powerful) feature on top of it. Regards, Buga
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 12/03/2018 13:56, Sergey Organov wrote: > > > > I agree with both of you that `pick ` is inflexible > > > (not to say just plain wrong), but I never thought about it like that. > > > > > > If we are to extract further mentioned explicit old:new merge > > > parameter mapping to a separate discussion point, what we`re > > > eventually left with is just replacing this: > > > > > > merge -R -C > > > > > > ... with this: > > > > > > pick > > > > I see where you are coming from. > > > > I also see where users will be coming from. Reading a todo list in the > > editor is as much documentation as it is a "program to execute". And I am > > afraid that reading a command without even mentioning the term "merge" > > once is pretty misleading in this setting. > > > > And even from the theoretical point of view: cherry-picking non-merge > > commits is *so much different* from "rebasing merge commits" as discussed > > here, so much so that using the same command would be even more > > misleading. > > This last statement is plain wrong when applied to the method in the > [RFC] you are replying to. Using the method in [RFC], "cherry-pick > non-merge" is nothing more or less than reduced version of generic > "cherry-pick merge", exactly as it should be. > > Or, in other words, "cherry-pick merge" is generalization of > "cherry-pick non-merge" to multiple parents. I think Sergey does have a point here, his approach showing it. Phillip`s simplification might be further from it, though, but we`re talking implementation again - important mental model should just be "rebasing a commit" (merge or non-merge), how we`re doing it is irrelevant for the user, the point (goal) is the same.
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Dscho, On 12/03/2018 11:37, Johannes Schindelin wrote: > > > If we are to extract further mentioned explicit old:new merge > > parameter mapping to a separate discussion point, what we`re > > eventually left with is just replacing this: > > > > merge -R -C > > > > ... with this: > > > > pick > > I see where you are coming from. > > I also see where users will be coming from. Reading a todo list in the > editor is as much documentation as it is a "program to execute". And I am > afraid that reading a command without even mentioning the term "merge" > once is pretty misleading in this setting. > > And even from the theoretical point of view: cherry-picking non-merge > commits is *so much different* from "rebasing merge commits" as discussed > here, so much so that using the same command would be even more > misleading. I would disagree here, as it seems you`re going too much into implementation and theory here, where it shouldn`t really matter from the user`s point of view - the point is to rebase a commit, `pick` it from one place and plant it elsewhere. Yes, some commits might have a bit different semantics then others (merge vs non-merge), but it should just be an implementation detail, in my opinion, no need to leak it in user`s face (more than necessary). I feel that "merge" is a command that works really well in the mindset of (re)creating merges. But if we are "only" rebasing an existing merge, `pick` seems much more appropriate (to me, at least), and it aligns with what I`m already expecting `pick` to be doing. Down below, if we are (re)creating the merge, or doing magic to somehow just port it over, should be irrelevant. So "rebase" equals "pick and plant" (port), not "merge". > > That is what I had in mind, seeming possibly more straightforward and > > beautifully aligned with previously existing (and well known) > > `rebase` terminology. > > > > Not to say this would make it possible to use other `rebase -i` todo > > list commands, too, like if you want to amend/edit merge commit after > > it was rebased, you would write: > > > > edit > > > > ..., where in case you would simply like to reword its commit > > message, it would be just: > > > > reword > > > > > > Even `squash` and `fixup` could have their place in combination with > > a (to be rebased) merge commit, albeit in a pretty exotic rebases, > > thus these could probably be just disallowed - for the time being, at > > least. > > Sure, for someone who read the manual, that would be easy to use. Of > course, that's the minority. I`m not following you here - the point is these are already existing commands, which would still fit in just nicely, so nothing new to learn nor read. Now, if we are to discuss use cases where people don`t even know what they`re doing, I would think that misses the point. Besides, it`s always easier to make more mistakes when you introduce yet more commands/semantics to think about/learn, and I think it can be avoided here, for the better. > Also: the `edit` command is poorly named to begin with. A much cleaner > design would be to introduce the `break` command as suggested by Stephan. This is orthogonal to what we`re discussing. Existing commands might not be perfect, but that`s what we have now, so let`s be consistent, not putting additional burden on the user there, at least. But for the record - I tend to agree, I often find myself wondering if `edit`-ed commit means `rebase` stops after applying the changes and _before_ making the commit itself (so we just edit and --continue), or _after_ it (so we edit, `commit --amend` and --continue). > > The real power would be buried in implementation, learning to rebase > > merge commits, so user is left with a very familiar interface, slightly > > adapted do accommodate a bit different nature of merge commit in > > comparison to an ordinary one, also to allow a bit more of interactive > > rebase functionality, but it would pretty much stay the same, without > > even a need to learn about new `merge`, `-R`, `-C`, and so on. > > > > Yes, those would have its purpose, but for real merging then > > (creating new merges, or recreating old ones), not necessarily for > > merge rebasing. > > > > With state of `merge -R -C ...` (that `-R` being the culprit), it > > kind of feels like we`re now trying to bolt "rebase merges" > > functionality onto a totally different one (recreate merges, serving > > a different purpose), making them both needlessly dependent on each > > other, further complicating user interface, making it more confusing > > and less tunable as per each separate functionality needs (rebase vs. > > recreate). > > > > I guess I`m the one to pretty much blame here, too, as I really > > wanted `--recreate-merges` to handle "rebase merges" better, only to > > later realize it might not be the best tool for the job, and that a > > more separate approach would be better (at least
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Johannes, Johannes Schindelinwrites: > Hi Buga, > > On Sun, 11 Mar 2018, Igor Djordjevic wrote: > >> I agree with both of you that `pick ` is inflexible >> (not to say just plain wrong), but I never thought about it like that. >> >> If we are to extract further mentioned explicit old:new merge >> parameter mapping to a separate discussion point, what we`re >> eventually left with is just replacing this: >> >> merge -R -C >> >> ... with this: >> >> pick > > I see where you are coming from. > > I also see where users will be coming from. Reading a todo list in the > editor is as much documentation as it is a "program to execute". And I am > afraid that reading a command without even mentioning the term "merge" > once is pretty misleading in this setting. > > And even from the theoretical point of view: cherry-picking non-merge > commits is *so much different* from "rebasing merge commits" as discussed > here, so much so that using the same command would be even more > misleading. This last statement is plain wrong when applied to the method in the [RFC] you are replying to. Using the method in [RFC], "cherry-pick non-merge" is nothing more or less than reduced version of generic "cherry-pick merge", exactly as it should be. Or, in other words, "cherry-pick merge" is generalization of "cherry-pick non-merge" to multiple parents. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, I just have two thoughts to contribute as answer, so please excuse the heavily cut quoted text: On Sun, 11 Mar 2018, Igor Djordjevic wrote: > Sometimes one just needs to read the manual, and I don`t really think > this is a ton complicated, but just something we didn`t really have > before (real merge rebasing), so it requires a moment to grasp the > concept. If that were the case, we would not keep getting bug reports about --preserve-merges failing to reorder patches. > Saying in favor of `--rebase-merges`, you mean as a separate option, > alongside `--recreate-merges` (once that series lands)? No. I am against yet another option. The only reason I pollute the option name space further with --recreate-merges is that it would be confusing to users if the new mode was called --preserve-merges=v2 (but work *totally differently*). Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Sun, 11 Mar 2018, Igor Djordjevic wrote: > I agree with both of you that `pick ` is inflexible > (not to say just plain wrong), but I never thought about it like that. > > If we are to extract further mentioned explicit old:new merge > parameter mapping to a separate discussion point, what we`re > eventually left with is just replacing this: > > merge -R -C > > ... with this: > > pick I see where you are coming from. I also see where users will be coming from. Reading a todo list in the editor is as much documentation as it is a "program to execute". And I am afraid that reading a command without even mentioning the term "merge" once is pretty misleading in this setting. And even from the theoretical point of view: cherry-picking non-merge commits is *so much different* from "rebasing merge commits" as discussed here, so much so that using the same command would be even more misleading. > That is what I had in mind, seeming possibly more straightforward and > beautifully aligned with previously existing (and well known) > `rebase` terminology. > > Not to say this would make it possible to use other `rebase -i` todo > list commands, too, like if you want to amend/edit merge commit after > it was rebased, you would write: > > edit > > ..., where in case you would simply like to reword its commit > message, it would be just: > > reword > > > Even `squash` and `fixup` could have their place in combination with > a (to be rebased) merge commit, albeit in a pretty exotic rebases, > thus these could probably be just disallowed - for the time being, at > least. Sure, for someone who read the manual, that would be easy to use. Of course, that's the minority. Also: the `edit` command is poorly named to begin with. A much cleaner design would be to introduce the `break` command as suggested by Stephan. > The real power would be buried in implementation, learning to rebase > merge commits, so user is left with a very familiar interface, slightly > adapted do accommodate a bit different nature of merge commit in > comparison to an ordinary one, also to allow a bit more of interactive > rebase functionality, but it would pretty much stay the same, without > even a need to learn about new `merge`, `-R`, `-C`, and so on. > > Yes, those would have its purpose, but for real merging then > (creating new merges, or recreating old ones), not necessarily for > merge rebasing. > > With state of `merge -R -C ...` (that `-R` being the culprit), it > kind of feels like we`re now trying to bolt "rebase merges" > functionality onto a totally different one (recreate merges, serving > a different purpose), making them both needlessly dependent on each > other, further complicating user interface, making it more confusing > and less tunable as per each separate functionality needs (rebase vs. > recreate). > > I guess I`m the one to pretty much blame here, too, as I really > wanted `--recreate-merges` to handle "rebase merges" better, only to > later realize it might not be the best tool for the job, and that a > more separate approach would be better (at least not through the same > `merge` todo list command)... > > [1] > https://public-inbox.org/git/f3872fb9-01bc-b2f1-aee9-cfc0e4db7...@gmail.com/ Well, the `-R` option is no worse than `git merge`'s `-s ` option (which *also* changes the strategies rather drastically). Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Dscho, On 11/03/2018 13:11, Johannes Schindelin wrote: > > > > I did wonder about using 'pick ' for rebasing merges > > > and keeping 'merge ...' for recreating them but I'm not sure if that > > > is a good idea. It has the advantage that the user cannot specify the > > > wrong parents for the merge to be rebased as 'git rebase' would work > > > out if the parents have been rebased, but maybe it's a bit magical to > > > use pick for merge commits. Also there isn't such a simple way for the > > > user to go from 'rabase this merge' to 'recreate this merge' as they'd > > > have to write the whole merge line themselves (though I guess > > > something like emacs' git-rebase.el would be able to help with that) > > > > Since the ultimate commit hashes of newly rebased commits would be > > unknown at the time of writing the todo file, I'm not sure how this > > would work to specify the parents? > > I agree with Phillip's follow-up that the `pick ` syntax > would pose a problem, but for different reasons: We already tried it, with > --preserve-merges, and it is just a really stupid syntax that does not > allow the user even to reorder commits. Or drop commits (except at the > very end of the todo list). Hehe, please excuse me, but in the light of that other explicit (or not) parent mapping discussion[1], I would take a chance to be really sneaky here and say that being non-explicit "is just a really stupid syntax that does not allow the user even to reorder rebased merge parents. Or drop parents (except at the very end of the parent list)." ;) [1] https://public-inbox.org/git/b329bb98-f9d6-3d51-2513-465aad2fa...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Dscho, On 11/03/2018 13:08, Johannes Schindelin wrote: > > > Hmm, funny enough, `pick ` was something I though about > > originally, too, feeling that it might make more sense in terms on > > what`s really going on, but I guess I wanted it incorporated into > > `--recreate-merges` too much that I tried really hard to fit it in, > > without changing it much :/ > > The `pick ` syntax is too limited to allow reordering, let > alone changing the parents. I agree, `pick ` syntax alone is never what I had in mind, so it`s missing further context here, touched in that other subthread[1]. My fault, sorry for confusion. > > pick :HEAD > > : > > I do not really like it, as it makes things a ton less intuitive. If you > did not know about this here discussion, and you did not read the manual > (and let's face it: a UI that does not require users to read the manual is > vastly superior to a UI that does), and you encountered this command: > > merge deadbeef cafecafe:download-button > > what would you think those parameters would mean? > > Granted, encountering > > merge -R -C deadbeef download-button # Merge branch 'download-button' > > is still not *quite* as intuitive as I would wish. Although, to be honest, > if I encountered this, I would think that I should probably leave the -R > and the -C deadbeef alone, and that I could change what is getting merged > by changing the `download-button` parameter. Agreed, encountering mapping is slightly more complicated, but I would argue it`s much more powerful at the same time, too, thus pretty much worth it. Without it, actually, it seems like we`re repeating the mistake of `--preserve-merges`, where we`re assuming too much (order of new and old parents being the same, and I guess number of them, too). Oh, and as we`re still discussing in terms of `merge` command, using (elsewhere mentioned[1]) `pick` instead, it might be even less non-intuitive, as we`re not married to `merge` semantics any more: pick deadbeef cafecafe:download-button And might be calling it "non-intuitive" is unfair, I guess it would rather be "not familiar yet", being case with any new functionality, let alone a very powerful one, where getting a clue on what it does at the beginning could do wonders later. Sacrificing that power for a bit of perceived simplicity, where it actually assumes stuff on its own (trying to stay simple for the user), doesn`t seem as a good way to go in the long run. Sometimes one just needs to read the manual, and I don`t really think this is a ton complicated, but just something we didn`t really have before (real merge rebasing), so it requires a moment to grasp the concept. But I`m still not sure if there isn`t a better way to present explicit mapping, just that : seemed as the most straightforward one to pass on the point for the purpose of discussing it. And I`m not reluctant to simplifying user interface, or for dropping the explicit mapping altogether, even, just for carefully measuring what we could lose without explicit mapping - think flexibility, but ease and correctness of implementation, too, as we need to guess the old merge parents and which new one they should correspond to. > > p.s. Are we moving towards `--rebase-merges` I mentioned in that > > other topic[1], as an add-on series after `--recreate-merges` hits > > the mainstream (as-is)...? :P > > That's an interesting question. One that I do not want to answer alone, > but I would be in favor of `--rebase-merges` as it is IMHO a much better > name for what this option is all about. Saying in favor of `--rebase-merges`, you mean as a separate option, alongside `--recreate-merges` (once that series lands)? That does seem as the most clean, intuitive and straightforward solution. Depending on the option you provide (recreate vs rebase), todo list would be populated accordingly by default - but important thing is "todo list parser" would know to parse both, so one can still adapt todo list to both recreate (some) and rebase (some other) merges at the same time. Of course, this only once `--rebase-merges` gets implemented, too, but as we had waited for it for so long, it won`t hurt to wait a bit more and possibly do it more properly, than rush it now and make a confusing user interface, needlessly welding two functionalities together (rebase vs. recreate). But I guess you already knew my thoughts, so let`s see what other`s think, too ;) Regards, Buga [1] https://public-inbox.org/git/f4e6237a-84dc-1aa8-150d-041806e24...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Dscho, On 11/03/2018 13:00, Johannes Schindelin wrote: > > > I actually like `pick` for _rebasing_ merge commits, as `pick` is > > already used for rebasing non-merge commits, too, so it feels natural. > > Phillip is right, though: this would repeat the design mistake of > --preserve-merges. > > We must not forget that the interactive mode is the target here, and that > the syntax (as well as the generated todo list) must allow for easy > modification. The `pick ` approach does not allow that, so we > cannot use it. > > The `merge -R -C ` approach is a lot better: > it offers the flexibility, without sacrificing the ease when not modifying > the todo list. Eh, I`m afraid the quote you took is missing the rest of its (important) context, where I mentioned already proposed format for `pick` in that other subthread[1], including other parameters beside merge commit to pick, as that parent mapping. I agree with both of you that `pick ` is inflexible (not to say just plain wrong), but I never thought about it like that. If we are to extract further mentioned explicit old:new merge parameter mapping to a separate discussion point, what we`re eventually left with is just replacing this: merge -R -C ... with this: pick That is what I had in mind, seeming possibly more straightforward and beautifully aligned with previously existing (and well known) `rebase` terminology. Not to say this would make it possible to use other `rebase -i` todo list commands, too, like if you want to amend/edit merge commit after it was rebased, you would write: edit ..., where in case you would simply like to reword its commit message, it would be just: reword Even `squash` and `fixup` could have their place in combination with a (to be rebased) merge commit, albeit in a pretty exotic rebases, thus these could probably be just disallowed - for the time being, at least. The real power would be buried in implementation, learning to rebase merge commits, so user is left with a very familiar interface, slightly adapted do accommodate a bit different nature of merge commit in comparison to an ordinary one, also to allow a bit more of interactive rebase functionality, but it would pretty much stay the same, without even a need to learn about new `merge`, `-R`, `-C`, and so on. Yes, those would have its purpose, but for real merging then (creating new merges, or recreating old ones), not necessarily for merge rebasing. With state of `merge -R -C ...` (that `-R` being the culprit), it kind of feels like we`re now trying to bolt "rebase merges" functionality onto a totally different one (recreate merges, serving a different purpose), making them both needlessly dependent on each other, further complicating user interface, making it more confusing and less tunable as per each separate functionality needs (rebase vs. recreate). I guess I`m the one to pretty much blame here, too, as I really wanted `--recreate-merges` to handle "rebase merges" better, only to later realize it might not be the best tool for the job, and that a more separate approach would be better (at least not through the same `merge` todo list command)... Regards, Buga [1] https://public-inbox.org/git/f3872fb9-01bc-b2f1-aee9-cfc0e4db7...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Thu, 8 Mar 2018, Igor Djordjevic wrote: > On 08/03/2018 16:16, Igor Djordjevic wrote: > > > > > Unless we reimplement the octopus merge (which works quite a bit > > > differently from the "rebase merge commit" strategy, even if it is > > > incremental, too), which has its own challenges: if there are merge > > > conflicts before merging the last MERGE_HEAD, the octopus merge will exit > > > with status 2, telling you "Should not be doing an octopus.". While we > > > will want to keep merge conflict markers and continue with the "rebase the > > > original merge commit" strategy. > > > > > > [...] > > > > The thing is, in my opinion, as long as we are _rebasing_, you can`t > > pick any merge strategy, as it doesn`t really make much sense. If you > > do want a specific strategy, than that`s _recreating_ a merge, and it > > goes fine with what you already have for `--recreate-merges`. > > > > On merge rebasing, the underlying strategy we decide to use is just an > > implementation detail, picking the one that works best (or the only > > one that works, even), user should have nothing to do with it. > > Just to add, if not already assumable, that I think we should stop > and let user react on conflicts on each of the "rebase the original > commit" strategy steps (rebase first parent, rebase second parent... > merge parents). I am not sure about that. Actually, I am pretty certain that we should imitate the recursive merge, which "accumulates" merge conflicts and only presents the final result, possibly with nested merge conflicts. In practice, we see that rarely, and more often than not, in those cases the user wants to re-do the merge differently, anyway. In the scenario we discuss here, I would wager a bet that the user encountering nested merge conflicts would most likely see that the rebase tried to rebase a merge, then `git reset --hard` and re-do the merge manually (i.e. *recreate* rather than *rebase*, most likely with (non-nested) merge conflicts). > I guess this stresses not using real "octopus merge" strategy even in > case where we`re rebasing octopus merge commit even more (and aligns > nicely with what you seem to expect already). My mistake, I should have pointed you my implementation: https://github.com/git/git/commit/d41a29ceb61ff445efd0f97a836f381de57c5a41 The full thicket of branches can be found here: https://github.com/git/git/compare/master...dscho:sequencer-shears I chose to make this an add-on patch after adding support for octopus merges because it actually makes it easier to think about "rebase merge commits" independently of the number of merge parents. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Jake, On Thu, 8 Mar 2018, Jacob Keller wrote: > On Thu, Mar 8, 2018 at 3:20 AM, Phillip Wood> wrote: > > I did wonder about using 'pick ' for rebasing merges > > and keeping 'merge ...' for recreating them but I'm not sure if that > > is a good idea. It has the advantage that the user cannot specify the > > wrong parents for the merge to be rebased as 'git rebase' would work > > out if the parents have been rebased, but maybe it's a bit magical to > > use pick for merge commits. Also there isn't such a simple way for the > > user to go from 'rabase this merge' to 'recreate this merge' as they'd > > have to write the whole merge line themselves (though I guess > > something like emacs' git-rebase.el would be able to help with that) > > Since the ultimate commit hashes of newly rebased commits would be > unknown at the time of writing the todo file, I'm not sure how this > would work to specify the parents? I agree with Phillip's follow-up that the `pick ` syntax would pose a problem, but for different reasons: We already tried it, with --preserve-merges, and it is just a really stupid syntax that does not allow the user even to reorder commits. Or drop commits (except at the very end of the todo list). As to your concern: The ultimate commit hashes do not have to be known until the merge commit is rebased. The current approach is to use labels for them (`label `), which Simply Works. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Thu, 8 Mar 2018, Igor Djordjevic wrote: > On 08/03/2018 12:20, Phillip Wood wrote: > > > > I did wonder about using 'pick ' for rebasing merges > > and keeping 'merge ...' for recreating them but I'm not sure if that > > is a good idea. It has the advantage that the user cannot specify the > > wrong parents for the merge to be rebased as 'git rebase' would work > > out if the parents have been rebased, but maybe it's a bit magical to > > use pick for merge commits. Also there isn't such a simple way for the > > user to go from 'rabase this merge' to 'recreate this merge' as they'd > > have to write the whole merge line themselves (though I guess > > something like emacs' git-rebase.el would be able to help with that) > > Hmm, funny enough, `pick ` was something I though about > originally, too, feeling that it might make more sense in terms on > what`s really going on, but I guess I wanted it incorporated into > `--recreate-merges` too much that I tried really hard to fit it in, > without changing it much :/ The `pick ` syntax is too limited to allow reordering, let alone changing the parents. > And now that I said this in a previous reply: > > > The thing is, in my opinion, as long as we are _rebasing_, you can`t > > pick any merge strategy, as it doesn`t really make much sense. If you > > do want a specific strategy, than that`s _recreating_ a merge, and it > > goes fine with what you already have for `--recreate-merges`. > > > > On merge rebasing, the underline strategy we decide to use is just an > > implementation detail, picking the one that works best (or the only > > one that works, even), user should have nothing to do with it. > > The difference between "rebase merge commit" and "recreate merge > commit" might starting to be more evident. True. > So... I might actually go for this one now. And (trying to stick with > explicit mappings, still :P), now that we`re not married to `merge` > expectations a user may already have, maybe a format like this: > > pick :HEAD > : I do not really like it, as it makes things a ton less intuitive. If you did not know about this here discussion, and you did not read the manual (and let's face it: a UI that does not require users to read the manual is vastly superior to a UI that does), and you encountered this command: merge deadbeef cafecafe:download-button what would you think those parameters would mean? Granted, encountering merge -R -C deadbeef download-button # Merge branch 'download-button' is still not *quite* as intuitive as I would wish. Although, to be honest, if I encountered this, I would think that I should probably leave the -R and the -C deadbeef alone, and that I could change what is getting merged by changing the `download-button` parameter. > p.s. Are we moving towards `--rebase-merges` I mentioned in that > other topic[1], as an add-on series after `--recreate-merges` hits > the mainstream (as-is)...? :P That's an interesting question. One that I do not want to answer alone, but I would be in favor of `--rebase-merges` as it is IMHO a much better name for what this option is all about. Other opinions? Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Thu, 8 Mar 2018, Igor Djordjevic wrote: > On 08/03/2018 13:16, Phillip Wood wrote: > > > > > I did wonder about using 'pick ' for rebasing merges > > > and keeping 'merge ...' for recreating them but I'm not sure if that > > > is a good idea. It has the advantage that the user cannot specify > > > the wrong parents for the merge to be rebased as 'git rebase' would > > > work out if the parents have been rebased, but maybe it's a bit > > > magical to use pick for merge commits. Also there isn't such a > > > simple way for the user to go from 'rabase this merge' to 'recreate > > > this merge' as they'd have to write the whole merge line themselves > > > (though I guess something like emacs' git-rebase.el would be able to > > > help with that) > > > > Scrub that, it is too magical and I don't think it would work with > > rearranged commits - it's making the --preserve-merges mistake all > > over again. It's a shame to have 'merge' mean 'recreate the merge' and > > 'rebase the merge' but I don't think there is an easy way round that. > > I actually like `pick` for _rebasing_ merge commits, as `pick` is > already used for rebasing non-merge commits, too, so it feels natural. Phillip is right, though: this would repeat the design mistake of --preserve-merges. We must not forget that the interactive mode is the target here, and that the syntax (as well as the generated todo list) must allow for easy modification. The `pick ` approach does not allow that, so we cannot use it. The `merge -R -C ` approach is a lot better: it offers the flexibility, without sacrificing the ease when not modifying the todo list. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Junio, On Thu, 8 Mar 2018, Junio C Hamano wrote: > Johannes Schindelinwrites: > > >> If we are talking about a drastic change, a few more days may not be > >> sufficient, but we are not in a hurry, as this already sounds like a > >> 2.18 material anyway. > > > > It is not at all a drastic change. It will actually make the current patch > > series better (simplifying the "can we fast-forward?" check). > > > > I just want to make sure that I already have Phillip's strategy working, > > but it will be yet another topic branch on top of the topic branch that > > will add support for octopus merges *after* the current --recreate-merges > > topic branch ;-) > > Oh, if the "not redoing the merge afresh, but attempt to reuse the > previous merge" that was discussed is going to be done as an > update/addition to the "redo the merge afresh" you already had in > production forever (and I had in 'pu' for quite a while in various > polished-ness during iteration), then I do prefer merging down what > has already proven to be 'next' worthy without waiting for the > discussion and your local verification of Phillip's new thing, > especially given that you'll be giving an explicit control to the > users which variant of "merge" insn will be used and the addition > of the Phillip's thing won't be a backward-compatibility issue when > it comes later. I would like to stress that the `--recreate-merges` functionality has *not* beed in production forever. It is a reimplementation in pure C of the Unix shell script that has been in production for five years (unchanged only for the last half year, the last critical fix happened in January last year). So I do see the value here to use `next` as test bed, and once the patch series will hit `next`, I will also merge it into Git for Windows (as an EXPERIMENTAL feature). As such, I am quite comfortable with refactoring a bit here and there, for example how to handle the case where a `merge` can be fast-forwarded. I think the changes I made in the last few days were an actual improvement to readability, even if the only reason for those changes was that I wanted to accommodate the "rebase merge commits" thing. FWIW I am fine with bumping this down to 2.18. I got a little bit of valuable feedback at GitMerge, e.g. that --recreate-merges does not (yet) have a mode where it can update refs corresponding to the rebased commits. (This could actually turn out to be independent of --recreate-merges, even). I already pushed out the updated branch, and it can be inspected at https://github.com/git/git/pull/447 The reason I did not send this out yet is that I want to give it a final look-over myself, so that I do not waste people's time again (as I did with that monster interdiff that was pretty bogus, too). Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 08/03/2018 16:16, Igor Djordjevic wrote: > > > Unless we reimplement the octopus merge (which works quite a bit > > differently from the "rebase merge commit" strategy, even if it is > > incremental, too), which has its own challenges: if there are merge > > conflicts before merging the last MERGE_HEAD, the octopus merge will exit > > with status 2, telling you "Should not be doing an octopus.". While we > > will want to keep merge conflict markers and continue with the "rebase the > > original merge commit" strategy. > > > > [...] > > The thing is, in my opinion, as long as we are _rebasing_, you can`t > pick any merge strategy, as it doesn`t really make much sense. If you > do want a specific strategy, than that`s _recreating_ a merge, and it > goes fine with what you already have for `--recreate-merges`. > > On merge rebasing, the underlying strategy we decide to use is just an > implementation detail, picking the one that works best (or the only > one that works, even), user should have nothing to do with it. Just to add, if not already assumable, that I think we should stop and let user react on conflicts on each of the "rebase the original commit" strategy steps (rebase first parent, rebase second parent... merge parents). I guess this stresses not using real "octopus merge" strategy even in case where we`re rebasing octopus merge commit even more (and aligns nicely with what you seem to expect already).
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On Thu, Mar 8, 2018 at 3:20 AM, Phillip Woodwrote: > On 07/03/18 07:26, Johannes Schindelin wrote: >> Hi Buga, >> >> On Tue, 6 Mar 2018, Igor Djordjevic wrote: >> >>> On 06/03/2018 19:12, Johannes Schindelin wrote: >> And I guess being consistent is pretty important, too - if you add new >> content during merge rebase, it should always show up in the merge, >> period. > > Yes, that should make it easy for the user to know what to expect from > rebase. [...] It will be slightly inconsistent. But in a defendable way, I think. >>> >>> I like where this discussion is heading, and here`s what I thought >>> about it :) >>> >>> [...] >>> >>> Here`s a twist - not letting `merge` trying to be too smart by >>> figuring out whether passed arguments correspond to rewritten >>> versions of the original merge parents (which would be too >>> restrictive, too, I`m afraid), but just be explicit about it, instead! >> >> That's the missing piece, I think. >> >>> So, it could be something like: >>> >>> merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed >> >> I like where this is heading, too, but I do not think that we can do this >> on a per-MERGE_HEAD basis. The vast majority of merge commits, in >> practice, have two parents. So the `merge` command would actually only >> have one revision to merge (because HEAD is the implicit first parent). So >> that is easy. >> >> But as soon as you go octopus, you can either perform an octopus merge, or >> rebase the original merge commit. You cannot really mix and match here. >> >> Unless we reimplement the octopus merge (which works quite a bit >> differently from the "rebase merge commit" strategy, even if it is >> incremental, too), which has its own challenges: if there are merge >> conflicts before merging the last MERGE_HEAD, the octopus merge will exit >> with status 2, telling you "Should not be doing an octopus.". While we >> will want to keep merge conflict markers and continue with the "rebase the >> original merge commit" strategy. >> >> And it would slam the door shut for adding support for *other* merge >> strategies to perform a more-than-two-parents merge. >> >> Also, I do not think that it makes a whole lot of sense in practice to let >> users edit what will be used for "original parent". If the user wants to >> do complicated stuff, they can already do that, via `exec`. The `merge` >> command really should be about facilitating common workflows, guiding the >> user to what is sane. >> >> Currently my favorite idea is to introduce a new flag: -R (for "rebase the >> original merge commit"). It would look like this: >> >> merge -R -C # >> >> This flag would of course trigger the consistency check (does the number >> of parents of the original merge commit agree with the parameter list? Was >> an original merge commit specified to begin with?), and it would not fall >> back to the recursive merge, but error out if that check failed. >> >> Side note: I wonder whether we really need to perform the additional check >> that ensures that the refers to the rewritten version of the >> original merge commit's parent. >> >> Second side note: if we can fast-forward, currently we prefer that, and I >> think we should keep that behavior with -R, too. > > I think that would be a good idea to avoid unpleasant surprises. > >> If the user wants to force a new merge, they simply remove that -R flag. >> >> What do you think? > > I did wonder about using 'pick ' for rebasing merges and > keeping 'merge ...' for recreating them but I'm not sure if that is a > good idea. It has the advantage that the user cannot specify the wrong > parents for the merge to be rebased as 'git rebase' would work out if > the parents have been rebased, but maybe it's a bit magical to use pick > for merge commits. Also there isn't such a simple way for the user to go > from 'rabase this merge' to 'recreate this merge' as they'd have to > write the whole merge line themselves (though I guess something like > emacs' git-rebase.el would be able to help with that) > > Best Wishes > > Phillip > Since the ultimate commit hashes of newly rebased commits would be unknown at the time of writing the todo file, I'm not sure how this would work to specify the parents? > >> Ciao, >> Dscho >> >
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 08/03/2018 13:16, Phillip Wood wrote: > > > Side note: I wonder whether we really need to perform the additional check > > that ensures that the refers to the rewritten version of the > > original merge commit's parent. > > > > [...] > > Oops that was referring to the first side note. I think fast forwarding > is a good idea. I'm not so sure about checking that refers > to the rewritten version of the original merge commit's parent any more > though. Having thought some more, I think we would want to allow the > user to rearrange a topic branch that is the parent of a merge and that > would require allowing a different parent as the old parent could be > dropped or swapped with another commit in the branch. I can't think of a > way to mechanically check that the new parent is 'somehow derived from' > the old one. Exactly, we must not depend on exact parent commits, but on parent "branches" (so to say). And that is why I think explicit mapping would be pretty helpful (if not the only approach). > > I did wonder about using 'pick ' for rebasing merges and > > keeping 'merge ...' for recreating them but I'm not sure if that is a > > good idea. It has the advantage that the user cannot specify the wrong > > parents for the merge to be rebased as 'git rebase' would work out if > > the parents have been rebased, but maybe it's a bit magical to use pick > > for merge commits. Also there isn't such a simple way for the user to go > > from 'rabase this merge' to 'recreate this merge' as they'd have to > > write the whole merge line themselves (though I guess something like > > emacs' git-rebase.el would be able to help with that) > > Scrub that, it is too magical and I don't think it would work with > rearranged commits - it's making the --preserve-merges mistake all over > again. It's a shame to have 'merge' mean 'recreate the merge' and > 'rebase the merge' but I don't think there is an easy way round that. I actually like `pick` for _rebasing_ merge commits, as `pick` is already used for rebasing non-merge commits, too, so it feels natural. Then `merge` is left to do what it is meant for - merging (or "recreate the merge", in the given context). I tried to outline a possible user interface in that other reply[1], elaborating it a bit, too, Regards, Buga [1] https://public-inbox.org/git/f3872fb9-01bc-b2f1-aee9-cfc0e4db7...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Phillip and Johannes, On 08/03/2018 12:20, Phillip Wood wrote: > > I did wonder about using 'pick ' for rebasing merges and > keeping 'merge ...' for recreating them but I'm not sure if that is a > good idea. It has the advantage that the user cannot specify the wrong > parents for the merge to be rebased as 'git rebase' would work out if > the parents have been rebased, but maybe it's a bit magical to use pick > for merge commits. Also there isn't such a simple way for the user to go > from 'rabase this merge' to 'recreate this merge' as they'd have to > write the whole merge line themselves (though I guess something like > emacs' git-rebase.el would be able to help with that) Hmm, funny enough, `pick ` was something I though about originally, too, feeling that it might make more sense in terms on what`s really going on, but I guess I wanted it incorporated into `--recreate-merges` too much that I tried really hard to fit it in, without changing it much :/ And now that I said this in a previous reply: > The thing is, in my opinion, as long as we are _rebasing_, you can`t > pick any merge strategy, as it doesn`t really make much sense. If you > do want a specific strategy, than that`s _recreating_ a merge, and it > goes fine with what you already have for `--recreate-merges`. > > On merge rebasing, the underline strategy we decide to use is just an > implementation detail, picking the one that works best (or the only > one that works, even), user should have nothing to do with it. The difference between "rebase merge commit" and "recreate merge commit" might starting to be more evident. So... I might actually go for this one now. And (trying to stick with explicit mappings, still :P), now that we`re not married to `merge` expectations a user may already have, maybe a format like this: pick :HEAD : Here, original-merge is a _commit_, where original-parent and new-parent are _labels_ (in terms of `--recreate-merges`). Everything else I previously said still holds - one is allowed to change or drop mappings, and add or drop new merge parents. Yes, in case user does something "stupid", he`ll get a lot of conflicts, but hey, we shouldn`t judge. p.s. Are we moving towards `--rebase-merges` I mentioned in that other topic[1], as an add-on series after `--recreate-merges` hits the mainstream (as-is)...? :P Regards, Buga [1] https://public-inbox.org/git/bc9f82fb-fd18-ee45-36a4-921a1381b...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Dscho, On 07/03/2018 08:26, Johannes Schindelin wrote: > > > So, it could be something like: > > > > merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed > > I like where this is heading, too, but I do not think that we can do this > on a per-MERGE_HEAD basis. The vast majority of merge commits, in > practice, have two parents. So the `merge` command would actually only > have one revision to merge (because HEAD is the implicit first parent). So > that is easy. > > But as soon as you go octopus, you can either perform an octopus merge, or > rebase the original merge commit. You cannot really mix and match here. > > Unless we reimplement the octopus merge (which works quite a bit > differently from the "rebase merge commit" strategy, even if it is > incremental, too), which has its own challenges: if there are merge > conflicts before merging the last MERGE_HEAD, the octopus merge will exit > with status 2, telling you "Should not be doing an octopus.". While we > will want to keep merge conflict markers and continue with the "rebase the > original merge commit" strategy. > > And it would slam the door shut for adding support for *other* merge > strategies to perform a more-than-two-parents merge. The thing is, in my opinion, as long as we are _rebasing_, you can`t pick any merge strategy, as it doesn`t really make much sense. If you do want a specific strategy, than that`s _recreating_ a merge, and it goes fine with what you already have for `--recreate-merges`. On merge rebasing, the underline strategy we decide to use is just an implementation detail, picking the one that works best (or the only one that works, even), user should have nothing to do with it. > Also, I do not think that it makes a whole lot of sense in practice to let > users edit what will be used for "original parent". If the user wants to > do complicated stuff, they can already do that, via `exec`. The `merge` > command really should be about facilitating common workflows, guiding the > user to what is sane. I thought of a situation like this: (1) ---o---o---o---M--- (master) \ / X1--X2--X3 (topic) Merge M was done with `-s ours`, obsoleting "topic" branch. But, I later realized that I actually do want that X2 commit in master. Now, I guess the most obvious approach is just cherry-picking it, but what if I would like to do something like this instead, with an interactive rebase (and rebasing the merge, not recreating it): (2) ---o---o---o---M'--- (master) |\ /| | X1'-X3'-/ | (topic) | | \--X2'--/ (new) This way, and having "topic" inherit original merge behavior due to merge rebasing, X1' and X3' would still be missing from M' as they did originally from M, but X2' would now be included, as it`s coming from a new branch, forged during interactive rebase. (note - implementation wise, this still wouldn`t be an octopus merge ;) > The vast majority of merge commits, in practice, have two parents. So > the `merge` command would actually only have one revision to merge > (because HEAD is the implicit first parent). Now, this is something I actually overlooked :( I guess order of parent commits could then be used to map to old commit parents, being a limitation in comparison to direct old-parent:new-parent mapping, but might be a more straightforward user experience... Though in case of octopus merge, where one would like to drop a branch from the middle, being merged with `-s ours`, that would be impossible, as then the next branch would be taking over dropped branch merge parent, yielding an incorrect result. So in this case: (3) ---o---o---o---M--- (master) |\ /| | X1--X3--/ | (topic) | | \--X2---/ (new) ... where "topic" was merged using `-s ours`, we wouldn`t be able to just remove whole "topic" branch from the rebased merge without influencing it incorrectly. With (any kind of) explicit old-parent:new-parent mapping, this is possible (and shouldn`t be any harder, implementation wise). Now, it`s a different story if we`re interested in such exotic scenarios in the first place, but if possible, I would be all for it... :) > Currently my favorite idea is to introduce a new flag: -R (for "rebase the > original merge commit"). It would look like this: > > merge -R -C # > > This flag would of course trigger the consistency check (does the number > of parents of the original merge commit agree with the parameter list? Was > an original merge commit specified to begin with?), and it would not fall > back to the recursive merge, but error out if that check failed. > > Side note: I wonder whether we really need to perform the additional check > that ensures that the refers to the rewritten version of the > original merge commit's parent. No, and even worse - I think we must not do that, as that merge parent might be moved elsewhere, which should be
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 08/03/18 11:20, Phillip Wood wrote: > On 07/03/18 07:26, Johannes Schindelin wrote: >> Hi Buga, >> >> On Tue, 6 Mar 2018, Igor Djordjevic wrote: >> >>> On 06/03/2018 19:12, Johannes Schindelin wrote: >> And I guess being consistent is pretty important, too - if you add new >> content during merge rebase, it should always show up in the merge, >> period. > > Yes, that should make it easy for the user to know what to expect from > rebase. [...] It will be slightly inconsistent. But in a defendable way, I think. >>> >>> I like where this discussion is heading, and here`s what I thought >>> about it :) >>> >>> [...] >>> >>> Here`s a twist - not letting `merge` trying to be too smart by >>> figuring out whether passed arguments correspond to rewritten >>> versions of the original merge parents (which would be too >>> restrictive, too, I`m afraid), but just be explicit about it, instead! >> >> That's the missing piece, I think. >> >>> So, it could be something like: >>> >>> merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed >> >> I like where this is heading, too, but I do not think that we can do this >> on a per-MERGE_HEAD basis. The vast majority of merge commits, in >> practice, have two parents. So the `merge` command would actually only >> have one revision to merge (because HEAD is the implicit first parent). So >> that is easy. >> >> But as soon as you go octopus, you can either perform an octopus merge, or >> rebase the original merge commit. You cannot really mix and match here. >> >> Unless we reimplement the octopus merge (which works quite a bit >> differently from the "rebase merge commit" strategy, even if it is >> incremental, too), which has its own challenges: if there are merge >> conflicts before merging the last MERGE_HEAD, the octopus merge will exit >> with status 2, telling you "Should not be doing an octopus.". While we >> will want to keep merge conflict markers and continue with the "rebase the >> original merge commit" strategy. >> >> And it would slam the door shut for adding support for *other* merge >> strategies to perform a more-than-two-parents merge. >> >> Also, I do not think that it makes a whole lot of sense in practice to let >> users edit what will be used for "original parent". If the user wants to >> do complicated stuff, they can already do that, via `exec`. The `merge` >> command really should be about facilitating common workflows, guiding the >> user to what is sane. >> >> Currently my favorite idea is to introduce a new flag: -R (for "rebase the >> original merge commit"). It would look like this: >> >> merge -R -C # >> >> This flag would of course trigger the consistency check (does the number >> of parents of the original merge commit agree with the parameter list? Was >> an original merge commit specified to begin with?), and it would not fall >> back to the recursive merge, but error out if that check failed. >> >> Side note: I wonder whether we really need to perform the additional check >> that ensures that the refers to the rewritten version of the >> original merge commit's parent. >> >> Second side note: if we can fast-forward, currently we prefer that, and I >> think we should keep that behavior with -R, too. > > I think that would be a good idea to avoid unpleasant surprises. Oops that was referring to the first side note. I think fast forwarding is a good idea. I'm not so sure about checking that refers to the rewritten version of the original merge commit's parent any more though. Having thought some more, I think we would want to allow the user to rearrange a topic branch that is the parent of a merge and that would require allowing a different parent as the old parent could be dropped or swapped with another commit in the branch. I can't think of a way to mechanically check that the new parent is 'somehow derived from' the old one. >> If the user wants to force a new merge, they simply remove that -R flag. >> >> What do you think? > > I did wonder about using 'pick ' for rebasing merges and > keeping 'merge ...' for recreating them but I'm not sure if that is a > good idea. It has the advantage that the user cannot specify the wrong > parents for the merge to be rebased as 'git rebase' would work out if > the parents have been rebased, but maybe it's a bit magical to use pick > for merge commits. Also there isn't such a simple way for the user to go > from 'rabase this merge' to 'recreate this merge' as they'd have to > write the whole merge line themselves (though I guess something like > emacs' git-rebase.el would be able to help with that) Scrub that, it is too magical and I don't think it would work with rearranged commits - it's making the --preserve-merges mistake all over again. It's a shame to have 'merge' mean 'recreate the merge' and 'rebase the merge' but I don't think there is an easy way round that. > Best Wishes > > Phillip > > >> Ciao, >> Dscho >> >
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 07/03/18 07:26, Johannes Schindelin wrote: > Hi Buga, > > On Tue, 6 Mar 2018, Igor Djordjevic wrote: > >> On 06/03/2018 19:12, Johannes Schindelin wrote: >>> > And I guess being consistent is pretty important, too - if you add new > content during merge rebase, it should always show up in the merge, > period. Yes, that should make it easy for the user to know what to expect from rebase. >>> >>> [...] >>> >>> It will be slightly inconsistent. But in a defendable way, I think. >> >> I like where this discussion is heading, and here`s what I thought >> about it :) >> >> [...] >> >> Here`s a twist - not letting `merge` trying to be too smart by >> figuring out whether passed arguments correspond to rewritten >> versions of the original merge parents (which would be too >> restrictive, too, I`m afraid), but just be explicit about it, instead! > > That's the missing piece, I think. > >> So, it could be something like: >> >> merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed > > I like where this is heading, too, but I do not think that we can do this > on a per-MERGE_HEAD basis. The vast majority of merge commits, in > practice, have two parents. So the `merge` command would actually only > have one revision to merge (because HEAD is the implicit first parent). So > that is easy. > > But as soon as you go octopus, you can either perform an octopus merge, or > rebase the original merge commit. You cannot really mix and match here. > > Unless we reimplement the octopus merge (which works quite a bit > differently from the "rebase merge commit" strategy, even if it is > incremental, too), which has its own challenges: if there are merge > conflicts before merging the last MERGE_HEAD, the octopus merge will exit > with status 2, telling you "Should not be doing an octopus.". While we > will want to keep merge conflict markers and continue with the "rebase the > original merge commit" strategy. > > And it would slam the door shut for adding support for *other* merge > strategies to perform a more-than-two-parents merge. > > Also, I do not think that it makes a whole lot of sense in practice to let > users edit what will be used for "original parent". If the user wants to > do complicated stuff, they can already do that, via `exec`. The `merge` > command really should be about facilitating common workflows, guiding the > user to what is sane. > > Currently my favorite idea is to introduce a new flag: -R (for "rebase the > original merge commit"). It would look like this: > > merge -R -C # > > This flag would of course trigger the consistency check (does the number > of parents of the original merge commit agree with the parameter list? Was > an original merge commit specified to begin with?), and it would not fall > back to the recursive merge, but error out if that check failed. > > Side note: I wonder whether we really need to perform the additional check > that ensures that the refers to the rewritten version of the > original merge commit's parent. > > Second side note: if we can fast-forward, currently we prefer that, and I > think we should keep that behavior with -R, too. I think that would be a good idea to avoid unpleasant surprises. > If the user wants to force a new merge, they simply remove that -R flag. > > What do you think? I did wonder about using 'pick ' for rebasing merges and keeping 'merge ...' for recreating them but I'm not sure if that is a good idea. It has the advantage that the user cannot specify the wrong parents for the merge to be rebased as 'git rebase' would work out if the parents have been rebased, but maybe it's a bit magical to use pick for merge commits. Also there isn't such a simple way for the user to go from 'rabase this merge' to 'recreate this merge' as they'd have to write the whole merge line themselves (though I guess something like emacs' git-rebase.el would be able to help with that) Best Wishes Phillip > Ciao, > Dscho >
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Johannes Schindelinwrites: >> If we are talking about a drastic change, a few more days may not be >> sufficient, but we are not in a hurry, as this already sounds like a >> 2.18 material anyway. > > It is not at all a drastic change. It will actually make the current patch > series better (simplifying the "can we fast-forward?" check). > > I just want to make sure that I already have Phillip's strategy working, > but it will be yet another topic branch on top of the topic branch that > will add support for octopus merges *after* the current --recreate-merges > topic branch ;-) Oh, if the "not redoing the merge afresh, but attempt to reuse the previous merge" that was discussed is going to be done as an update/addition to the "redo the merge afresh" you already had in production forever (and I had in 'pu' for quite a while in various polished-ness during iteration), then I do prefer merging down what has already proven to be 'next' worthy without waiting for the discussion and your local verification of Phillip's new thing, especially given that you'll be giving an explicit control to the users which variant of "merge" insn will be used and the addition of the Phillip's thing won't be a backward-compatibility issue when it comes later. >> As you made it clear that it is OK not to merge the current one for now, >> my objective of asking the question is already satisfied ;-) > > Depending how much GitMerge will occupy my time, I hope to have something > polished by tomorrow. My "for now" above was just for the coming few days. Don't rush things, and use valuable in-person time wisely, and have fun. Thanks.
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Junio, On Wed, 7 Mar 2018, Junio C Hamano wrote: > Johannes Schindelinwrites: > > >> OK, does this mean we want to wait before merging the "recreate > >> merge" topic down to 'next'? For more than a few weeks, it has been > >> slated for 'next'. > > > > Maybe a few more days. > > ... > > I want to discuss this in the other subthread, though. > > If we are talking about a drastic change, a few more days may not be > sufficient, but we are not in a hurry, as this already sounds like a > 2.18 material anyway. It is not at all a drastic change. It will actually make the current patch series better (simplifying the "can we fast-forward?" check). I just want to make sure that I already have Phillip's strategy working, but it will be yet another topic branch on top of the topic branch that will add support for octopus merges *after* the current --recreate-merges topic branch ;-) > As you made it clear that it is OK not to merge the current one for now, > my objective of asking the question is already satisfied ;-) Depending how much GitMerge will occupy my time, I hope to have something polished by tomorrow. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Johannes Schindelinwrites: >> OK, does this mean we want to wait before merging the "recreate >> merge" topic down to 'next'? For more than a few weeks, it has been >> slated for 'next'. > > Maybe a few more days. > ... > I want to discuss this in the other subthread, though. If we are talking about a drastic change, a few more days may not be sufficient, but we are not in a hurry, as this already sounds like a 2.18 material anyway. As you made it clear that it is OK not to merge the current one for now, my objective of asking the question is already satisfied ;-)
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Buga, On Tue, 6 Mar 2018, Igor Djordjevic wrote: > On 06/03/2018 19:12, Johannes Schindelin wrote: > > > > > > And I guess being consistent is pretty important, too - if you add new > > > > content during merge rebase, it should always show up in the merge, > > > > period. > > > > > > Yes, that should make it easy for the user to know what to expect from > > > rebase. > > > > [...] > > > > It will be slightly inconsistent. But in a defendable way, I think. > > I like where this discussion is heading, and here`s what I thought > about it :) > > [...] > > Here`s a twist - not letting `merge` trying to be too smart by > figuring out whether passed arguments correspond to rewritten > versions of the original merge parents (which would be too > restrictive, too, I`m afraid), but just be explicit about it, instead! That's the missing piece, I think. > So, it could be something like: > > merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed I like where this is heading, too, but I do not think that we can do this on a per-MERGE_HEAD basis. The vast majority of merge commits, in practice, have two parents. So the `merge` command would actually only have one revision to merge (because HEAD is the implicit first parent). So that is easy. But as soon as you go octopus, you can either perform an octopus merge, or rebase the original merge commit. You cannot really mix and match here. Unless we reimplement the octopus merge (which works quite a bit differently from the "rebase merge commit" strategy, even if it is incremental, too), which has its own challenges: if there are merge conflicts before merging the last MERGE_HEAD, the octopus merge will exit with status 2, telling you "Should not be doing an octopus.". While we will want to keep merge conflict markers and continue with the "rebase the original merge commit" strategy. And it would slam the door shut for adding support for *other* merge strategies to perform a more-than-two-parents merge. Also, I do not think that it makes a whole lot of sense in practice to let users edit what will be used for "original parent". If the user wants to do complicated stuff, they can already do that, via `exec`. The `merge` command really should be about facilitating common workflows, guiding the user to what is sane. Currently my favorite idea is to introduce a new flag: -R (for "rebase the original merge commit"). It would look like this: merge -R -C # This flag would of course trigger the consistency check (does the number of parents of the original merge commit agree with the parameter list? Was an original merge commit specified to begin with?), and it would not fall back to the recursive merge, but error out if that check failed. Side note: I wonder whether we really need to perform the additional check that ensures that the refers to the rewritten version of the original merge commit's parent. Second side note: if we can fast-forward, currently we prefer that, and I think we should keep that behavior with -R, too. If the user wants to force a new merge, they simply remove that -R flag. What do you think? Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Junio, On Tue, 6 Mar 2018, Junio C Hamano wrote: > Johannes Schindelinwrites: > > >> I don't think its possible to guess the semantics of the original merge > >> as users can use custom merge strategies and amend the result. It would > >> be possible to detect and unamended '-s ours' merge but special casing > >> that may end up causing users more confusion rather than helping them. > > > > FWIW I agree. > > I think it is a mistake to sacrifice predictability only to add > cleverness that sometimes work. Elsewhere in the thread, I think I > saw an argument to treat interactive and non-interactive something > very different, but there is no fundamental difference between them > (it is far easier with interactive to force the command to "port" > each change to a vastly different context) so having consistent > behaviour between the two cases is important, too. I could be swayed both ways, but Buga already pointed out that we do not have to compromise any consistency, simply by adding some syntactic sugar to the `merge` command so that the different behavior is *explicit*. > > My original plan was to always merge recursively and suggest to use `exec` > > commands if anything else is needed. > > > > But now with that excellent new idea to perform successive three-way > > merges of the original merge commit with the new tips, using the old tips > > as merge base, I am considering to change that. > > OK, does this mean we want to wait before merging the "recreate > merge" topic down to 'next'? For more than a few weeks, it has been > slated for 'next'. Maybe a few more days. My current thinking is to rework the handling of -c vs -C and *not* have two different todo_command enum values, but rather introduce an unsigned integer that has flags such as TODO_MERGE_EDIT. And for this new behavior, we could introduce a new flag (TODO_MERGE_REBASE_MERGE_COMMIT or something less unwieldy) and set that flag via merge -R -C ... (i.e. via a new flag `-R`). I want to discuss this in the other subthread, though. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Johannes Schindelinwrites: >> I don't think its possible to guess the semantics of the original merge >> as users can use custom merge strategies and amend the result. It would >> be possible to detect and unamended '-s ours' merge but special casing >> that may end up causing users more confusion rather than helping them. > > FWIW I agree. I think it is a mistake to sacrifice predictability only to add cleverness that sometimes work. Elsewhere in the thread, I think I saw an argument to treat interactive and non-interactive something very different, but there is no fundamental difference between them (it is far easier with interactive to force the command to "port" each change to a vastly different context) so having consistent behaviour between the two cases is important, too. > > My original plan was to always merge recursively and suggest to use `exec` > commands if anything else is needed. > > But now with that excellent new idea to perform successive three-way > merges of the original merge commit with the new tips, using the old tips > as merge base, I am considering to change that. OK, does this mean we want to wait before merging the "recreate merge" topic down to 'next'? For more than a few weeks, it has been slated for 'next'.
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 06/03/2018 19:12, Johannes Schindelin wrote: > > > > And I guess being consistent is pretty important, too - if you add new > > > content during merge rebase, it should always show up in the merge, > > > period. > > > > Yes, that should make it easy for the user to know what to expect from > > rebase. > > Indeed. We have seen time and time again that consistent behavior is the > only thing that lets us adhere to the Law of Least Surprise. > > And here lies the rub: do we really want to let `merge -C ` behave > completely differently than `merge`? Granted, in one case we provide a > template merge commit, in the other case, we do not. And the idea is > already to behave differently, although that difference only extends to > the commit message so far. > > But given the benefit (i.e. that the strategy to transform the original > merge commit into the new merge commit), I am willing to run that risk, > especially since I foresee only few users wanting to create new merge > commits from scratch using the `merge` todo command. > > Of course, even then we need to be careful: the user might have > *changed* or *moved* the original `merge` command. For example, if the > merge command read: > > merge -C deadbee cafecafe bedbedbed > > and the user switched the order of the merged branches into > > merge -C deadbee bedbedbed cafecafe > > we would have to detect the changed order of the arguments so that we > could still find the original branch tips. > > But the user might also have changed the branch(es) to merge completely, > in which case we might not even be able to find original branch tips. > > My preferred solution would be to let the `merge` command figure out > whether the passed arguments correspond to the rewritten versions of the > original merge parents. And only in that case would we use the fancy > strategy, in all other cases we would fall back to performing a regular > recursive (or octopus) merge. > > How does that sound? > > It will be slightly inconsistent. But in a defendable way, I think. I like where this discussion is heading, and here`s what I thought about it :) First, starting from non-interactive rebase, I guess we may now agree that _rebasing_ merges is an actually expected behavior, not recreating them (thus keeping manual conflict resolutions and amendments, not losing them). Now, interactive rebase is a totally different story, we already said user can change pretty much about everything, making merge _recreation_ to be a more sane choice, but let`s leave this other extreme for a brief moment. In the least interesting situation, though, user could just review and close todo list, without changing anything - and in that case it would be important, consistency wise, to behave exactly like in case of non-interactive rebase, meaning still rebasing merges, not recreating them. Ok, so that still aligns with what`s written so far - we need to be able to rebase merges interactively, too (not just recreate them), to stay consistent in less complex interactive rebases. But, what if user really wants to _recreate_ merges, for whatever reason? Come on, this is interactive rebase we`re talking about, why being restrictive? :) Here`s a twist - not letting `merge` trying to be too smart by figuring out whether passed arguments correspond to rewritten versions of the original merge parents (which would be too restrictive, too, I`m afraid), but just be explicit about it, instead! So, it could be something like: merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed The format is still something to think about, but the point is rather simple - explicitly map old and new merge parents, showing this inside todo list by default. This makes it much easier for later processing (and correct, no need to guess which one goes where), but also gives more power to the user, being able to decide which merge parents get "rebased", and which ones should go into the merge just like "new". So if a user gets an interactive todo list like that and just closes it, we still have exact situation like non-interactive rebase (and no guessing on implementation side). But, user might still decide to introduce new merge parents into the mix, even, where we could then just be merging those (as there is no old merge parent to actually rebase from): merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed new-branch Here, "new-branch" is something new, introduced inside interactive rebase, and it will be just merged into the other two (which are still being rebased). Also, another example - if original merge parent "123abc" was merged from the other side using `-s ours` strategy, that means all the content this branch originally had will still be missing from the rebased merge (expect for what`s been cherry-picked elsewhere). But, I would argue it`s quite legit to want to revise that decision, and let that content in this time. To make that
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Phillip & Buga, On Tue, 6 Mar 2018, Phillip Wood wrote: > On 02/03/18 23:33, Igor Djordjevic wrote: > > > > [...] > > Otherwise, I would be interested to learn how context/semantics > > guessing could provide a better default action (without introducing > > more complexity for might not be much benefit, if any). > > I don't think its possible to guess the semantics of the original merge > as users can use custom merge strategies and amend the result. It would > be possible to detect and unamended '-s ours' merge but special casing > that may end up causing users more confusion rather than helping them. FWIW I agree. My original plan was to always merge recursively and suggest to use `exec` commands if anything else is needed. But now with that excellent new idea to perform successive three-way merges of the original merge commit with the new tips, using the old tips as merge base, I am considering to change that. There is a big problem here, though: consistency. See below for more musings about that. > > And I guess being consistent is pretty important, too - if you add new > > content during merge rebase, it should always show up in the merge, > > period. > > Yes, that should make it easy for the user to know what to expect from > rebase. Indeed. We have seen time and time again that consistent behavior is the only thing that lets us adhere to the Law of Least Surprise. And here lies the rub: do we really want to let `merge -C ` behave completely differently than `merge`? Granted, in one case we provide a template merge commit, in the other case, we do not. And the idea is already to behave differently, although that difference only extends to the commit message so far. But given the benefit (i.e. that the strategy to transform the original merge commit into the new merge commit), I am willing to run that risk, especially since I foresee only few users wanting to create new merge commits from scratch using the `merge` todo command. Of course, even then we need to be careful: the user might have *changed* or *moved* the original `merge` command. For example, if the merge command read: merge -C deadbee cafecafe bedbedbed and the user switched the order of the merged branches into merge -C deadbee bedbedbed cafecafe we would have to detect the changed order of the arguments so that we could still find the original branch tips. But the user might also have changed the branch(es) to merge completely, in which case we might not even be able to find original branch tips. My preferred solution would be to let the `merge` command figure out whether the passed arguments correspond to the rewritten versions of the original merge parents. And only in that case would we use the fancy strategy, in all other cases we would fall back to performing a regular recursive (or octopus) merge. How does that sound? It will be slightly inconsistent. But in a defendable way, I think. Ciao, Dscho
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 02/03/18 23:33, Igor Djordjevic wrote: > Hi Phillip, > >> On Fri, Mar 2, 2018 at 4:36 AM, Phillip Wood wrote: >>> It is interesting to think what it means to faithfully rebase a '-s ours' merge. >>> >>> I should have explained that I mean is a faithful rebase one that >>> adheres to the semantics of '-s ours' (i.e. ignores any changes in the >>> side branch) or one that merges new changes from the side branch into >>> the content of the original merge? In your example you add B4 to B. If >>> M' preserves the semantics of '-s ours' then it will not contain the >>> changes in B4. I think your result does (correct me if I'm wrong) so it >>> is preserving the content of the original merge rather than the >>> semantics of it. > > Yeah, I understood what you mean, and I see you noticed that B4 > commit, for which I did anticipate possibly bringing up a discussion > like this ;) > > I agree with Jake here, my thoughts exactly (what I wrote in that > other subthread[1], too): > > On 02/03/2018 17:02, Jacob Keller wrote: >> >> We only have the content, and we don't know the semantics (nor, I >> think, should we attempt to understand or figure out the semantics). > > Hmm, I wanted to elaborate a bit here, but that sentence seems to > summarize the pure essence of it, and whatever I write looks like > just repeating the same stuff again... > > That`s just it. And stopping to give the user a chance to > review/amend the result, where he might decide he actually did want > something else - so all good. > > Otherwise, I would be interested to learn how context/semantics > guessing could provide a better default action (without introducing > more complexity for might not be much benefit, if any). I don't think its possible to guess the semantics of the original merge as users can use custom merge strategies and amend the result. It would be possible to detect and unamended '-s ours' merge but special casing that may end up causing users more confusion rather than helping them. > But in the end, I guess we can just discuss the "most sane default" > to present to the user (introduce or obsolete that new commit B4, in > the discussed example[2]), as we should definitely stop for amending > anyway, not proceeding automatically whenever U1' != U2'. I can see the reason for that but I'm concerned that it might get annoying with an interactive rebase as it would stop whenever one of the commits on a topic branch that is a parent of a merge gets amended. (squashing and reordering existing commits on a topic branch would be OK though) > Oh, and what about situations where we introduce new or drop existing > branches (which should be possible with new `--recreate-merges`)...? > "Preserving old branch semantics" may have even less sense here - the > (possibly heavily reorganized) content is the only thing we have, > where context will (and should) be provided by the user. In this scheme there is now way to change the parents of a merge so preserving the old branch sementics is well defined. If the user wants to change the parents of the merge then this scheme wont help them. > And I guess being consistent is pretty important, too - if you add > new content during merge rebase, it should always show up in the > merge, period. Yes, that should make it easy for the user to know what to expect from rebase. > It seems pretty confusing to find out one of the > branches "declared special" (even more if it`s based on uncertain > guess-work), so when you add something to it it`s just "swallowed", > as the whole branch is always obsoleted, for now and ever. > > I might even see a value in such behavior, but only as a conscious > user action, not something done automatically... I guess? :) > > Regards, Buga > > [1] > https://public-inbox.org/git/f26cdbe2-1bc3-02ff-7b99-12a6ebab5...@gmail.com/ > [2] > https://public-inbox.org/git/f1a960dc-cc5c-e7b0-10b6-39e551665...@gmail.com/ >
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Igor, Igor Djordjevicwrites: [...] > Now, not to get misinterpreted to pick sides in "(re)create" vs > "rebase" merge commit discussion, I just think these two (should) have > a different purpose, and actually having both inside interactive rebase > is what we should be aiming for. Yes, if the user has an existing merge that he intends to throw away by re-merging from scratch, he should be given a way to do it during history editing session, no argues. What I argue against is that this mode of operation is the default one, let alone the only one. > And that`s what I think is important to understand before any further > discussion - _(re)creating_ existing merge commits is not the same as > _rebasing_ them, even though the former can sometimes be used to > achieve the latter. Yes, indeed. Sometimes creating new merge instead of original does the job of rebasing the original, only it does it by pure accident. -- Sergey
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 02/03/2018 19:14, Igor Djordjevic wrote: > > > > It is interesting to think what it means to faithfully rebase a '-s > > > ours' merge. In your example the rebase does not introduce any new > > > changes into branch B that it doesn't introduce to branch A. Had it > > > added a fixup to branch B1 for example or if the topology was more > > > complex so that B ended up with some other changes that the rebase did > > > not introduce into A, then M' would contain those extra changes whereas > > > '--recreate-merges' with '-s ours' (once it supports it) would not. > > > > Unless the method of merging was stored, I don't think we *can* > > correctly automate resolving of "-s ours" because all we store is the > > resulting content, and we don't know how or why the user generated it > > as such. I believe the "correct" solution in any case would be to take > > the content we DO know and then ask the user to stop for amendments. > > I agree with Jake, and for the exact same reasons. > > That said, I`d like to see what mentioned '--recreate-merges' with > '-s ours' does (or would do, once it supports it), would you have a > pointer for me where to look at? > > But if that`s something yet to come, might be it`s still open for > discussion. I mean, even this topic started inside original > `--recreate-merges` one[1], and hopefully it can still bring > improvements there (sooner or later). > > [1] > https://public-inbox.org/git/cover.1516225925.git.johannes.schinde...@gmx.de/ Ok, I went through mentioned `--recreate-merges` topic again, and I think I see one crucial merge commit handling difference. In there, as it is at the moment, merge commits are really to be _recreated_... as the option name seems to imply ;) In terms of interactive rebasing, it actually comes from "todo list" becoming much more powerful, gaining ability to create (new) merges, which is wonderful from the aspect of history rewriting (what rebase is all about). But, I would argue it is a different concept from actually _rebasing_ existing merge commits, being what we`re discussing about here. Yes, you can use merge commit (re)creation to "rebase" existing merge commit so the end result is the same, being what `--recreate-merges` now does, but that only goes for some (uninteresting?) merge commits where not knowing the deeper context of how the merge commit is originally created is not important as no content is to be lost (due to missing that deeper and utterly unknown context). But as soon as you try to do that with more complex merge commits, like holding manual amendments and conflict resolutions, it doesn`t really work as expected - which I demonstrated in my original example script[1] in this topic, original merge commit amendment lost on rebase, and even worse - that happened silently. Now, not to get misinterpreted to pick sides in "(re)create" vs "rebase" merge commit discussion, I just think these two (should) have a different purpose, and actually having both inside interactive rebase is what we should be aiming for. And that`s what I think is important to understand before any further discussion - _(re)creating_ existing merge commits is not the same as _rebasing_ them, even though the former can sometimes be used to achieve the latter. Regards, Buga [1] https://public-inbox.org/git/bbe64321-4d3a-d3fe-8bb9-58b600fab...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Phillip, > On Fri, Mar 2, 2018 at 4:36 AM, Phillip Wood wrote: > > > > > It is interesting to think what it means to faithfully rebase a '-s > > > ours' merge. > > > > I should have explained that I mean is a faithful rebase one that > > adheres to the semantics of '-s ours' (i.e. ignores any changes in the > > side branch) or one that merges new changes from the side branch into > > the content of the original merge? In your example you add B4 to B. If > > M' preserves the semantics of '-s ours' then it will not contain the > > changes in B4. I think your result does (correct me if I'm wrong) so it > > is preserving the content of the original merge rather than the > > semantics of it. Yeah, I understood what you mean, and I see you noticed that B4 commit, for which I did anticipate possibly bringing up a discussion like this ;) I agree with Jake here, my thoughts exactly (what I wrote in that other subthread[1], too): On 02/03/2018 17:02, Jacob Keller wrote: > > We only have the content, and we don't know the semantics (nor, I > think, should we attempt to understand or figure out the semantics). Hmm, I wanted to elaborate a bit here, but that sentence seems to summarize the pure essence of it, and whatever I write looks like just repeating the same stuff again... That`s just it. And stopping to give the user a chance to review/amend the result, where he might decide he actually did want something else - so all good. Otherwise, I would be interested to learn how context/semantics guessing could provide a better default action (without introducing more complexity for might not be much benefit, if any). But in the end, I guess we can just discuss the "most sane default" to present to the user (introduce or obsolete that new commit B4, in the discussed example[2]), as we should definitely stop for amending anyway, not proceeding automatically whenever U1' != U2'. Oh, and what about situations where we introduce new or drop existing branches (which should be possible with new `--recreate-merges`)...? "Preserving old branch semantics" may have even less sense here - the (possibly heavily reorganized) content is the only thing we have, where context will (and should) be provided by the user. And I guess being consistent is pretty important, too - if you add new content during merge rebase, it should always show up in the merge, period. It seems pretty confusing to find out one of the branches "declared special" (even more if it`s based on uncertain guess-work), so when you add something to it it`s just "swallowed", as the whole branch is always obsoleted, for now and ever. I might even see a value in such behavior, but only as a conscious user action, not something done automatically... I guess? :) Regards, Buga [1] https://public-inbox.org/git/f26cdbe2-1bc3-02ff-7b99-12a6ebab5...@gmail.com/ [2] https://public-inbox.org/git/f1a960dc-cc5c-e7b0-10b6-39e551665...@gmail.com/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
Hi Phillip, On 02/03/2018 17:00, Jacob Keller wrote: > > > It is interesting to think what it means to faithfully rebase a '-s > > ours' merge. In your example the rebase does not introduce any new > > changes into branch B that it doesn't introduce to branch A. Had it > > added a fixup to branch B1 for example or if the topology was more > > complex so that B ended up with some other changes that the rebase did > > not introduce into A, then M' would contain those extra changes whereas > > '--recreate-merges' with '-s ours' (once it supports it) would not. > > Unless the method of merging was stored, I don't think we *can* > correctly automate resolving of "-s ours" because all we store is the > resulting content, and we don't know how or why the user generated it > as such. I believe the "correct" solution in any case would be to take > the content we DO know and then ask the user to stop for amendments. I agree with Jake, and for the exact same reasons. That said, I`d like to see what mentioned '--recreate-merges' with '-s ours' does (or would do, once it supports it), would you have a pointer for me where to look at? But if that`s something yet to come, might be it`s still open for discussion. I mean, even this topic started inside original `--recreate-merges` one[1], and hopefully it can still bring improvements there (sooner or later). Thanks, Buga [1] https://public-inbox.org/git/cover.1516225925.git.johannes.schinde...@gmx.de/
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On Fri, Mar 2, 2018 at 4:36 AM, Phillip Woodwrote: > On 02/03/18 11:17, Phillip Wood wrote: >> >> On 02/03/18 01:16, Igor Djordjevic wrote: >>> >>> Hi Sergey, >>> >>> On 01/03/2018 06:39, Sergey Organov wrote: >> (3) ---X1---o---o---o---o---o---X2 >>|\ |\ >>| A1---A2---A3---U1 | A1'--A2'--A3'--U1' >>| \ | >>| M | >>| / | >>\-B1---B2---B3---U2 \-B1'--B2'--B3'--U2' >> > > Meh, I hope I`m rushing it now, but for example, if we had decided to > drop commit A2 during an interactive rebase (so losing A2' from > diagram above), wouldn`t U2' still introduce those changes back, once > U1' and U2' are merged, being incorrect/unwanted behavior...? :/ > > [...] Yeah, I now see it myself. I'm sorry for being lazy and not inspecting this more carefully in the first place. >>> >>> No problem, that`s why we`re discussing it, and I`m glad we`re >>> aligned now, so we can move forward :) >>> > So while your original proposal currently seems like it could be > working nicely for non-interactive rebase (and might be some simpler > interactive ones), now hitting/acknowledging its first real use > limit, my additional quick attempt[1] just tries to aid pretty > interesting case of complicated interactive rebase, too, where we > might be able to do better as well, still using you original proposal > as a base idea :) Yes, thank you for pushing me back to reality! :-) The work and thoughts you are putting into solving the puzzle are greatly appreciated! >>> >>> You`re welcome, and I am enjoying it :) >>> Thinking about it overnight, I now suspect that original proposal had a mistake in the final merge step. I think that what you did is a way to fix it, and I want to try to figure what exactly was wrong in the original proposal and to find simpler way of doing it right. The likely solution is to use original UM as a merge-base for final 3-way merge of U1' and U2', but I'm not sure yet. Sounds pretty natural though, as that's exactly UM from which both U1' and U2' have diverged due to rebasing and other history editing. >>> >>> Yes, this might be it...! ;) >>> >>> To prove myself it works, I`ve assembled a pretty crazy `-s ours` >>> merge interactive rebase scenario, and it seems this passes the test, >>> ticking all the check boxes (I could think of) :P > > Hi Igor > >> It is interesting to think what it means to faithfully rebase a '-s >> ours' merge. > I should have explained that I mean is a faithful rebase one that > adheres to the semantics of '-s ours' (i.e. ignores any changes in the > side branch) or one that merges new changes from the side branch into > the content of the original merge? In your example you add B4 to B. If > M' preserves the semantics of '-s ours' then it will not contain the > changes in B4. I think your result does (correct me if I'm wrong) so it > is preserving the content of the original merge rather than the > semantics of it. > > Best Wishes > > Phillip > I believe this was always the outline of this type of approach, as per Sergey's original email. We only have the content, and we don't know the semantics (nor, I think, should we attempt to understand or figure out the semantics). Regards, Jake
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On Fri, Mar 2, 2018 at 3:17 AM, Phillip Woodwrote: > > It is interesting to think what it means to faithfully rebase a '-s > ours' merge. In your example the rebase does not introduce any new > changes into branch B that it doesn't introduce to branch A. Had it > added a fixup to branch B1 for example or if the topology was more > complex so that B ended up with some other changes that the rebase did > not introduce into A, then M' would contain those extra changes whereas > '--recreate-merges' with '-s ours' (once it supports it) would not. > Unless the method of merging was stored, I don't think we *can* correctly automate resolving of "-s ours" because all we store is the resulting content, and we don't know how or why the user generated it as such. I believe the "correct" solution in any case would be to take the content we DO know and then ask the user to stop for amendments. Thanks, Jake
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 02/03/18 11:17, Phillip Wood wrote: > > On 02/03/18 01:16, Igor Djordjevic wrote: >> >> Hi Sergey, >> >> On 01/03/2018 06:39, Sergey Organov wrote: >>> > (3) ---X1---o---o---o---o---o---X2 >|\ |\ >| A1---A2---A3---U1 | A1'--A2'--A3'--U1' >| \ | >| M | >| / | >\-B1---B2---B3---U2 \-B1'--B2'--B3'--U2' > Meh, I hope I`m rushing it now, but for example, if we had decided to drop commit A2 during an interactive rebase (so losing A2' from diagram above), wouldn`t U2' still introduce those changes back, once U1' and U2' are merged, being incorrect/unwanted behavior...? :/ [...] >>> >>> Yeah, I now see it myself. I'm sorry for being lazy and not inspecting >>> this more carefully in the first place. >> >> No problem, that`s why we`re discussing it, and I`m glad we`re >> aligned now, so we can move forward :) >> So while your original proposal currently seems like it could be working nicely for non-interactive rebase (and might be some simpler interactive ones), now hitting/acknowledging its first real use limit, my additional quick attempt[1] just tries to aid pretty interesting case of complicated interactive rebase, too, where we might be able to do better as well, still using you original proposal as a base idea :) >>> >>> Yes, thank you for pushing me back to reality! :-) The work and thoughts >>> you are putting into solving the puzzle are greatly appreciated! >> >> You`re welcome, and I am enjoying it :) >> >>> Thinking about it overnight, I now suspect that original proposal had a >>> mistake in the final merge step. I think that what you did is a way to >>> fix it, and I want to try to figure what exactly was wrong in the >>> original proposal and to find simpler way of doing it right. >>> >>> The likely solution is to use original UM as a merge-base for final >>> 3-way merge of U1' and U2', but I'm not sure yet. Sounds pretty natural >>> though, as that's exactly UM from which both U1' and U2' have diverged >>> due to rebasing and other history editing. >> >> Yes, this might be it...! ;) >> >> To prove myself it works, I`ve assembled a pretty crazy `-s ours` >> merge interactive rebase scenario, and it seems this passes the test, >> ticking all the check boxes (I could think of) :P Hi Igor > It is interesting to think what it means to faithfully rebase a '-s > ours' merge. I should have explained that I mean is a faithful rebase one that adheres to the semantics of '-s ours' (i.e. ignores any changes in the side branch) or one that merges new changes from the side branch into the content of the original merge? In your example you add B4 to B. If M' preserves the semantics of '-s ours' then it will not contain the changes in B4. I think your result does (correct me if I'm wrong) so it is preserving the content of the original merge rather than the semantics of it. Best Wishes Phillip (ignore the rest of what I wrote earlier I don't think it's correct) In your example the rebase does not introduce any new > changes into branch B that it doesn't introduce to branch A. Had it > added a fixup to branch B1 for example or if the topology was more > complex so that B ended up with some other changes that the rebase did > not introduce into A, then M' would contain those extra changes whereas > '--recreate-merges' with '-s ours' (once it supports it) would not. > >> >> Let`s see our starting situation: >> >> (0) ---X8--B2'--X9 (master) >> |\ >> | A1---A2---A3 (A) >> | \ >> | M (topic) >> | / >> \-B1---B2---B3 (B) >> >> >> Here, merge commit M is done with `-s ours` (obsoleting branch "B"), >> plus amended to make it an "evil merge", where a commit B2 from >> obsoleted branch "B" is cherry picked to "master". >> >> Now, we want to rebase "topic" (M) onto updated "master" (X9), but to >> make things more interesting, we`ll do it interactively, with some >> amendments, drops, additions and even more cherry-picks! >> >> This is what the final result looks like: >> >> (1) ---X8--B2'--X9 (master) >> |\ >> | A12--A2'---B3' (A) >> | \ >> | M' (topic) >> | / >> \-B1'--B3'---B4 (B) >> >> >> During interactive rebase, on branch "A", we amended A1 into A12, >> dropped A3 and cherry-picked B3. On branch "B", B4 is added, B2' being >> omitted automatically as already present in "master". >> >> So... In comparison to original merge commit M, rebased merge commit >> M' is expected to: >> >> - Add X9, from updated "master" >> - Have A1 changed to A12, due to A12 commit amendment >> - Keep A2, rebased as A2' >> - Remove A3, due to
Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)
On 02/03/18 01:16, Igor Djordjevic wrote: > > Hi Sergey, > > On 01/03/2018 06:39, Sergey Organov wrote: >> (3) ---X1---o---o---o---o---o---X2 |\ |\ | A1---A2---A3---U1 | A1'--A2'--A3'--U1' | \ | | M | | / | \-B1---B2---B3---U2 \-B1'--B2'--B3'--U2' >>> >>> Meh, I hope I`m rushing it now, but for example, if we had decided to >>> drop commit A2 during an interactive rebase (so losing A2' from >>> diagram above), wouldn`t U2' still introduce those changes back, once >>> U1' and U2' are merged, being incorrect/unwanted behavior...? :/ >>> >>> [...] >> >> Yeah, I now see it myself. I'm sorry for being lazy and not inspecting >> this more carefully in the first place. > > No problem, that`s why we`re discussing it, and I`m glad we`re > aligned now, so we can move forward :) > >>> So while your original proposal currently seems like it could be >>> working nicely for non-interactive rebase (and might be some simpler >>> interactive ones), now hitting/acknowledging its first real use >>> limit, my additional quick attempt[1] just tries to aid pretty >>> interesting case of complicated interactive rebase, too, where we >>> might be able to do better as well, still using you original proposal >>> as a base idea :) >> >> Yes, thank you for pushing me back to reality! :-) The work and thoughts >> you are putting into solving the puzzle are greatly appreciated! > > You`re welcome, and I am enjoying it :) > >> Thinking about it overnight, I now suspect that original proposal had a >> mistake in the final merge step. I think that what you did is a way to >> fix it, and I want to try to figure what exactly was wrong in the >> original proposal and to find simpler way of doing it right. >> >> The likely solution is to use original UM as a merge-base for final >> 3-way merge of U1' and U2', but I'm not sure yet. Sounds pretty natural >> though, as that's exactly UM from which both U1' and U2' have diverged >> due to rebasing and other history editing. > > Yes, this might be it...! ;) > > To prove myself it works, I`ve assembled a pretty crazy `-s ours` > merge interactive rebase scenario, and it seems this passes the test, > ticking all the check boxes (I could think of) :P It is interesting to think what it means to faithfully rebase a '-s ours' merge. In your example the rebase does not introduce any new changes into branch B that it doesn't introduce to branch A. Had it added a fixup to branch B1 for example or if the topology was more complex so that B ended up with some other changes that the rebase did not introduce into A, then M' would contain those extra changes whereas '--recreate-merges' with '-s ours' (once it supports it) would not. > > Let`s see our starting situation: > > (0) ---X8--B2'--X9 (master) > |\ > | A1---A2---A3 (A) > | \ > | M (topic) > | / > \-B1---B2---B3 (B) > > > Here, merge commit M is done with `-s ours` (obsoleting branch "B"), > plus amended to make it an "evil merge", where a commit B2 from > obsoleted branch "B" is cherry picked to "master". > > Now, we want to rebase "topic" (M) onto updated "master" (X9), but to > make things more interesting, we`ll do it interactively, with some > amendments, drops, additions and even more cherry-picks! > > This is what the final result looks like: > > (1) ---X8--B2'--X9 (master) > |\ > | A12--A2'---B3' (A) > | \ > | M' (topic) > | / > \-B1'--B3'---B4 (B) > > > During interactive rebase, on branch "A", we amended A1 into A12, > dropped A3 and cherry-picked B3. On branch "B", B4 is added, B2' being > omitted automatically as already present in "master". > > So... In comparison to original merge commit M, rebased merge commit > M' is expected to: > > - Add X9, from updated "master" > - Have A1 changed to A12, due to A12 commit amendment > - Keep A2, rebased as A2' > - Remove A3, due to dropped A3 commit > - Keep amendment from original (evil) merge commit M > - Miss B1' like M does B, due to original `-s ours` merge strategy > - Add B2, cherry-picked as B2' into "master" > - Add B3, cherry-picked as B3' into "A" > - Add B4, added to "B" > - Most important, provide safety mechanism to "fail loud", being >aware of non-trivial things going on, allowing to stop for user >inspection/decision > > > There, I hope I didn`t miss any expectation. And, it _seems_ to work > exactly as expected :D > > Not to leave this to imagination only, and hopefully helping others > to get to speed and possibly discuss this, pointing to still possible > flaws, I`m adding a demo script[1], showing how this exact