patch, 6 partstheirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy

On Mon, Jul 28, 2008 at 15:12, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

I've been wanting to mail about this for a few days now, but didn't really know how to bring it up, this seems a good opportunity.

It has happened a few times on #git already that someone asked for the merge strategies described above (e.g., _not_ the insane ones) for what I deemed to be valid use cases. (The main reason was that they wanted to merge with a conflicting branch, discarding the current master, but still allowing people to 'git pull'.)

I was wondering what to tell those people? Will there ever be such a version of 'merge theirs' (that is the strict opposite of 'ours')? Or should they do:

$git checkout otherbranch $git merge -s ours master $git checkout master $git merge otherbranch

Thus resulting in a 'wrong way around' merge as part of master? It would say "Merge branch 'master' into otherbranch", while what happened was "Merge branch 'otherbranch' into master".

So, in short: what does the list think about adding "git-merge-theirs", that does (although possibly less 'hackish'):

cat > git-merge-theirs << EOF #!/bin/sh eval git read-tree --reset -u \\\$\$# EOF

On Mon, Jul 28, 2008 at 04:54:17PM +0200, Sverre Rabbelier <alturin@gmail.com> wrote:

Isn't this the stupid one?

It's perfect for my testing needs, but this is not something that people should ever use on a real repo.

On Mon, Jul 28, 2008 at 20:14, Miklos Vajna <vmiklos@frugalware.org> wrote:

No, the stupid one did "take all non-conflicting hunks from our side, and any for conflicting hunks, take theirs", which was rather silly I must say, although I have heard one use-cases where it makes sense (no I don't think we should have a git-merge-theirs-on-conflict).

What about the use-case I described in my first mail?

On Mon, Jul 28, 2008 at 04:54:17PM +0200, Sverre Rabbelier wrote:

I ran into this exact situation while showing somebody how awesome git was, and it was a little embarrasing to say "oops, now we have to do this backwards."

So I think it would be nice for completeness, although I admit that my situation was rare (but no rarer, perhaps, than "-s ours").

-Peff

PS You may find your shell snippet a bit more readable by quoting 'EOF'.

Hi,

On Mon, 28 Jul 2008, Jeff King wrote:

Well, I have to say that the workflow is a bit backwards if the person who _publishes_ the thing is the one saying "Ooops, my version no goodie, other version please, but so that pull still works".

I would have expected the one who has the good version to make the choice.

Ciao, Dscho

On Mon, Jul 28, 2008 at 08:09:55PM +0100, Johannes Schindelin wrote:

My situation was two long-running branches, "stable" and "devel", both of which were worked on by many developers. One person was in charge of integration and branch management. They wanted "stable" to get the contents of "devel" (which were now ready for release), ignoring any small fixes that had been done on "stable" (since they had all been moved over to "devel" previously, but in subtly different ways that would create conflicts). And "git reset" was not an option, because they wanted to keep the history of "stable" in case those fixes needed to be looked at later.

So the logical sequence was:

  git checkout production
  git merge -s theirs master

-Peff

On 7/28/08, Jeff King <peff@peff.net> wrote:

I have to say, this somehow feels wrong to me. What you're saying is essentially that "stable has already been merged into devel" followed by "and now we want to catch stable up to devel."

It really is two separate thoughts, and merging devel directly into stable - literally by *undoing* all the changes from stable - doesn't sound like it should be considered a safe operation.

Personally, I've started enjoying the "--no-ff" option to git-merge. That way I can do

   git checkout master
   git merge production
   git checkout production
   git merge --no-ff master

The latter merge isn't really a "merge" since it could have been just fast forwarded. But it avoids the aesthetic problems of commits like "merge production into master" showing up in the master branch. It also means that "git reset --hard HEAD^" works whether or not a fastforward would have been theoretically possible.

Of course, this whole discussion is really just about how to make your log look cleaner, and we could debate forever about that. It may make sense to simply provide "theirs" as an exact mirror of "ours" if only in the name of symmetry.

Have fun,

Avery

Hi,

On Mon, 28 Jul 2008, Jeff King wrote:

To me, this suggests that they were too married to 'production' being the "dominant" branch.

Thing is: they had two branches. They should be merged, but one should prevail: 'master'.

So if I have two branches, say "x" and "y", and I want to merge them, but really throw away the tree of "x", I would check out 'y', naturally. Then 'git merge -s ours x'.

If the result should become the state of 'x', too, I would then just 'git push origin y:x'.

Maybe I am "Git-braindead" by now, so that you can make fun of me like I used to make fun of CVSers and SVNers...

Ciao, Dscho

On Tue, Jul 29, 2008 at 01:27, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

<snip>

But this means that everybody doing a 'git pull' on that repo will get complaints when pulling, right? Should they just send out a message to all their users that they'll need to rebase all their changes now? (Not being sarcastic, am trying to work out what the recommended workflow is here.)

On Tue, Jul 29, 2008 at 02:09:18AM +0200, Sverre Rabbelier wrote:

I think you are missing the fact that he is doing this push _after_ having merged the history into master via "-s ours". So it is a fast-forward to push at that point.

-Peff

On Tue, Jul 29, 2008 at 01:27:44AM +0200, Johannes Schindelin wrote:

Perhaps. But I see this as an operation on the production branch: "pull in master's changes, forgetting ours". In your workflow (git checkout master && git merge -s ours production && git push origin master:production) we perform an operation on master, which doesn't seem as intuitive to me.

Not to mention that we might not _control_ master. What about (and I think Sverre mentioned something like this previously):

 I forked the kernel and made some changes. Some of my changes got
 applied upstream. The others are now obsolete. Now I want to bring
 myself in sync with Linus, but I want to keep my history (either
 because the history is interesting to me, or because others are basing
 their work on it).

Then your workflow, while still possible within the local repository, means you are munging the "linus" branch, which seems wrong. That branch is probably even just a tracking branch, which you would not want to build on, anyway.

-Peff

Hi,

On Tue, 29 Jul 2008, Jeff King wrote:

First of all, I cannot say how wrong it is to forget any changes in a production branch without proper explanation. I.e. without a commit message explaining _why_ the change was wrong to begin with.

It is messy at best, and I am happy that Git does not make that easy.

But why?  Isn't the _content_ of "master" what we want?

This is Git.  We control all local branches.

No, this workflow almost _dictates_ a plain "pull" into your local branch. The fact that a few commits were applied to upstream usually only means that your merge succeeds trivially, since the merged branches contain the _same_ changes.

Ciao, Dscho

On Tue, Jul 29, 2008 at 01:05:11PM +0200, Johannes Schindelin wrote:

Of course; I even mentioned the same in another part of the thread. But that isn't a difference between "ours" and "theirs"; any time you are discarding some changes, you should mention why.

Sure, which means we must _read_ from master. But you are _changing_ master. Whereas I view this as an operation on the production branch.

Please don't misunderstand me. I am not saying your way of thinking about it is wrong (or even less right than mine). What I have been trying to say this whole thread is that it is reasonable for a user to model the goal as I have described, and that git can easily support the direct implementation of achieving that goal (which is what Sverre asked originally -- is this useful to people?).

Sort of. Consider the kernel example I gave. A "linus" branch represents "this is where Linus is." But that _isn't_ where Linus is if you have added an extra merge commit to it. So either we throw away the change made to the "linus" branch, or we forever have extra merges that Linus does not have.

So yes, obviously you can do whatever you like with your local branches. But you complained in my example that the "production" branch was unnecessarily being treated as "dominant". My example was meant to indicate that the "thrown away" branch is dominant for a reason (in this case, it is my work branch, while the other is a tracking branch).

I don't see the point in talking about "usually". In the scenario in which I used it, the merge _didn't_ succeed trivially. Of course, usually you would not use "-s theirs". But the question was "is this ever useful?" and my answer was "rarely, but here is an example of when I wanted it."

If you are using "-s theirs" frequently, you are probably doing something wrong. But that doesn't mean it is wrong for it to exist.

-Peff

On Tue, Jul 29, 2008 at 14:36, Jeff King <peff@peff.net> wrote: <good explanation of what I meant snipped>

Exactly, thank you for that :). I hope it is clear to everybody what I meant now, although it seems that especially Junio and Dscho feel 'git-merge-theirs' should not be part of git in the suggested form.

Jeff King <peff@peff.net> writes:

I sense a slightly broken workflow here, whether the "-s theirs" strategy is used or the merge is done in the other direction using "-s ours" strategy.

Remember, when you create a merge commit between one history and another, you are making this statement:

    I have looked at the tree state and the development history behind
    both of these commits, and came up with this tree, which I believe
    suits the purpose of _my_ history better than either of them.

That is why, after making such a merge with "git merge other", you won't see any output from "git log ..other", which asks "what do I have yet to merge?" Everything that was included in other is now in your history and there is nothing you have to worry about having left out anymore.

So if you suspect that the sutuation "in case those fixes needed to be looked at later" ever arises, such a merge should *not* be recorded as a proper merge on the 'stable' branch, because at that point when you are doing that "-s theirs" merge (and this applies equally to the case where you make "-s ours" merge as well), you actually have not looked at "those fixes" closely enough to make the above statement with confidence.

Having said that, that "looking back in history" can easily be done if you mark such a "Use '-s theirs' for expediency" merge as potentially an iffy one in its commit log message somewhere. Later if you actually hit issues, you can locate such a merge commit, and inspect the output from "git log $commit^2..$commit^1". You would see those fixes the "devel" history did not have in the "stable" branch when such a merge was made.

So the above is not a fundamental objection to the approach, and that is why I said "slightly broken". With a proper explanation between the right use case (I think what you outlined is an example of good practice) and the wrong use case (for example, the one described in $gmane/89024, the whole thing after 'I think "-s theirs" is even worse.', not just the part that was quoted in $gmane/89178), I think it is Ok to have "-s theirs" strategy in our toolset.

Even though having said all of the above, I would actually prefer such a "pull all of the devel down to stable" be done with this workflow instead:

 (1) go to 'devel';
 (2) merge all of 'stable';
 (3) look at the result and prove it is perfect;
 (4) go to 'stable';
 (5) merge 'devel'.

The last step would be a fast-forward, and you do not need "-s theirs" anywhere in this procedure. Step (2) can be helped with "-s ours" (which have the same issue I discussed above), but the result is checked before it hits the 'stable' (presumably more precious branch), which is conceptually a big difference. This is where the existing asymmetry between theirs and ours comes from.

Incidentally, this is how 'maint' skips to tip of 'master' after a new major version is released, but 'maint' is merged up into 'master' often enough that we rarely need to even use "ours" strategy.

On Mon, Jul 28, 2008 at 05:37:33PM -0700, Junio C Hamano wrote:

Right, that is precisely what I wanted to say. These are the histories of the devel and stable branches, and now they both are the contents of stable. In this case, "I have looked at all of the commits in stable that were _not_ in devel, and confirmed that they have moral equivalents in devel".

Right, that is just what I wanted.

No, I had looked at them with confidence. I just didn't want history thrown away for two reasons:

  - historical interest; some of the commits had counterparts in devel
    that were done differently (because the two branches had diverged),
    but it might later be interesting to see how and why the stable
    changes were done (e.g., if a similar situation arose)

  - this project did not rebase, favoring the simplicity of "git pull"
    over clean history.

Bear in mind that this project was not very big. I think devel had ~20 commits, and stable had about ~5. So it was easy to get such confidence.

Of course you can do this (and that is, in fact, exactly what I did). But there is no point discussing "what hits stable" since neither branch is precious. All of this is happening in a private repo that nobody is looking at. So it really is a case of thinking about it as "devel subsumes stable, and then stable becomes devel" versus "stable is discarded in favor of master".

I think both are equally valid ways of looking at what is happening. The only differences will be:

  - the commit message will be reversed ("Merge X into Y"). And this
    really comes down to "how would I want to see this presented in 3
    months when I look at it?".  And either is valid, depending on how you
    think of the problem (but I think in both cases, you owe it to
    future readers to write a bit of text saying _why_ such a strategy
    was OK to use).

  - the parents will be swapped. Using "-s theirs" should let you ask
    "what changes did I make on my stable branch" using
    "--first-parent". I don't know how useful that is, as I don't
    actively work on that project anymore.

-Peff

2008/7/29 Jeff King <peff@peff.net>:

Is there any reason you couldn't have reverted the stable commits in preparation for the merge from devel?

I.e. these commits were necessary to fix problems in production, they now need to be reverted in order to cleanly apply the changes for the next stable version, which includes fixes for all of these problems.

I can see you'd be preserving twice as much history instead of throwing any away, but if scalability became an issue, you could always squash all the reverts into one pre-merge commit.

git-merge-theirs-revert anyone?

Mike

On Tue, Jul 29, 2008 at 10:36:32AM +0100, Mike Ralphson wrote:

No, there is no technical reason. I think that is a perfectly valid way of accomplishing the same goal (as is switching to the "kept" branch and using "-s ours"). It's just that we had a particular mental model, and the simplest way of translating that model into a git history graph was as I described.

Again, I don't think this is a common problem, and I have certainly not been aching for "-s theirs". The question was whether such a thing might be useful, and I think it is, if only because it most directly matches how a user might be thinking of the problem; for other users, or other similar situations, one of the other methods might make more sense.

To me, seeing a commit that joins two histories with a comment saying "these two branches are now becoming the same, but we don't care about what happened down this ancestry chain because of X" most directly models what happened (in my case).

-Peff

On Mon, Jul 28, 2008 at 21:09, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Why so? In this case the other branch was also owned by the publishing person. I don't quite follow how this is any stranger than ours? (Which is stranger to me, why would you want to merge in a branch if you're not going to do anything with it anyway? I'm sure there are valid workflows for it, which is why we have it, just saying that I think 'theirs' makes more sense to me than 'ours')

Why have the person with the good version merge with... a bad version? Isn't it usually "I will merge with you, because I know your branch makes things go twice as fast" (paraphrasing Linus from his git talk at google).

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

That reminds me of:

    http://thread.gmane.org/gmane.comp.version-control.git/89178

to one of whose messages I sent a response today.

On Mon, Jul 28, 2008 at 22:07, Junio C Hamano <gitster@pobox.com> wrote:

Mhhh, but the proposed strategy there was in response to the 'insane' git-merge-theirs version, not to the 'exact opposite of git-merge-ours' that I refer to now, yes? Do you have any particular feelings wrt to that?

"Sverre Rabbelier" <alturin@gmail.com> writes:

No.

On Mon, Jul 28, 2008 at 22:20, Junio C Hamano <gitster@pobox.com> wrote:

Nanako Shiraishi's patch was not in response to the "git-merge-theirs" thread, or I am missing something here....?

"Sverre Rabbelier" <alturin@gmail.com> writes:

The quoted sentence by me in that message was after I explained why "per hunk theirs" aka "-Xtheirs" was not such a great idea I further went on to say "by the way, '-s theirs' is even worse and here is why".

Junio C Hamano <gitster@pobox.com> writes:

Heh, I ended up doing the "digging" myself.   The quote came from this:

    http://thread.gmane.org/gmane.comp.version-control.git/89010/focus=89024

and "tried not to sound too negative" there refers to:

    http://thread.gmane.org/gmane.comp.version-control.git/89010/focus=89021

which _was_ about the "-Xtheirs", not "-s theirs".

On Mon, Jul 28, 2008 at 23:35, Junio C Hamano <gitster@pobox.com> wrote:

Aaah, ok, now I see where my confusion came from. Thank you for clarifying that. Then remains the question, what to tell those that want '-s theirs' not to keep track of their changes, but to keep a fast-forwardable master?

Understood, it makes sense now.

On Mon, Jul 28, 2008 at 02:16:35PM -0700, Junio C Hamano wrote:

Your reason was "it keeps your crap in the history". And while I generally am in favor of getting rid of crap and keeping a clean history, I think it is very much dependent on the individual project's preferences. IOW, that history might not contain "crap" but rather now-obsolete changes that are of historical interest.

But I do agree that -Xtheirs is crap. ;)

-Peff

Jeff King <peff@peff.net> writes:

Yes, that is why I did not merge 'master' with "theirs" merge into it to subsume it. I reverted -Xtheirs from 'next' (due to "never-rewind during the cycle" rule) and intend to rebuild 'next' without it when 1.6.0 ships.

However, that does not keep me from holding onto its tip privately (and I do, as the machinery to pass -Xoption through git-merge to backends would be useful later).

IOW, "now-obsolete changes that are of historical interest" does not necessarily justify a "subsuming" merge using "-s ours" or "-s theirs".