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
> --
> 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
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
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
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
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.~brianMy 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 upstreamOverall, 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
- 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
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.~brianMy 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.
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
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.~brianMy 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 revertSingle node merge; reversion is the same potential level of pain.
- it becomes a lot harder to bisectBisection 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 branchPretty 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 treePlease clarify.
- for upstream repos it becomes harder to drop patches when merging/rebasing upstreamAgain, 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.
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 (ferr...@google.com) wrote:Nice!
> 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.
>
So if I understand correctly:
> 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.
>
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?
--
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
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.
I've done this a couple of times today and had no problem (see Gerret
CLs 14897, 14898, 14899).
Sean
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