Hi Stephen,
On Tue, 09 Oct 2012 17:04:06 -0600, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/09/2012 04:19 PM, Albert ARIBAUD wrote:
Apart from this, I'm not sure why forbidding fast-forward is a good thing, but if there are benefits, why not.
It provides documentation in the git history of when merges were made, and what the source of the merge was (at least using the remote name that the merger has configured, which is better than nothing).
This is what it provides, but this does not tell me why it is a good thing. My own use of git history is to find out which (local or remote) branch --contains a given commit, and this is insensitive to allowing ff merges or not.
Related, not rebasing when merging a branch into upstream makes validating Signed-off-by a lot easier; when a patch is directly applied, it should be Signed-off-by the person who applied it. When a person does a rebase rather than a merge, the git committer for the commits is re-written as if the person doing the rebase applied the patch. Instead when merging (and disallowing fast-forward) a merge commit is always created so it's obvious where S-o-b should be applied (direct patch application) and where not (to commits that are merged).
I think we've got several things running here: merges with or without ffs, hiding patches inside a merge (IIUC) and committer identity.
Re hiding patches in a merge if that's really what you mean, I agree that no merge should contains a hidden patch application or removal. If this (sensible IMO) rule is followed, then necessarily, any patch exists as a commit and its Signed-off-by (aong with any other "by" tag) is there for all to see and easy to find.
Re committer identity, I don't see the relationship with "by" tags, and especially with Singed-off-by, since the sign-off is not and must not be related to the committer of the patch, but to its author(s).
Note that neither of these two issues is related to non-fast-forwarding of merges, as the only thing a non-ff merge does is create a commit that, by definition, will not add any content and not affect any existing commit, less so its "by" tags.
Re merging from upstream back into downstream branches, I tend to think that must be allowed considering custodian trees are supposed to be useable, and as such may need to merge back from mainline.
Why is that required for downstream trees to be usable? What is the definition of "usable" you're using?
See http://www.denx.de/wiki/view/U-Boot/CustodianGitTrees#Philosophy_of_custodian_trees:
"My idea of a custodian repository is that it is more than just a working tool for collecting patches and preparing these for merge into mainline. My idea is instead that these are pretty much independent incarnations of U-Boot source trees which users (note: users, not only developers) with specific needs or interests can refer to."
Say 2012.10 is released. We assume that is usable.
Now, someone creates some ARM patches for the next release. As ARM maintainer you do e.g.:
git checkout -b for-201304 v2012.10 git am ...
Now, there's a branch with a bunch of ARM patches applied. Presumably none of those patches are supposed to break anything, and hence this branch is also still usable?
Not sur I follow you reasonning. My personal approach is, once a patch for ARM (but not for an ARM platform for which there is a more specific custodian) is reviewed positively, I do "git checkout master", git am (or rather, pwclient git-am) and run ad hoc and general tests. If this succeeds, I push master (and fetch it back for local personal consistency).
Perhaps the issue is that say a new SoC or feature is added, and some of the patches go through the ARM tree, and some drivers through the USB, I2C, video, ... trees. In that case, in order to use all of those features at once, somebody might have to:
Usually in mixed-repo patch series, a main custodian takes ownership of the series and gets approval from other custodians.
git checkout -b tmp v2012.10 git merge u-boot-arm/next git merge u-boot-i2c/next ...
This requirement is I think one of the main reasons that linux-next exists; to provide a place where all features can be tested at once after having been integrated together. linux-next also allows early detection of merge conflicts that will happen when u-boot-*/next are sent to the maintainer of u-boot/master to be merged.
Now, perhaps you're thinking that in this scenario that u-boot-arm/next can simply merge in u-boot-usb/next, u-boot-i2c/next, u-boot-video/next, etc. in order to create a fully working system. But, wouldn't it be better if all those merges happened only in u-boot.git in a co-ordinated fashion once? After all, perhaps the I2C maintainer also wants his/her branch to be usable on that new platform, and does the reverse merges. Then you end up with spaghetti and unparsable merge history.
You may be making the point that next should be handled just as master as far as the process I laid out can apply to next as well as master only one release further -- and I might understand this. "Master" for coming release, "next" for next release when merge window is closed, and unruly topical branches for anythingthat does not fit in there.
But that's not making the point (IMO) that we should have a flurry of branch names.
And I am pretty sure we don't need to create branches "for such version" "based on such version" all the time; keeping each custodian master current enough should suffice IMO.
Well, we already have this, it's just that the branch names are re-used in a rolling fashion rather than having static names for each release.
Precisely: that's why I think multiplying branches for no added value does not make sense. :)
While v2012.10 is the next release, u-boot-arm/master is for-v2012.10 and u-boot-arm/next is for-v2013.xx. Then, when v2012.10 is release, doesn't u-boot-arm/master become for-v2013.xx and u-boot-arm/next become for-v2013.yy.
Amicalement,