Hello Arnd,
I'd like to follow up to the discussion which took place during "Android Code Review" session at LDS (https://wiki.linaro.org/Platform/Android/Specs/AndroidCodeReview).
You expressed concern that Gerrit is not too flexible for working with multiple topic branches, in particular, that it enforces work against single (master) branch (as far as I understood).
Well, looking at AOSP's Gerrit instance, https://review.source.android.com/ one can see that there's a "branch" field present, and shown in the list of patches, and there're patches which reference non-master branches (here's list for "froyo" branch for example: https://review.source.android.com/#q,status:open+branch:froyo,n,z ). So, it at least allows to label a patch with a branch it is submitted against.
So, can you please elaborate on the concerns you expressed? A generalized user story and specific example would be very helpful. (And please keep in mind that I still know little about Gerrit, so if the above doesn't relate directly to your concerns, I'm sorry, that's why I'd like to collect more detailed info on Gerrit's features/misfeatures people know).
Thanks, Paul
On 16 May 2011 17:52, Paul Sokolovsky paul.sokolovsky@linaro.org wrote:
Hello Arnd,
I'd like to follow up to the discussion which took place during "Android Code Review" session at LDS (https://wiki.linaro.org/Platform/Android/Specs/AndroidCodeReview).
You expressed concern that Gerrit is not too flexible for working with multiple topic branches, in particular, that it enforces work against single (master) branch (as far as I understood).
Well, looking at AOSP's Gerrit instance, https://review.source.android.com/ one can see that there's a "branch" field present, and shown in the list of patches, and there're patches which reference non-master branches (here's list for "froyo" branch for example: https://review.source.android.com/#q,status:open+branch:froyo,n,z ). So, it at least allows to label a patch with a branch it is submitted against.
So, can you please elaborate on the concerns you expressed? A generalized user story and specific example would be very helpful. (And please keep in mind that I still know little about Gerrit, so if the above doesn't relate directly to your concerns, I'm sorry, that's why I'd like to collect more detailed info on Gerrit's features/misfeatures people know).
A patch belongs to a branch even in Gerrit. One user story is moving patches from branch B to branch A. I believe the only way to handle this in Gerrit is to resubmit as new commits. Another one is how to perform rebase in Gerrit, reorder/remove/squash commits on a branch. How is this described in Gerrit?
Regards, Per
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
2011/5/16 Paul Sokolovsky paul.sokolovsky@linaro.org:
You expressed concern that Gerrit is not too flexible for working with multiple topic branches, in particular, that it enforces work against single (master) branch (as far as I understood).
It was mainly me blathering about Gerrit in that session so don't go after Arnd...
I take a step back: there is nothing technical wrong with Gerrit really, the problem is how it is used.
And the problem is that Gerrit is often used (also by Google) as a one-stop shop for integration.
This means: you don't have what the kernel developers refer to as topic branches. Instead, in the Google case, look for example here:
http://android.git.kernel.org/?p=kernel/experimental.git%3Ba=summary (...) 4 months ago android-goldfish-2.6.35 7 months ago android-msm-2.6.35-wip 8 months ago linux-tegra-2.6.36-wip 10 months ago android-2.6.34-test2
These are indeed branches, but *what* is the topic actually?
Well, it turs out the topic is: "branch to rebase an big whole shebang mega-patchset on top of a new kernel upstream realease".
That's not what kernel developers mean by "topic branch". which is for example: http://git.kernel.org/?p=linux/kernel/git/tip/linux-2.6-tip.git%3Ba=summary
With examples like this it is not strange that Gerrit is used as it is in companies doing Android development.
I think it basically boils down to the fact that a single Gerrit branch is seen as "the place to integrate", whereas in kernel terms, you should integrate a single topic (such as "i2c updates", "boardfiles", "regulators" etc) and the system integrator should use plain git (no web interface!) to integrate the result with an octopus merge to produce a complete product. The result of the octopus merge should be frozen, i.e. it is not allowed to be used as a starting point for further development.
On most companies where I know the details, nobody even has a clue what an octopus merge is. Eevrything is integrated in Gerrit directly, on a single branch.
One reason is that I think you have to configure Gerrit for each new branch you create or delete, and that means a real bad fit with the kernel model of quickly creating/deleting and rebasing branches.
So: Gerrit is indeed a good review system and has the ability to use (topic) branches. But the examples from companies like Google are not very good models of what to do with them.
Now there may be a reason why everything is still integrated on a single branch, and that is cross-subsystem dependency hell. If all drivers you develop are cross-dependent on other drivers, you end up in this place, such as you want to develop a driver for I2C peripheral X but you don't have a driver for the I2C bus yet (and a hundre times more complex examples). What you need to do in that case is to create new topic branch based on top of the i2c-driver topic branch and so on, and then you have to use just git, using Gerrit at the same time will make it necessary to add/remove/rebase branches at neckbreaking speed. If you just use a singe branch the integration seems simpler to the organization.
Yours, Linus Walleij
On 18 May 2011 13:07, Linus Walleij linus.walleij@linaro.org wrote:
2011/5/16 Paul Sokolovsky paul.sokolovsky@linaro.org:
You expressed concern that Gerrit is not too flexible for working with multiple topic branches, in particular, that it enforces work against single (master) branch (as far as I understood).
It was mainly me blathering about Gerrit in that session so don't go after Arnd...
I take a step back: there is nothing technical wrong with Gerrit really, the problem is how it is used.
And the problem is that Gerrit is often used (also by Google) as a one-stop shop for integration.
This means: you don't have what the kernel developers refer to as topic branches. Instead, in the Google case, look for example here:
http://android.git.kernel.org/?p=kernel/experimental.git%3Ba=summary (...) 4 months ago android-goldfish-2.6.35 7 months ago android-msm-2.6.35-wip 8 months ago linux-tegra-2.6.36-wip 10 months ago android-2.6.34-test2
These are indeed branches, but *what* is the topic actually?
Well, it turs out the topic is: "branch to rebase an big whole shebang mega-patchset on top of a new kernel upstream realease".
That's not what kernel developers mean by "topic branch". which is for example: http://git.kernel.org/?p=linux/kernel/git/tip/linux-2.6-tip.git%3Ba=summary
With examples like this it is not strange that Gerrit is used as it is in companies doing Android development.
I think it basically boils down to the fact that a single Gerrit branch is seen as "the place to integrate", whereas in kernel terms, you should integrate a single topic (such as "i2c updates", "boardfiles", "regulators" etc) and the system integrator should use plain git (no web interface!) to integrate the result with an octopus merge to produce a complete product. The result of the octopus merge should be frozen, i.e. it is not allowed to be used as a starting point for further development.
If we have gerrit, build servers and test farm set up and we have managed to get the whole continuous integration process going is there the anything that prevents that we push to a topic branch for review and that the first step before the build starts it to do a merge of all topic branches for the kernel.
On most companies where I know the details, nobody even has a clue what an octopus merge is. Eevrything is integrated in Gerrit directly, on a single branch.
One reason is that I think you have to configure Gerrit for each new branch you create or delete, and that means a real bad fit with the kernel model of quickly creating/deleting and rebasing branches.
Branches are "configured" in the same way as in git. :) You do a git push to the branch you want to create. The difference is that when you do your git push gerrit will check if you have the permission to create branches.
/Patrik
So: Gerrit is indeed a good review system and has the ability to use (topic) branches. But the examples from companies like Google are not very good models of what to do with them.
Now there may be a reason why everything is still integrated on a single branch, and that is cross-subsystem dependency hell. If all drivers you develop are cross-dependent on other drivers, you end up in this place, such as you want to develop a driver for I2C peripheral X but you don't have a driver for the I2C bus yet (and a hundre times more complex examples). What you need to do in that case is to create new topic branch based on top of the i2c-driver topic branch and so on, and then you have to use just git, using Gerrit at the same time will make it necessary to add/remove/rebase branches at neckbreaking speed. If you just use a singe branch the integration seems simpler to the organization.
Yours, Linus Walleij
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
2011/5/18 Patrik Ryd patrik.ryd@linaro.org:
On 18 May 2011 13:07, Linus Walleij linus.walleij@linaro.org wrote:
I think it basically boils down to the fact that a single Gerrit branch is seen as "the place to integrate", whereas in kernel terms, you should integrate a single topic (such as "i2c updates", "boardfiles", "regulators" etc) and the system integrator should use plain git (no web interface!) to integrate the result with an octopus merge to produce a complete product. The result of the octopus merge should be frozen, i.e. it is not allowed to be used as a starting point for further development.
If we have gerrit, build servers and test farm set up and we have managed to get the whole continuous integration process going is there the anything that prevents that we push to a topic branch for review and that the first step before the build starts it to do a merge of all topic branches for the kernel.
Well occasionally there are merge conflicts. Which need to be resolved manually :-P
But there is nothing stopping you from testing each topic branch in isolation I guess. The problem appears when you want to continously test "the whole product", i.e. a mergedown of all topic branches.
So in this particular case the agile programming axiom to always have a running build every night clashes with the idea of working in isolated topic branches - the former really benefit from all integration being done on a single branch.
One reason is that I think you have to configure Gerrit for each new branch you create or delete, and that means a real bad fit with the kernel model of quickly creating/deleting and rebasing branches.
Branches are "configured" in the same way as in git. :) You do a git push to the branch you want to create. The difference is that when you do your git push gerrit will check if you have the permission to create branches.
Yes I was completely wrong on this, Gerrit is really flexible with branches, Ian Kumlien also taught me this...
Thanks, Linus Walleij
Hello Linus,
On Wed, 18 May 2011 13:07:50 +0200 Linus Walleij linus.walleij@linaro.org wrote:
2011/5/16 Paul Sokolovsky paul.sokolovsky@linaro.org:
You expressed concern that Gerrit is not too flexible for working with multiple topic branches, in particular, that it enforces work against single (master) branch (as far as I understood).
It was mainly me blathering about Gerrit in that session so don't go after Arnd...
Well, maybe I missed something, but somehow I have a feeling that Gerrit was discussed at two (or more) sessions. I look at Gerrit from Linaro Android perspective, and there's of course one good reason to use it in this scenario: it's what the upstream uses, so good it or bad, if we want to work with upstrean, we'll need to eat Google's own dogfood. But yet at LDS I got an idea that there was talk/thinking about using Gerrit for more than Android, and now I looked thru Kernel Track session (pity I couldn't follow it closer) and saw that's the case.
In either case, concerns expressed about Gerrit could have been either:
1. Gerrit workflow is hardcoded to be too rigid and inflexible.
or
2. Typical Gerrit workflow is not well suited, flexible, or advanced for multi-(topic)-branch and other workflow patterns.
I take a step back: there is nothing technical wrong with Gerrit really, the problem is how it is used.
Ok, so it seems that it's point 2 above after all.
And the problem is that Gerrit is often used (also by Google) as a one-stop shop for integration.
This means: you don't have what the kernel developers refer to as topic branches. Instead, in the Google case, look for example here:
http://android.git.kernel.org/?p=kernel/experimental.git%3Ba=summary (...) 4 months ago android-goldfish-2.6.35 7 months ago android-msm-2.6.35-wip 8 months ago linux-tegra-2.6.36-wip 10 months ago android-2.6.34-test2
These are indeed branches, but *what* is the topic actually?
Well, yeah, they're release, not topic centered ;-).
Well, it turs out the topic is: "branch to rebase an big whole shebang mega-patchset on top of a new kernel upstream realease".
That's not what kernel developers mean by "topic branch". which is for example: http://git.kernel.org/?p=linux/kernel/git/tip/linux-2.6-tip.git%3Ba=summary
With examples like this it is not strange that Gerrit is used as it is in companies doing Android development.
True, but there're also (good?) reasons for that. For Android, kernel is, while big and important, just one of a hundred of components. And not even primary development focus, which otherwise would be platform middleware and user apps - again, hundreds of components, so if guys used topic branches (for each of them), they would be drowned in them. So, instead there're release-based branches, and Gerrit which appears to help to deal with all of multitude of changes to all those components.
I think it basically boils down to the fact that a single Gerrit branch is seen as "the place to integrate", whereas in kernel terms, you should integrate a single topic (such as "i2c updates", "boardfiles", "regulators" etc) and the system integrator should use plain git (no web interface!) to integrate the result with an octopus merge to produce a complete product. The result of the octopus merge should be frozen, i.e. it is not allowed to be used as a starting point for further development.
On most companies where I know the details, nobody even has a clue what an octopus merge is. Eevrything is integrated in Gerrit directly, on a single branch.
One reason is that I think you have to configure Gerrit for each new branch you create or delete, and that means a real bad fit with the kernel model of quickly creating/deleting and rebasing branches.
Well, so bottom line of this, as far as I see it, is: Gerrit is de facto tool for Android, and for Linaro Android we'd like to be sure that it supports more flexible/advanced approach than which is seen typically in Android shops. It's also nice idea to select single official Linaro git review tool. But I agree that Gerrit doesn't immediately fit with canonical kernel workflow, while its choice is almost fixed for Android by upstream usage.
So, we either would need to experiment with using Gerrit in news ways, for example, like you suggest, set up separate a Gerrit instance for each "project" (each topic branch can be see as such), or... just keep our options open and don't fix on Gerrit as pan-Linaro panacea.
So: Gerrit is indeed a good review system and has the ability to use (topic) branches. But the examples from companies like Google are not very good models of what to do with them.
Now there may be a reason why everything is still integrated on a single branch, and that is cross-subsystem dependency hell. If all drivers you develop are cross-dependent on other drivers, you end up in this place, such as you want to develop a driver for I2C peripheral X but you don't have a driver for the I2C bus yet (and a hundre times more complex examples). What you need to do in that case is to create new topic branch based on top of the i2c-driver topic branch and so on, and then you have to use just git, using Gerrit at the same time will make it necessary to add/remove/rebase branches at neckbreaking speed. If you just use a singe branch the integration seems simpler to the organization.
Yours, Linus Walleij
On Fri, May 20, 2011 at 9:08 AM, Paul Sokolovsky paul.sokolovsky@linaro.org wrote:
Hello Linus,
[snip]
Well, so bottom line of this, as far as I see it, is: Gerrit is de facto tool for Android, and for Linaro Android we'd like to be sure that it supports more flexible/advanced approach than which is seen typically in Android shops. It's also nice idea to select single official Linaro git review tool. But I agree that Gerrit doesn't immediately fit with canonical kernel workflow, while its choice is almost fixed for Android by upstream usage.
Note that QEMU also uses GIT but has a different workflow that probably doesn't fit Gerrit. Linaro GCC uses the Launchpad merge request system which fits our needs well and matches upstream's style.
I've yet to find two projects that have the same workflow.
-- Michael
On 19 May 2011 23:01, Michael Hope michael.hope@linaro.org wrote:
On Fri, May 20, 2011 at 9:08 AM, Paul Sokolovsky wrote:
Well, so bottom line of this, as far as I see it, is: Gerrit is de facto tool for Android, and for Linaro Android we'd like to be sure that it supports more flexible/advanced approach than which is seen typically in Android shops. It's also nice idea to select single official Linaro git review tool. But I agree that Gerrit doesn't immediately fit with canonical kernel workflow, while its choice is almost fixed for Android by upstream usage.
Note that QEMU also uses GIT but has a different workflow that probably doesn't fit Gerrit.
Yep. QEMU review is straightforward on-mailing-list stuff. What QEMU could use is better patch tracking (patchwork has several fatal flaws for our purposes, unfortunately), but there doesn't really seem to be any demand for a better review tool.
I've yet to find two projects that have the same workflow.
Agreed -- "do what upstream does" is much more important than trying to do the same thing across all of Linaro IMHO.
-- PMM
2011/5/19 Paul Sokolovsky paul.sokolovsky@linaro.org:
I look at Gerrit from Linaro Android perspective, and there's of course one good reason to use it in this scenario: it's what the upstream uses, so good it or bad, if we want to work with upstrean, we'll need to eat Google's own dogfood.
True for any FOSS that comes out of Google, for kernel, BlueZ etc Google cannot really be considered upstream. And we were discussing the kernel here I believe.
In either case, concerns expressed about Gerrit could have been either:
- Gerrit workflow is hardcoded to be too rigid and inflexible.
or
- Typical Gerrit workflow is not well suited, flexible, or advanced
for multi-(topic)-branch and other workflow patterns.
It's (2)
I take a step back: there is nothing technical wrong with Gerrit really, the problem is how it is used.
Ok, so it seems that it's point 2 above after all.
Yeah.
These are indeed branches, but *what* is the topic actually?
Well, yeah, they're release, not topic centered ;-).
Yes and that is not how kernel development works. And what the Linaro organization, especially the landing teams, want to achieve is merging the code to Torvalds, which means clean cut topic that get pulled into mainline.
With examples like this it is not strange that Gerrit is used as it is in companies doing Android development.
True, but there're also (good?) reasons for that. For Android, kernel is, while big and important, just one of a hundred of components.
We are talking past each other here.
The problem in the sessions I was to was about working with the kernel community, not managing Android integration.
if guys used topic branches (for each of them), they would be drowned in them. So, instead there're release-based branches, and Gerrit which appears to help to deal with all of multitude of changes to all those components.
One does not exclude the other I believe.
I also believe it is possible to make an exception for the kernel and other upstream projects, it's just a GIT after all, and it has a very clean cut API amd ABI.
But I agree that Gerrit doesn't immediately fit with canonical kernel workflow, while its choice is almost fixed for Android by upstream usage.
Define the "canonical kernel workflow", what I'm talking about is the kernel community workflow.
So, we either would need to experiment with using Gerrit in news ways, for example, like you suggest, set up separate a Gerrit instance for each "project" (each topic branch can be see as such), or... just keep our options open and don't fix on Gerrit as pan-Linaro panacea.
Gerrit handled topic branches, it's mainly about integration and how people are told to work. But mainly yes.
Yours, Linus Walleij
Hello Linus,
On Sat, 21 May 2011 09:23:28 +0200 Linus Walleij linus.walleij@linaro.org wrote:
2011/5/19 Paul Sokolovsky paul.sokolovsky@linaro.org:
I look at Gerrit from Linaro Android perspective, and there's of course one good reason to use it in this scenario: it's what the upstream uses, so good it or bad, if we want to work with upstrean, we'll need to eat Google's own dogfood.
True for any FOSS that comes out of Google, for kernel, BlueZ etc Google cannot really be considered upstream. And we were discussing the kernel here I believe.
No, sorry for the confusion and not making this clear - I wrote the original mail as a follow-up to "Android Code Review" session, https://blueprints.launchpad.net/linaro-android/+spec/linaro-android-o-code-... http://summit.linaro.org/uds-o/meeting/linaro-android-o-code-review/ , where Arnd expression concerns which were discussed. I look at Gerrit from viewpoint of Android/Infrastructure team, and collect and appreciate feedback and usecases from wider audience.
So, I guess I'd skip further discussion of usage Gerrit for other projects, because I have little to add to what was told, and didn't participate in the corresponding sessions.
On Saturday 21 May 2011, Paul Sokolovsky wrote:
On Sat, 21 May 2011 09:23:28 +0200 Linus Walleij linus.walleij@linaro.org wrote:
2011/5/19 Paul Sokolovsky paul.sokolovsky@linaro.org:
I look at Gerrit from Linaro Android perspective, and there's of course one good reason to use it in this scenario: it's what the upstream uses, so good it or bad, if we want to work with upstrean, we'll need to eat Google's own dogfood.
True for any FOSS that comes out of Google, for kernel, BlueZ etc Google cannot really be considered upstream. And we were discussing the kernel here I believe.
No, sorry for the confusion and not making this clear - I wrote the original mail as a follow-up to "Android Code Review" session, https://blueprints.launchpad.net/linaro-android/+spec/linaro-android-o-code-... http://summit.linaro.org/uds-o/meeting/linaro-android-o-code-review/ , where Arnd expression concerns which were discussed. I look at Gerrit from viewpoint of Android/Infrastructure team, and collect and appreciate feedback and usecases from wider audience.
Sorry for the late follow-up, too many things kept coming up at once in the last few weeks. Here is what I had in mind, tell me if I should add this to the blueprint or how else you want to proceed.
== User Stories ==
Karl is a developer for a kernel driver and works within a project using Gerrit. Every work item he does results in a patch that he sends for review to his subsystem maintainer. He marks his work items as done once the patch has been accepted into the subsystem branch.
Eva is Karl's subsystem maintainer and all of his patches that she accepts are applied to the branch that she maintains for that kind of device driver. She also maintains branches for a small set of other subsystems. Since her main target is to integrate the work upstream, the separation between the branches is based on how her kernel upstream maintainers work. When Eva considers one branch ready for upstream submission, she rebases the branches as necessary to avoid conflicts and she sometimes folds patches in order to be acceptable.
Harald is a release maintainer in the same project. He maintains a master branch with all the work done by subsystem maintainers such as Eva, and he pulls in changes from all their branches into his master branch. Harald sometimes gets upset when Eva has to rebase a branch, but he accepts that as part of his daily work. After a product release, Harald archives his branch and starts a new branch based on the latest upstream kernel and the subsystem branches that have not yet been merged into that version.
== Missing Pieces ==
* Gerrit apparently does not have a good way to automatically integrate work from multiple branches. Ideally, manual interaction should only be necessary when branches don't merge cleanly, or when they have been rebased.
* Rebases don't fit the Gerrit model of always just adding patches on top. However, in order to work with upstream, it is often necessary to change main design choices throughout a patch series, not just by adding changes at the end of the series.
Arnd
Hello Arnd,
[]
No, sorry for the confusion and not making this clear - I wrote the original mail as a follow-up to "Android Code Review" session, https://blueprints.launchpad.net/linaro-android/+spec/linaro-android-o-code-... http://summit.linaro.org/uds-o/meeting/linaro-android-o-code-review/ , where Arnd expression concerns which were discussed. I look at Gerrit from viewpoint of Android/Infrastructure team, and collect and appreciate feedback and usecases from wider audience.
Sorry for the late follow-up, too many things kept coming up at once in the last few weeks. Here is what I had in mind, tell me if I should add this to the blueprint or how else you want to proceed.
Thanks, I've added link to this in linaro-dev list archive to Android's blueprint, https://blueprints.launchpad.net/linaro-android/+spec/linaro-android-o-code-...
If there's a blueprint for kernel code review, I guess it makes sense to add it there too. (I also consider starting a wiki page with code review tools comparison/issues, I'll add it there to when I get to it)
== User Stories ==
[]
-
Arnd Bergmann -
Linus Walleij -
Michael Hope -
Patrik Ryd -
Paul Sokolovsky -
Per Forlin -
Peter Maydell