* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-28 19:26 ` Jeff King
@ 2008-07-28 20:00 ` Avery Pennarun
2008-07-28 23:27 ` Johannes Schindelin
2008-07-29 0:37 ` Junio C Hamano
2 siblings, 0 replies; 28+ messages in thread
From: Avery Pennarun @ 2008-07-28 20:00 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, sverre, Git Mailinglist, Miklos Vajna
On 7/28/08, Jeff King <peff@peff.net> 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
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-28 19:26 ` Jeff King
2008-07-28 20:00 ` Avery Pennarun
@ 2008-07-28 23:27 ` Johannes Schindelin
2008-07-29 0:09 ` Sverre Rabbelier
2008-07-29 4:38 ` Jeff King
2008-07-29 0:37 ` Junio C Hamano
2 siblings, 2 replies; 28+ messages in thread
From: Johannes Schindelin @ 2008-07-28 23:27 UTC (permalink / raw)
To: Jeff King; +Cc: sverre, Git Mailinglist, Miklos Vajna
Hi,
On Mon, 28 Jul 2008, Jeff King wrote:
> On Mon, Jul 28, 2008 at 08:09:55PM +0100, Johannes Schindelin 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.
>
> 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
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-28 23:27 ` Johannes Schindelin
@ 2008-07-29 0:09 ` Sverre Rabbelier
2008-07-29 4:31 ` Jeff King
2008-07-29 4:38 ` Jeff King
1 sibling, 1 reply; 28+ messages in thread
From: Sverre Rabbelier @ 2008-07-29 0:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Git Mailinglist, Miklos Vajna
On Tue, Jul 29, 2008 at 01:27, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> To me, this suggests that they were too married to 'production' being the
> "dominant" branch.
<snip>
> If the result should become the state of 'x', too, I would then just
> 'git push origin y:x'.
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.)
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 0:09 ` Sverre Rabbelier
@ 2008-07-29 4:31 ` Jeff King
0 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2008-07-29 4:31 UTC (permalink / raw)
To: sverre; +Cc: Johannes Schindelin, Git Mailinglist, Miklos Vajna
On Tue, Jul 29, 2008 at 02:09:18AM +0200, Sverre Rabbelier wrote:
> > If the result should become the state of 'x', too, I would then just
> > 'git push origin y:x'.
>
> 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.)
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-28 23:27 ` Johannes Schindelin
2008-07-29 0:09 ` Sverre Rabbelier
@ 2008-07-29 4:38 ` Jeff King
2008-07-29 11:05 ` Johannes Schindelin
1 sibling, 1 reply; 28+ messages in thread
From: Jeff King @ 2008-07-29 4:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: sverre, Git Mailinglist, Miklos Vajna
On Tue, Jul 29, 2008 at 01:27:44AM +0200, Johannes Schindelin wrote:
> > So the logical sequence was:
> >
> > git checkout production
> > git merge -s theirs master
>
> To me, this suggests that they were too married to 'production' being the
> "dominant" branch.
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 4:38 ` Jeff King
@ 2008-07-29 11:05 ` Johannes Schindelin
2008-07-29 12:36 ` Jeff King
0 siblings, 1 reply; 28+ messages in thread
From: Johannes Schindelin @ 2008-07-29 11:05 UTC (permalink / raw)
To: Jeff King; +Cc: sverre, Git Mailinglist, Miklos Vajna
Hi,
On Tue, 29 Jul 2008, Jeff King wrote:
> On Tue, Jul 29, 2008 at 01:27:44AM +0200, Johannes Schindelin wrote:
>
> > > So the logical sequence was:
> > >
> > > git checkout production
> > > git merge -s theirs master
> >
> > To me, this suggests that they were too married to 'production' being
> > the "dominant" branch.
>
> Perhaps. But I see this as an operation on the production branch: "pull
> in master's changes, forgetting ours".
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.
> 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.
But why? Isn't the _content_ of "master" what we want?
> Not to mention that we might not _control_ master.
This is Git. We control all local branches.
> 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.
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 11:05 ` Johannes Schindelin
@ 2008-07-29 12:36 ` Jeff King
2008-07-29 12:42 ` Sverre Rabbelier
0 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2008-07-29 12:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: sverre, Git Mailinglist, Miklos Vajna
On Tue, Jul 29, 2008 at 01:05:11PM +0200, Johannes Schindelin wrote:
> > Perhaps. But I see this as an operation on the production branch: "pull
> > in master's changes, forgetting ours".
>
> 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.
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.
> > 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.
>
> But why? Isn't the _content_ of "master" what we want?
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?).
> > Not to mention that we might not _control_ master.
>
> This is Git. We control all local branches.
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).
> 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.
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 12:36 ` Jeff King
@ 2008-07-29 12:42 ` Sverre Rabbelier
0 siblings, 0 replies; 28+ messages in thread
From: Sverre Rabbelier @ 2008-07-29 12:42 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, Git Mailinglist, Miklos Vajna
On Tue, Jul 29, 2008 at 14:36, Jeff King <peff@peff.net> wrote:
<good explanation of what I meant snipped>
> 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.
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.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-28 19:26 ` Jeff King
2008-07-28 20:00 ` Avery Pennarun
2008-07-28 23:27 ` Johannes Schindelin
@ 2008-07-29 0:37 ` Junio C Hamano
2008-07-29 5:02 ` Jeff King
2 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2008-07-29 0:37 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, sverre, Git Mailinglist, Miklos Vajna
Jeff King <peff@peff.net> writes:
> 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.
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 0:37 ` Junio C Hamano
@ 2008-07-29 5:02 ` Jeff King
2008-07-29 9:36 ` Mike Ralphson
0 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2008-07-29 5:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, sverre, Git Mailinglist, Miklos Vajna
On Mon, Jul 28, 2008 at 05:37:33PM -0700, Junio C Hamano wrote:
> 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.
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".
> 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.
Right, that is just what I wanted.
> 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.
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.
> 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.
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 5:02 ` Jeff King
@ 2008-07-29 9:36 ` Mike Ralphson
2008-07-29 12:42 ` Jeff King
0 siblings, 1 reply; 28+ messages in thread
From: Mike Ralphson @ 2008-07-29 9:36 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Johannes Schindelin, sverre, Git Mailinglist,
Miklos Vajna
2008/7/29 Jeff King <peff@peff.net>:
> On Mon, Jul 28, 2008 at 05:37:33PM -0700, Junio C Hamano wrote:
>
> 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.
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: theirs/ours was Re: [PATCH 6/6] Add a new test for using a custom merge strategy
2008-07-29 9:36 ` Mike Ralphson
@ 2008-07-29 12:42 ` Jeff King
0 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2008-07-29 12:42 UTC (permalink / raw)
To: Mike Ralphson
Cc: Junio C Hamano, Johannes Schindelin, sverre, Git Mailinglist,
Miklos Vajna
On Tue, Jul 29, 2008 at 10:36:32AM +0100, Mike Ralphson wrote:
> > 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.
>
> Is there any reason you couldn't have reverted the stable commits in
> preparation for the merge from devel?
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
^ permalink raw reply [flat|nested] 28+ messages in thread