[U-Boot] U-Boot git usage model

[Albert ARIBAUD]

Hi Stephen,

On Tue, 09 Oct 2012 17:04:06 -0600, Stephen Warren
<swarren at 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,
-- 
Albert.