gerrit: enabling content merging

561 views
Skip to first unread message

Brian Harring

unread,
Jan 9, 2012, 11:24:59 PM1/9/12
to Chromium OS dev
Gerrit's traditional merging mode is what's called trivial merging; specifically, if there are two commits and neither is the parent of each other, then they can only be merged/cherry-picked if they modify completely seperate files.  This mode is fairly painful as people are probably aware- rebasing being required.  Additionally, depending on how gerrit merged the patch, even if one patch was the parent of the other a rebase may still be needed.

Gerrit 2.1.6 supports what they call 'content merging'; basically 3 way merging, the git norm.  I'd like to see this turned one for the vast majority of our repositories.

What are folks views on this?  In a past project I've dealt with this feature being on for ~6 months w/ 40+ devs- never saw an issue that trivial merging would've caught.  I did however see far less wasting of peoples time and less frustration over forcing people to manually do trivial rebases- thus why I'd like it enabled for our uses.

Comments?

Olof Johansson

unread,
Jan 9, 2012, 11:39:17 PM1/9/12
to Brian Harring, Chromium OS dev
Hi,


My main concern here is that repo history will no longer be linear.
It's not an issue for most of trees, but it won't work well on kernel
where rebasing will become a pain.

For other repos, and for chromiumos-overlay in particular it sounds
like a good idea.


-Olof

Brian Harring

unread,
Jan 10, 2012, 3:01:48 PM1/10/12
to Olof Johansson, Chromium OS dev
My personal experience is that this isn't any additional pain when it comes to rebasing; the rebasing itself being 99% of the pain.  When you go to do the rebase, git will force linearization of the revs you intend to transplant- dropping any merge commits created (as said, it linearizes the target range).  For the rebase operation, you can either give the from/root or have git figure it out on its own- while it's possible for it to miscalculate the root, for it to occur it requires some fairly special revs- the scenario that comes to mind is if a dev uploaded a rev w/ an ancient parent (older than the root) which ought to be caught during review anyways.  Either way, I've not seen it actually occur in practice; plus for operations of this sort if you're explicit as to the origination point, git should linearize the merge correctly since you'll be giving it enough info for it to know whether to choose the left or right parent.

Again, that's my personal experience in dealing w/ android kernels while using content-merging, and doing some fairly nasty rebasing (transplanting the android patchset to new stable versions, while transplanting our own fairly massive patchset in addition).

Our kernel workflow may differ in some key aspect here that I'm overlooking- if you can point me at a doc/thread for how we do our kernel rebasing that would help.

Realistically, if y'all aren't running into the trivial rebase workflow issue for the kernel this discussion can be punted until that time if it's y'alls preference.
~brian

Chris Sosa

unread,
Jan 10, 2012, 3:04:29 PM1/10/12
to Brian Harring, Olof Johansson, Chromium OS dev, Darin Petkov, Richard Barnette
+Richard, Darin who had some reservations originally when discussed with them.

> --
> Chromium OS Developers mailing list: chromiu...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-os-dev?hl=en

Mike Frysinger

unread,
Jan 10, 2012, 3:06:50 PM1/10/12
to Olof Johansson, Brian Harring, Chromium OS dev

i thought gerrit operated on cherry-picks and not merges. and the
proposal was to change the gerrit settings to use a more liberal
cherry-pick method. so there shouldn't be a change in the linearity
of the history. or is my understanding wrong ?
-mike

Brian Harring

unread,
Jan 10, 2012, 4:50:53 PM1/10/12
to Mike Frysinger, Olof Johansson, Chromium OS dev
You'd have to be a bit clearer about 'liberal cherry-pick' but you're likely pretty close.  Content-merging is basically threeway merging.  Basically enabling patch fuzziness, and turning off the "any modification to a file we're modifying must be done by a direct parent" restriction.

If gerrit can't FF the rev, and can't cherry-pick it, if content-merging is available and the merge is successful it'll generate a merge commit pulling in the original rev, and commit that set.  This is what they're worrying about- the fact gerrit in the worst case will generate a merge commit there.  This is just how git does it- original rev is preserved, and a glue/merge commit is generated.  Linearizing that shouldn't be an issue however since implicitly you're changing the rev anyways (parentage changes).
~brian

Brian Harring

unread,
Jan 12, 2012, 8:36:54 PM1/12/12
to Richard Barnette, Darin Petkov, Chromium OS dev
Any other comments?  People in the To: apparently had past criticisms, so now would be the time to speak up.

No response nor further issues raised means I'll be pinging infra for this EOW.

~brian

Darin Petkov

unread,
Jan 13, 2012, 3:18:34 AM1/13/12
to Brian Harring, Richard Barnette, Chromium OS dev


On Fri, Jan 13, 2012 at 9:16 AM, Darin Petkov <pet...@google.com> wrote:

On Fri, Jan 13, 2012 at 2:36 AM, Brian Harring <ferr...@google.com> wrote:
Any other comments?  People in the To: apparently had past criticisms, so now would be the time to speak up.

No comments/objections on my side.

Darin

Antoine Labour

unread,
Jan 13, 2012, 12:38:27 PM1/13/12
to Brian Harring, Richard Barnette, Darin Petkov, Chromium OS dev
On Thu, Jan 12, 2012 at 5:36 PM, Brian Harring <ferr...@google.com> wrote:
Any other comments?  People in the To: apparently had past criticisms, so now would be the time to speak up.

No response nor further issues raised means I'll be pinging infra for this EOW.

~brian

My concerns of submitting merges, that I evoked when we switched to gerrit, are that:
- it becomes a lot harder to revert
- it becomes a lot harder to bisect
- it becomes a lot harder to cherry-pick to a release branch
- the developer's local state ends up polluting the tree
- for upstream repos it becomes harder to drop patches when merging/rebasing upstream

Overall, it feels like to gain a bit of convenience for the developer who submits his work, we add a large cost to the commons. So I'm not sure it's worth it.

Could we instead allow gerrit to do the rebase (if it has no conflict)? It should be about equivalent as the merge path in terms of what goes in without developer interaction, but preserves the nice properties of a linear tree.
For that matter, that's essentially how the chrome CQ behaves.

Antoine
 

Jonathan Kliegman

unread,
Jan 13, 2012, 1:23:08 PM1/13/12
to Antoine Labour, Brian Harring, Richard Barnette, Darin Petkov, Chromium OS dev
On Fri, Jan 13, 2012 at 12:38 PM, Antoine Labour <pi...@chromium.org> wrote:
On Thu, Jan 12, 2012 at 5:36 PM, Brian Harring <ferr...@google.com> wrote:
Any other comments?  People in the To: apparently had past criticisms, so now would be the time to speak up.

No response nor further issues raised means I'll be pinging infra for this EOW.

~brian

My concerns of submitting merges, that I evoked when we switched to gerrit, are that:
- it becomes a lot harder to revert
- it becomes a lot harder to bisect
- it becomes a lot harder to cherry-pick to a release branch
- the developer's local state ends up polluting the tree
- for upstream repos it becomes harder to drop patches when merging/rebasing upstream

Overall, it feels like to gain a bit of convenience for the developer who submits his work, we add a large cost to the commons. So I'm not sure it's worth it.

Could we instead allow gerrit to do the rebase (if it has no conflict)? It should be about equivalent as the merge path in terms of what goes in without developer interaction, but preserves the nice properties of a linear tree.
For that matter, that's essentially how the chrome CQ behaves.

Antoine
I'm definitely in favor of gerrit doing an automatic rebase if it can safely do so and commit.  I'd really like to avoid having merges show up in my local repo (if that's what this change does - the under the covers parts of gerrit are a bit of a mystery to me).

Mandeep Singh Baines

unread,
Jan 13, 2012, 1:42:26 PM1/13/12
to Antoine Labour, Brian Harring, Richard Barnette, Darin Petkov, Chromium OS dev
Antoine Labour (pi...@chromium.org) wrote:
> On Thu, Jan 12, 2012 at 5:36 PM, Brian Harring <ferr...@google.com> wrote:
>
> > Any other comments? People in the To: apparently had past criticisms, so
> > now would be the time to speak up.
> >
> > No response nor further issues raised means I'll be pinging infra for this
> > EOW.
> >
> > ~brian
> >
>
> My concerns of submitting merges, that I evoked when we switched to gerrit,
> are that:
> - it becomes a lot harder to revert
> - it becomes a lot harder to bisect
> - it becomes a lot harder to cherry-pick to a release branch
> - the developer's local state ends up polluting the tree
> - for upstream repos it becomes harder to drop patches when
> merging/rebasing upstream

- it makes gitk almost unusable

>
> Overall, it feels like to gain a bit of convenience for the developer who
> submits his work, we add a large cost to the commons. So I'm not sure it's
> worth it.
>
> Could we instead allow gerrit to do the rebase (if it has no conflict)? It
> should be about equivalent as the merge path in terms of what goes in
> without developer interaction, but preserves the nice properties of a
> linear tree.
> For that matter, that's essentially how the chrome CQ behaves.
>

+1 (on no merge commits) For all the reasons piman pointed out.

I'm late to this thread. What problem are we solving by allowing merge
commits? And can't it be solved by gerrit rebasing the topic branch.

Regards,
Mandeep

Brian Harring

unread,
Jan 13, 2012, 2:34:38 PM1/13/12
to Antoine Labour, Mandeep Singh Baines, Richard Barnette, Darin Petkov, Chromium OS dev
On Fri, Jan 13, 2012 at 9:38 AM, Antoine Labour <pi...@chromium.org> wrote:
On Thu, Jan 12, 2012 at 5:36 PM, Brian Harring <ferr...@google.com> wrote:
Any other comments?  People in the To: apparently had past criticisms, so now would be the time to speak up.

No response nor further issues raised means I'll be pinging infra for this EOW.

~brian

My concerns of submitting merges, that I evoked when we switched to gerrit, are that:

To be absolutely clear, I'm not suggesting people upload merges: gerrit review of a merge commit is pretty worthless.

What I'm suggesting is purely that gerrit generate a merge commit if a trivial rebase was required.  Single node merge.  Basically the equivalent of your average cherry-pick invocation (which does 3way), just done w/ a single rev merge so history is properly preserved.


- it becomes a lot harder to revert
 
Single node merge; reversion is the same potential level of pain.

- it becomes a lot harder to bisect

Bisection linearizes history without any problems here.

Keep in mind that bisection w/ git-repo is basically impossible most of the time anyways- if you're lucky and the bug is contained w/in a single repo (say the kernel), you can do bisection.  With our manifest.xml usage which doesn't lock the rev of the target git-repo, there is no atomic view of the repo checkout as a whole, thus bisection isn't possible.  Git submodules, or switching to lkgm snapshots would restore that- regardless, as I indicated, bisection should behave fine here anyways.

 
- it becomes a lot harder to cherry-pick to a release branch

Pretty sure you're thinking of merging N revs rather than a single; cherry-picking the one rev across to a seperate lineage is going to be basically the same level of fun.

Other thing to keep in mind is that cherry-picking is able to adjust the parentage- meaning it can do 3way, not trivial... so the logic of this critique reinforcing content-merging from my standpoint. :)

 
- the developer's local state ends up polluting the tree

Please clarify.
 

- for upstream repos it becomes harder to drop patches when merging/rebasing upstream

Again, rebase will linearize, meaning the merge commits will effectively fall out (since rebase is rewriting history, git doesn't need the merge commit to bind the original rev into history- it just rewrites the original rev parentage to point at the other parent of the merge commit).

Meaning this is effectively the same situation we're in now; the content of what you put in the branch is what makes dealing w/ upstream repos hard for rebasing, not the usage of a single rev merge commit.


Overall, it feels like to gain a bit of convenience for the developer who submits his work, we add a large cost to the commons. So I'm not sure it's worth it.

This is a fair bit more than just convenience; personal example, over the holidays I generated 5-6 different feature sets for chromite.  At least 4 of them all touched the same file in a fashion that was mergable.

Now, if one is using gerrit /correctly/, feature 1 is derived from HEAD, feature 2 is derived from HEAD, etc.  You set the parentage/dependencies appropriately.  This however means each feature has to be one by one rebased as each goes in (N^2), or you try to induce some artificial ordering via parentage thus slowing up the changes landing, rather than allowing them to go in as they're ready.

This directly wasted my time, and reviewers time.  I could probably claim that having to continually juggle patches like this was the source of a bug or two in those commits additionally- although that could be attributed more to sloppyness in rebasing a change for the 5th time. ;)

Convenience to me is 'git commit --fixup', a feature that removes a couple of keystrokes from the rebase workflow; it's not "continually upload new copies of a patch via `git rebase -i` when I can make gerrit do the same thing"; the latter definition is busy work rather than convenience and is a source of potential bugs in my experience since people suck at doing busywork correctly.

 

Could we instead allow gerrit to do the rebase (if it has no conflict)? It should be about equivalent as the merge path in terms of what goes in without developer interaction, but preserves the nice properties of a linear tree.
For that matter, that's essentially how the chrome CQ behaves.

There is an alternate solution to trivial rebasing that involves using hooks to scan for conflicts, and attempting to do rebases and reuploading.  This has some negative properties however:
1) it's a pain in the ass setting it up, and requires custom code for functionality that already exists in gerrit's content-merging.
2) doing as you're suggesting destroys the historical angle for a patch.  If a patch lands and breaks things, knowing the parentage (aka, that the person tested history X) is no longer possible directly from git- have to go digging through gerrit to find what the original parentage was since automatic trivial rebasing scripts implicitly create a new history Y.
3) it's able to generates a fair amount of noise on gerrit- this isn't at all desirable since gerrit's already got a crappy signal/noise ratio.  Specifically, the scanner would either have to proactively do rebases as needed (which can screwup review since patchsets start piling up), or has to detect that Gerrit punted the patch, or worse, identify from CQ logs that it did, do the trivial rebasing, than flip the approvals all back to the same level.  To do it properly, this requires a bot w/ an admin account to make use of gerrit's suexec functionality to preserve the association of who rated the patch what.

Finally, addressing the "I don't want to see merges" crowd, `git log` has a helpful --no-merges option that collapses the history down.  gitk last I knew just passed any arguments it received directly to git log, so if you want to view a linear approximation, that's how you do it.  Note that the sort of merging I'm suggesting here isn't going to result in the sort of gitk uglyness that you get from projects like linux and git.

Counterarguments?  I realize my responses are a bit dense here- if I didn't clarify something well enough, please ask.
~brian

Mandeep Singh Baines

unread,
Jan 13, 2012, 3:46:54 PM1/13/12
to Brian Harring, Antoine Labour, Richard Barnette, Darin Petkov, Chromium OS dev

Thanks much for clarifying:) This clears up a lot of confusion for me.

However, I'm still unsure of this proposal. I still feel the merge
commits add some extra complexity to developers so there is a cost.

It would be interesting to measure the benefit. How often are folks
hitting the case where a merge commit would have prevented a failure?

In the cases where a cherry-pick would have failed but a merge would have
succeeded, maybe its not so bad that the developer has to take a second
look. There might be a logical conflict. So you are taking some risk
when auto-merging. You're about to checkin code which doesn't exactly
match what the developer testing.

Earlier, we used to only allow fast-forward merges in order to guarantee
that what was commited is what was tested. This turned out to be a pain
so we loosend things and moved to cherry-picking. This proposal losens
things even further.

Normally when I'm working on the same file, its in a topic branch and
not separate branches.

Regards,
Mandeep

> ~brian

Antoine Labour

unread,
Jan 13, 2012, 4:32:30 PM1/13/12
to Brian Harring, Mandeep Singh Baines, Richard Barnette, Darin Petkov, Chromium OS dev
On Fri, Jan 13, 2012 at 11:34 AM, Brian Harring <ferr...@google.com> wrote:
On Fri, Jan 13, 2012 at 9:38 AM, Antoine Labour <pi...@chromium.org> wrote:
On Thu, Jan 12, 2012 at 5:36 PM, Brian Harring <ferr...@google.com> wrote:
Any other comments?  People in the To: apparently had past criticisms, so now would be the time to speak up.

No response nor further issues raised means I'll be pinging infra for this EOW.

~brian

My concerns of submitting merges, that I evoked when we switched to gerrit, are that:

To be absolutely clear, I'm not suggesting people upload merges: gerrit review of a merge commit is pretty worthless.

What I'm suggesting is purely that gerrit generate a merge commit if a trivial rebase was required.  Single node merge.  Basically the equivalent of your average cherry-pick invocation (which does 3way), just done w/ a single rev merge so history is properly preserved.

I'm still not sure I understand what you mean by that. Do you mean:

I start developing, the tree is like:
a-b--trunk

I write my patch (x), commit it locally / upload it to gerrit:
a-b
   \-x

Meanwhile the tree moved on:

a-b-c-d---trunk
   \-x

When it commit you get:

a-b-c-d-m---trunk
   \-x-/

Is that what you have in mind by "single node merge"? I'm going to assume so in the discussion below.

First, it makes git log painful to follow. Because it shows log in linear fashion, it will interleave commits from the 2 branches (based on chronological order iirc), showing e.g. a,b,c,x,d,m.
So you can do git log --first-parent, but then you see a,b,c,d,m. In particular you see m (the merge), not x. I don't know what gerrit does for the merge commit, but unless it also copies the commit message, you lose information.

- it becomes a lot harder to revert
 
Single node merge; reversion is the same potential level of pain.

Well, you can't trivially revert the merge commit.
I guess you can revert x, but then you need to dig for it.


- it becomes a lot harder to bisect

Bisection linearizes history without any problems here.

If you have only one commit, I agree that it's ok, if you look at only the changes in the mainline (i.e. git log --first-parent), but that assumes you have enough info there (see above).
 
Keep in mind that bisection w/ git-repo is basically impossible most of the time anyways- if you're lucky and the bug is contained w/in a single repo (say the kernel), you can do bisection.  With our manifest.xml usage which doesn't lock the rev of the target git-repo, there is no atomic view of the repo checkout as a whole, thus bisection isn't possible.  Git submodules, or switching to lkgm snapshots would restore that- regardless, as I indicated, bisection should behave fine here anyways.

If you want to get technical (and you have enough time on your hands), you can bisect chromiumos-overlay, find the offending ebuild bump, find the repo that it references, and bisect that.


 
- it becomes a lot harder to cherry-pick to a release branch

Pretty sure you're thinking of merging N revs rather than a single; cherry-picking the one rev across to a seperate lineage is going to be basically the same level of fun.

Again you have to dig for it. You can't cherry-pick m, you have to cherry-pick x.
 
Other thing to keep in mind is that cherry-picking is able to adjust the parentage- meaning it can do 3way, not trivial... so the logic of this critique reinforcing content-merging from my standpoint. :)

I'm not sure I understand what you are saying here. 

 
- the developer's local state ends up polluting the tree

Please clarify.

You were talking about committing merges, I didn't know you were talking about a single commit.

What happens if the developer has this locally:

a-b-c-d---trunk
   \-j-k-l--branch1
      \-x
and they upload/review/submit x?

- for upstream repos it becomes harder to drop patches when merging/rebasing upstream

Again, rebase will linearize, meaning the merge commits will effectively fall out (since rebase is rewriting history, git doesn't need the merge commit to bind the original rev into history- it just rewrites the original rev parentage to point at the other parent of the merge commit).

Meaning this is effectively the same situation we're in now; the content of what you put in the branch is what makes dealing w/ upstream repos hard for rebasing, not the usage of a single rev merge commit.


Overall, it feels like to gain a bit of convenience for the developer who submits his work, we add a large cost to the commons. So I'm not sure it's worth it.

This is a fair bit more than just convenience; personal example, over the holidays I generated 5-6 different feature sets for chromite.  At least 4 of them all touched the same file in a fashion that was mergable.

Now, if one is using gerrit /correctly/, feature 1 is derived from HEAD, feature 2 is derived from HEAD, etc.  You set the parentage/dependencies appropriately.  This however means each feature has to be one by one rebased as each goes in (N^2), or you try to induce some artificial ordering via parentage thus slowing up the changes landing, rather than allowing them to go in as they're ready.

I don't understand why this can't be done with a rebase instead of a merge for all commits that don't conflict? In which case would a merge succeed but a rebase wouldn't?


This directly wasted my time, and reviewers time.  I could probably claim that having to continually juggle patches like this was the source of a bug or two in those commits additionally- although that could be attributed more to sloppyness in rebasing a change for the 5th time. ;)

Convenience to me is 'git commit --fixup', a feature that removes a couple of keystrokes from the rebase workflow; it's not "continually upload new copies of a patch via `git rebase -i` when I can make gerrit do the same thing"; the latter definition is busy work rather than convenience and is a source of potential bugs in my experience since people suck at doing busywork correctly.

 

Could we instead allow gerrit to do the rebase (if it has no conflict)? It should be about equivalent as the merge path in terms of what goes in without developer interaction, but preserves the nice properties of a linear tree.
For that matter, that's essentially how the chrome CQ behaves.

There is an alternate solution to trivial rebasing that involves using hooks to scan for conflicts, and attempting to do rebases and reuploading.  This has some negative properties however:
1) it's a pain in the ass setting it up, and requires custom code for functionality that already exists in gerrit's content-merging.

I don't understand.
git rebase will fail with an error code in case of conflict. Just like git merge would. I don't understand why it'd be more difficult to implement the hook for git rebase if it's already there for git merge.
 
2) doing as you're suggesting destroys the historical angle for a patch.  If a patch lands and breaks things, knowing the parentage (aka, that the person tested history X) is no longer possible directly from git- have to go digging through gerrit to find what the original parentage was since automatic trivial rebasing scripts implicitly create a new history Y.

That's sort of what I mean by polluting the tree with the developer state.
I don't care that you wrote your patch x against b. What I (and every other developer) care about is that it went into the tree between d and m. I don't care that you tested against b, and when it merged it broke because it didn't work against d. Your contribution to the tree is the difference between d and m.
 
3) it's able to generates a fair amount of noise on gerrit- this isn't at all desirable since gerrit's already got a crappy signal/noise ratio.  Specifically, the scanner would either have to proactively do rebases as needed (which can screwup review since patchsets start piling up), or has to detect that Gerrit punted the patch, or worse, identify from CQ logs that it did, do the trivial rebasing, than flip the approvals all back to the same level.  To do it properly, this requires a bot w/ an admin account to make use of gerrit's suexec functionality to preserve the association of who rated the patch what.

Why does this have to be apparent at all in gerrit?
I upload my patch to gerrit, I get review/approval. I submit my patch to the CQ. It gets rebased. If it has conflicts it tells me so and I need to upload a new patch. Otherwise it gets tested and integrated.
Compare to:
I upload my patch to gerrit, I get review/approval. I submit my patch to the CQ. It gets merged. If it has conflicts it tells me so and I need to upload a new patch. Otherwise it gets tested and integrated. 

Finally, addressing the "I don't want to see merges" crowd, `git log` has a helpful --no-merges option that collapses the history down.  gitk last I knew just passed any arguments it received directly to git log, so if you want to view a linear approximation, that's how you do it.  Note that the sort of merging I'm suggesting here isn't going to result in the sort of gitk uglyness that you get from projects like linux and git.

--no-merge is harmful and counter-productive because it logically changes the orders of commits (i.e. it would show a,b,c,x,d whereas x logically went into the tree after d).

Brian Harring

unread,
Jan 19, 2012, 4:57:52 PM1/19/12
to Chromium OS dev, Mandeep Singh Baines, Antoine Labour
Just updating the thread for an offline conversation regarding this- gerrit actually supports content-merge for cherry-picking.  So the whole debate of merge vs cherry-pick doesn't apply.

Cherry-picking can be left as the mode used- just now if content-merging is enabled, it'll succeed in the cherry-pick if git would for the equivalent operation.

For build, we'll be asking for this to be turned on for chromite initially.  After a trial period, will be looking at enabling this globally- at such time I'll be sending out another set of emails.

If anyone else wants in on this, the ticket is http://code.google.com/p/chromium-os/issues/detail?id=25258 .

Thanks-
~brian

Jonathan Kliegman

unread,
Jan 19, 2012, 5:42:58 PM1/19/12
to Brian Harring, Chromium OS dev, Mandeep Singh Baines, Antoine Labour
Sounds great - thanks for following up on this.

Mandeep Singh Baines

unread,
Jan 20, 2012, 12:49:49 PM1/20/12
to Brian Harring, Chromium OS dev, Antoine Labour
Brian Harring (ferr...@google.com) wrote:
> Just updating the thread for an offline conversation regarding this- gerrit
> actually supports content-merge for cherry-picking. So the whole debate of
> merge vs cherry-pick doesn't apply.
>

Nice!

> Cherry-picking can be left as the mode used- just now if content-merging is
> enabled, it'll succeed in the cherry-pick if git would for the equivalent
> operation.
>
> For build, we'll be asking for this to be turned on for chromite initially.
> After a trial period, will be looking at enabling this globally- at such
> time I'll be sending out another set of emails.
>

So if I understand correctly:

We'll continue to use cherry-pick (so no merge commits) but we'll change
the merge strategy such that some cherry-picks that would have failed
before won't now with the new strategy.

Is that correct?

Regards,
Mandeep

Brian Harring

unread,
Jan 20, 2012, 1:09:09 PM1/20/12
to Mandeep Singh Baines, Chromium OS dev
On Fri, Jan 20, 2012 at 9:49 AM, Mandeep Singh Baines <m...@chromium.org> wrote:
Brian Harring (ferr...@google.com) wrote:
> Just updating the thread for an offline conversation regarding this- gerrit
> actually supports content-merge for cherry-picking.  So the whole debate of
> merge vs cherry-pick doesn't apply.
>

Nice!

> Cherry-picking can be left as the mode used- just now if content-merging is
> enabled, it'll succeed in the cherry-pick if git would for the equivalent
> operation.
>
> For build, we'll be asking for this to be turned on for chromite initially.
>  After a trial period, will be looking at enabling this globally- at such
> time I'll be sending out another set of emails.
>

So if I understand correctly:

We'll continue to use cherry-pick (so no merge commits) but we'll change
the merge strategy such that some cherry-picks that would have failed
before won't now with the new strategy.

Is that correct?

Ish.  The merge strategy (whether for a merge commit or a cherry-pick) is the same- all that is changed is we would now allow two non-descendant changes (literally, neither rev is a parent of the other) to modify the same file as long as the delta/patch applies cleanly, following the normal patch fuzziness rules.

Alternative phrasing: gerrit cherry-picking, merging, etc, will now behave like git cherry-picking, merging, etc. ;)
~brian

Zdenek Behan

unread,
Jan 27, 2012, 9:33:28 AM1/27/12
to Brian Harring, Mandeep Singh Baines, Chromium OS dev, Chris Sosa
+sosa

Just another data point:
A trivial conflict (someone commited in the same path, not the same code) currently causes re-upload of the rebased CL, and loss of all voting except for -2s.
This became a huge pain with CQ; before, I routinely approved my own commits given I had received a prior +2 and all I did was a trivial rebase just to satisfy gerrit. But CQ doesn't seem to consider your own +2 as a LGTM at all, and it patiently waits for someone elses.
For me this creates up to almost infinite approval loops, I can't expect the reviewers to have a 5 second reply interval, bug them because of each CL, but I can't approve CL myself in the above case. I merely have to rebase, +1 +1, wait, and hope that a reviewer gets to +2 it again before someone touches the same file. On frequently touched files, this can take really forever, on CLs that are totally ready.

I'm not sure if being able to self-approve is the right thing to have, probably not, but it was a nice workaround for not having content merging enabled: +1 to this thread, and the sooner it's turned on globally, the better. ;)

--

Chris Sosa

unread,
Jan 27, 2012, 11:28:11 AM1/27/12
to Zdenek Behan, Brian Harring, Mandeep Singh Baines, Chromium OS dev
Hey Zdenek,

That sounds like a bug. The Commit Queue shouldn't care who +2's it
(this is different in Chromium world as retveld doesn't have ACL's on
who can LGTM like Gerrit does). If you see this problem, file a bug
and/or send me a link and I'll investigate.

-Sosa

Nasser Grainawi

unread,
Jan 27, 2012, 11:38:28 AM1/27/12
to Chromium OS dev
On Jan 27, 7:33 am, Zdenek Behan <zbe...@chromium.org> wrote:
> +sosa
>
> Just another data point:
> A trivial conflict (someone commited in the same path, not the same code)
> currently causes re-upload of the rebased CL, and loss of all voting except
> for -2s.
> This became a huge pain with CQ; before, I routinely approved my own
> commits given I had received a prior +2 and all I did was a trivial rebase
> just to satisfy gerrit. But CQ doesn't seem to consider your own +2 as a
> LGTM at all, and it patiently waits for someone elses.
> For me this creates up to almost infinite approval loops, I can't expect
> the reviewers to have a 5 second reply interval, bug them because of each
> CL, but I can't approve CL myself in the above case. I merely have to
> rebase, +1 +1, wait, and hope that a reviewer gets to +2 it again before
> someone touches the same file. On frequently touched files, this can take
> really forever, on CLs that are totally ready.

The Gerrit community would *gladly* accept some patches that did a
trivial-rebase check on the fly when you're uploading a patch and
(perhaps optionally?) left existing approvals in place when the
operation is trivial.

Nasser

>
> I'm not sure if being able to self-approve is the right thing to have,
> probably not, but it was a nice workaround for not having content merging
> enabled: +1 to this thread, and the sooner it's turned on globally, the
> better. ;)
>
>
>
>
>
>
>
> On Fri, Jan 20, 2012 at 7:09 PM, Brian Harring <ferri...@google.com> wrote:
> > On Fri, Jan 20, 2012 at 9:49 AM, Mandeep Singh Baines <m...@chromium.org>wrote:
>
> > Chromium OS Developers mailing list: chromium-os-...@chromium.org

Zdenek Behan

unread,
Jan 27, 2012, 5:19:56 PM1/27/12
to Chris Sosa, Brian Harring, Mandeep Singh Baines, Chromium OS dev

On Fri, Jan 27, 2012 at 5:28 PM, Chris Sosa <so...@chromium.org> wrote:
Hey Zdenek,

That sounds like a bug.  The Commit Queue shouldn't care who +2's it
(this is different in Chromium world as retveld doesn't have ACL's on
who can LGTM like Gerrit does).  If you see this problem, file a bug
and/or send me a link and I'll investigate.

Fair enough. I distinctly remember going all +1 +1 +2 in a single comment and then waiting and waiting with no discernible result, I'll try again and be more scientific about recording the data.

Sean Paul

unread,
Jan 27, 2012, 5:22:08 PM1/27/12
to Zdenek Behan, Chris Sosa, Brian Harring, Mandeep Singh Baines, Chromium OS dev
On Fri, Jan 27, 2012 at 5:19 PM, Zdenek Behan <zbe...@chromium.org> wrote:
>
> On Fri, Jan 27, 2012 at 5:28 PM, Chris Sosa <so...@chromium.org> wrote:
>>
>> Hey Zdenek,
>>
>> That sounds like a bug.  The Commit Queue shouldn't care who +2's it
>> (this is different in Chromium world as retveld doesn't have ACL's on
>> who can LGTM like Gerrit does).  If you see this problem, file a bug
>> and/or send me a link and I'll investigate.
>
>
> Fair enough. I distinctly remember going all +1 +1 +2 in a single comment
> and then waiting and waiting with no discernible result, I'll try again and
> be more scientific about recording the data.
>

I've done this a couple of times today and had no problem (see Gerret
CLs 14897, 14898, 14899).

Sean

Mike Frysinger

unread,
Jan 27, 2012, 5:25:34 PM1/27/12
to Brian Harring, Olof Johansson, Chromium OS dev
On Tue, Jan 10, 2012 at 13:50, Brian Harring wrote:
> If gerrit can't FF the rev, and can't cherry-pick it, if content-merging is
> available and the merge is successful it'll generate a merge commit pulling
> in the original rev, and commit that set.  This is what they're worrying
> about- the fact gerrit in the worst case will generate a merge commit there.
>  This is just how git does it- original rev is preserved, and a glue/merge
> commit is generated.  Linearizing that shouldn't be an issue however since
> implicitly you're changing the rev anyways (parentage changes).

i find the merge commits to generally be noise, but i'm used to it
now. i'd prefer they didn't exist, but i won't complain (on lists) if
they do.
-mike

Reply all
Reply to author
Forward
This conversation is locked
You cannot reply and perform actions on locked conversations.
0 new messages