On 10/09/2012 08:23 AM, Tom Rini wrote:

On Sun, Oct 07, 2012 at 08:49:00PM +0200, Marek Vasut wrote:

...

NOTE: I get a few more size issues with ELDK 4.2 on IXP (that big-endian ARM) after this patchset is applied. I wonder if we shouldn't just throw these away, since they're dead code mostly.

The following changes since commit c7ee66a8222660b565e9240775efa4c82cb348c2:

Merge branch 'next' of git://www.denx.de/git/u-boot-ppc4xx into next (2012-10-02 10:16:40 -0700)

are available in the git repository at:

git://git.denx.de/u-boot-usb.git next

for you to fetch changes up to f0ede0e8305bc3c959862446bce40cb028b36293:

usb.h: Add udc_disconnect prototype to usb.h (2012-10-07 02:08:48 +0200)

I had to rebase this locally to merge (such is next), and now it's applied to u-boot/next, thanks!

Hmm. Can't "git merge" solve merge conflicts just as well as "git rebase"?

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

It would be awesome if U-Boot could adopt something more similar to the Linux kernel's git usage model, namely:

* All downstream branches are based off some known stable point in the master branch (e.g. 2012.10-rc1). Before these branches are merged into any other branch, they can be rebased if absolutely needed, but preferably not.

* Once a downstream branch is merged upwards, the downstream branch doesn't merge upstream back down into the downstream branch, but either:

a) Keeps adding to the existing branch so that incremental pull requests can be sent.

Or often when u-boot/master has made a complete new release does:

b) Creates a new branch based on the latest rc or release from u-boot/master.

(in practice, downstream branches typically end up with something like for-3.5 based on v3.4-rcN, for-3.6 based on v3.5-rcN, for-3.7 based on v3.6-rcN, some running in parallel containing either important bugfixes for the release or new development as determined by the current state of the various releases in the mainline tree).

* When a branch is merged from a repo to a parent repo, it's always a git merge --no-ff; never a rebase or fast-forward.

* In order to resolve merge conflicts/dependencies between different downstream branches, one of the following happens:

1)

a) The first downstream branch gets merged into u-boot/master. b) The second downstream branch creates a new branch starting at an an rc or release in u-boot-master that contains it the required patches. c) The dependent patches are applied to the second downstream branch. d) The second downstream branch gets merged into u-boot/master.

2)

All the patches that would usually be merged through downstream branch 2 actually get ack'd by the maintainer of downstream branch 2 and applied to downstream branch 1 after the patches they depend on. This is simplest, but may cause complications if both branches need to take patches that build on the merged patches they're merged into an rc or release in u-boot/master.

3)

A topic branch is created by one of the downstream maintainers, branched from a u-boot/master rc or release, and containing just the patches that other patches depend on, and this topic branch gets merged into both the two downstream branches for further work.

Yes, this does all take a little bit more thought, planning, and co-ordination, but I think having a simpler and more stable git history is worth it.

On 10/09/2012 03:32 PM, Tom Rini wrote:

On Tue, Oct 09, 2012 at 03:03:28PM -0600, Stephen Warren wrote:

...

On 10/09/2012 08:23 AM, Tom Rini wrote:

...

On Sun, Oct 07, 2012 at 08:49:00PM +0200, Marek Vasut wrote:

...

NOTE: I get a few more size issues with ELDK 4.2 on IXP (that big-endian ARM) after this patchset is applied. I wonder if we shouldn't just throw these away, since they're dead code mostly.

The following changes since commit c7ee66a8222660b565e9240775efa4c82cb348c2:

Merge branch 'next' of git://www.denx.de/git/u-boot-ppc4xx into next (2012-10-02 10:16:40 -0700)

are available in the git repository at:

git://git.denx.de/u-boot-usb.git next

for you to fetch changes up to f0ede0e8305bc3c959862446bce40cb028b36293:

usb.h: Add udc_disconnect prototype to usb.h (2012-10-07 02:08:48 +0200)

I had to rebase this locally to merge (such is next), and now it's applied to u-boot/next, thanks!

Hmm. Can't "git merge" solve merge conflicts just as well as "git rebase"?

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

The short answer is that right now, u-boot/next follows the linux-next model and we rebase as needed.

I don't quite follow that; linux-next is also purely merge-based. Are you referring to the fact that it's re-created every day, and the source branches that go into the merge can be rebased if needed?

Instead, I think u-boot/next is just a place where patches get applied, or branches get merged, before u-boot/master is open to accept new patches for the next release. Unless I'm misunderstanding it purpose of course...

Now, having a linux-next style daily merge of u-boot-*/next would be pretty awesome.

...

It would be awesome if U-Boot could adopt something more similar to the Linux kernel's git usage model, namely:

• All downstream branches are based off some known stable point

in the master branch (e.g. 2012.10-rc1). Before these branches are merged into any other branch, they can be rebased if absolutely needed, but preferably not.

• Once a downstream branch is merged upwards, the downstream

branch doesn't merge upstream back down into the downstream branch, but either:

a) Keeps adding to the existing branch so that incremental pull requests can be sent.

Or often when u-boot/master has made a complete new release does:

b) Creates a new branch based on the latest rc or release from u-boot/master.

(in practice, downstream branches typically end up with something like for-3.5 based on v3.4-rcN, for-3.6 based on v3.5-rcN, for-3.7 based on v3.6-rcN, some running in parallel containing either important bugfixes for the release or new development as determined by the current state of the various releases in the mainline tree).

• When a branch is merged from a repo to a parent repo, it's

always a git merge --no-ff; never a rebase or fast-forward.

• In order to resolve merge conflicts/dependencies between

different downstream branches, one of the following happens:

a) The first downstream branch gets merged into u-boot/master. b) The second downstream branch creates a new branch starting at an an rc or release in u-boot-master that contains it the required patches. c) The dependent patches are applied to the second downstream branch. d) The second downstream branch gets merged into u-boot/master.

All the patches that would usually be merged through downstream branch 2 actually get ack'd by the maintainer of downstream branch 2 and applied to downstream branch 1 after the patches they depend on. This is simplest, but may cause complications if both branches need to take patches that build on the merged patches they're merged into an rc or release in u-boot/master.

A topic branch is created by one of the downstream maintainers, branched from a u-boot/master rc or release, and containing just the patches that other patches depend on, and this topic branch gets merged into both the two downstream branches for further work.

Yes, this does all take a little bit more thought, planning, and co-ordination, but I think having a simpler and more stable git history is worth it.

Interesting. As this is more work on the custodians end, what does everyone else say?

This actually turns out to be less work for custodians if there aren't any dependencies between patch series, since whenever you send a pull request right now, you do:

a) Fetch latest upstream. b) Rebase onto it. c) Send pull request.

but with the Linux model, you simply:

a) Send pull request.

Admittedly the recipient then might need to resolve some merge conflicts. However, hopefully people have been planning for these and have avoided them.

Now, if there are dependencies between branches, then perhaps the formalized options above seem like more work, but I don't think there's a good solution in place today for this anyway, is there? So, it still seems like this simplifies things.

Hi Albert,

On Wed, Oct 10, 2012 at 9:43 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:

Hi Stephen,

On Tue, 09 Oct 2012 16:14:23 -0600, Stephen Warren swarren@wwwdotorg.org wrote:

...

This actually turns out to be less work for custodians if there aren't any dependencies between patch series, since whenever you send a pull request right now, you do:

a) Fetch latest upstream. b) Rebase onto it. c) Send pull request.

Uh, no, you don't rebase. I've learnt that well. :)

Yes, I learn't that the hard way :)

But I suspect the Wiki page is still lagging behind.

Yes because, to some extent at least, maintainers have a certain amount of latitude in how they manage their own repository. The whole rebase against master theory came about, I think, to avoid the maintainer repositories being littered with merge commits (which I assume adds more merge commits to mainline). I personally don't have an issue with merge commits - YMMV

Regards,

Graeme

On 10/09/2012 04:59 PM, Tom Rini wrote:

On Tue, Oct 09, 2012 at 04:14:23PM -0600, Stephen Warren wrote:

...

On 10/09/2012 03:32 PM, Tom Rini wrote:

...

On Tue, Oct 09, 2012 at 03:03:28PM -0600, Stephen Warren wrote:

...

On 10/09/2012 08:23 AM, Tom Rini wrote:

...

On Sun, Oct 07, 2012 at 08:49:00PM +0200, Marek Vasut wrote:

...

NOTE: I get a few more size issues with ELDK 4.2 on IXP (that big-endian ARM) after this patchset is applied. I wonder if we shouldn't just throw these away, since they're dead code mostly.

The following changes since commit c7ee66a8222660b565e9240775efa4c82cb348c2:

Merge branch 'next' of git://www.denx.de/git/u-boot-ppc4xx into next (2012-10-02 10:16:40 -0700)

are available in the git repository at:

git://git.denx.de/u-boot-usb.git next

for you to fetch changes up to f0ede0e8305bc3c959862446bce40cb028b36293:

usb.h: Add udc_disconnect prototype to usb.h (2012-10-07 02:08:48 +0200)

I had to rebase this locally to merge (such is next), and now it's applied to u-boot/next, thanks!

Hmm. Can't "git merge" solve merge conflicts just as well as "git rebase"?

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

The short answer is that right now, u-boot/next follows the linux-next model and we rebase as needed.

I don't quite follow that; linux-next is also purely merge-based. Are you referring to the fact that it's re-created every day, and the source branches that go into the merge can be rebased if needed?

I'm referring to that it's always rebased on top of Linus' tree. That's what caused the issue here, u-boot-usb was based on top of u-boot/next (which is may not have strictly needed to be) so I did the job of rebasing for Marek since it was an easy one.

...

Instead, I think u-boot/next is just a place where patches get applied, or branches get merged, before u-boot/master is open to accept new patches for the next release. Unless I'm misunderstanding it purpose of course...

Now, having a linux-next style daily merge of u-boot-*/next would be pretty awesome.

Well, it's a merge of what people want to get into the next merge window, as often as they're willing to submit if they are a custodian or as often as I can otherwise. In fact, right now some pull requests need to be on top of next rather than a "stable" point because we're making some pretty big cleanups and changes in a few areas right now.

So, is u-boot/next purely incremental, or lets say something is merged in there, then needs to be reworked because of some nasty git bisect issue say, or some patches need to be taken through different trees due to dependencies, can the already-merged patches be removed and replaced with a completely new set, or can only incremental patches be applied?

I assume u-boot/next isn't rebuilt using fresh merges each time, hence isn't the linux-next model exactly?

On 10/09/2012 05:14:23 PM, Stephen Warren wrote:

On 10/09/2012 03:32 PM, Tom Rini wrote:

...

On Tue, Oct 09, 2012 at 03:03:28PM -0600, Stephen Warren wrote:

...

On 10/09/2012 08:23 AM, Tom Rini wrote:

...

On Sun, Oct 07, 2012 at 08:49:00PM +0200, Marek Vasut wrote:

...

NOTE: I get a few more size issues with ELDK 4.2 on IXP (that big-endian ARM) after this patchset is applied. I wonder if we shouldn't just throw these away, since they're dead code mostly.

The following changes since commit c7ee66a8222660b565e9240775efa4c82cb348c2:

Merge branch 'next' of git://www.denx.de/git/u-boot-ppc4xx into next (2012-10-02 10:16:40 -0700)

are available in the git repository at:

git://git.denx.de/u-boot-usb.git next

for you to fetch changes up to f0ede0e8305bc3c959862446bce40cb028b36293:

usb.h: Add udc_disconnect prototype to usb.h (2012-10-07 02:08:48 +0200)

I had to rebase this locally to merge (such is next), and now it's applied to u-boot/next, thanks!

Hmm. Can't "git merge" solve merge conflicts just as well as "git rebase"?

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

The short answer is that right now, u-boot/next follows the linux-next model and we rebase as needed.

I don't quite follow that; linux-next is also purely merge-based. Are you referring to the fact that it's re-created every day, and the source branches that go into the merge can be rebased if needed?

What's the difference between "re-created every day" and "rebased every day"?

Instead, I think u-boot/next is just a place where patches get applied, or branches get merged, before u-boot/master is open to accept new patches for the next release. Unless I'm misunderstanding it purpose of course...

That was my impression as well.

Now, having a linux-next style daily merge of u-boot-*/next would be pretty awesome.

Not really needed if the main next tree can permanently merge those branches.

...

...

It would be awesome if U-Boot could adopt something more similar to the Linux kernel's git usage model, namely:

• All downstream branches are based off some known stable point

in the master branch (e.g. 2012.10-rc1). Before these branches are merged into any other branch, they can be rebased if absolutely needed, but preferably not.

• Once a downstream branch is merged upwards, the downstream

branch doesn't merge upstream back down into the downstream branch, but either:

a) Keeps adding to the existing branch so that incremental pull requests can be sent.

How does merging back down prevent incremental pull requests?

...

...

Or often when u-boot/master has made a complete new release does:

b) Creates a new branch based on the latest rc or release from u-boot/master.

That's a rebase. How is that better than a (likely fast-forward) merge?

...

...

(in practice, downstream branches typically end up with something like for-3.5 based on v3.4-rcN, for-3.6 based on v3.5-rcN, for-3.7 based on v3.6-rcN, some running in parallel containing either important bugfixes for the release or new development as determined by the current state of the various releases in the mainline tree).

I thought you said your way was less work? :-)

...

...

• When a branch is merged from a repo to a parent repo, it's

always a git merge --no-ff; never a rebase or fast-forward.

• In order to resolve merge conflicts/dependencies between

different downstream branches, one of the following happens:

a) The first downstream branch gets merged into u-boot/master. b) The second downstream branch creates a new branch starting at an an rc or release in u-boot-master that contains it the required patches. c) The dependent patches are applied to the second downstream branch. d) The second downstream branch gets merged into u-boot/master.

All the patches that would usually be merged through downstream branch 2 actually get ack'd by the maintainer of downstream branch 2 and applied to downstream branch 1 after the patches they depend on. This is simplest, but may cause complications if both branches need to take patches that build on the merged patches they're merged into an rc or release in u-boot/master.

A topic branch is created by one of the downstream maintainers, branched from a u-boot/master rc or release, and containing just the patches that other patches depend on, and this topic branch gets merged into both the two downstream branches for further work.

Yes, this does all take a little bit more thought, planning, and co-ordination, but I think having a simpler and more stable git history is worth it.

What is the specific improvement in git history as a result of this?

...

Interesting. As this is more work on the custodians end, what does everyone else say?

This actually turns out to be less work for custodians if there aren't any dependencies between patch series, since whenever you send a pull request right now, you do:

a) Fetch latest upstream. b) Rebase onto it. c) Send pull request.

That's what I used to do, but recently Wolfgang said no rebases, so I merge instead.

but with the Linux model, you simply:

a) Send pull request.

Admittedly the recipient then might need to resolve some merge conflicts. However, hopefully people have been planning for these and have avoided them.

How do you plan for them and avoid them, and how is that less work that what we do now?

It's one thing if a merge conflict comes from multiple pull requests being processed at once, but someone submitting a pull request should at least make sure that it doesn't conflict with top-of-tree by itself (some actual testing of the merge would be good too...). And to do that, you've got to either merge or rebase, not just blindly request a pull.

I especially do not want to have to work with some artificially chosen old tree as my base. I also do not want to create a bunch of named branches for each

-Scott

On 10/09/2012 06:25:47 PM, Stephen Warren wrote:

On 10/09/2012 05:00 PM, Scott Wood wrote:

...

On 10/09/2012 05:14:23 PM, Stephen Warren wrote:

...

I don't quite follow that; linux-next is also purely merge-based.

Are

...

...

you referring to the fact that it's re-created every day, and the source branches that go into the merge can be rebased if needed?

What's the difference between "re-created every day" and "rebased

every

...

day"?

The linux-next model (and what I mean by "re-created every day") is roughly the following approximately daily:

git fetch --all git branch -D tmp git checkout -b tmp u-boot/master # or latest rc in fact git merge u-boot-arm/next git merge u-boot-i2c/next git merge u-boot-video/next etc. git tag next-${datestamp}

I'm not sure what "rebased every day means"; perhaps it's running the following on u-boot/next every day?

git rebase u-boot/master

That doesn't pull in any new commits from child trees though.

By "rebasing" I was referring to rewriting history in any form. It looks like the difference is that linux-next gives the result a new name each time, instead of working a branch with a fixed name.

...

How does merging back down prevent incremental pull requests?

It doesn't prevent incremental pull requests, but it does pollute the history if you merge back down. Instead of a fairly simple:

(M* == main branch, B* == side branch)

B1-B2-B3-B4-B5-B6

/ \ \ M1-M2-M3-M4-M4-M5-M6-M7

you might end up with:

B1-B2-B3-B4-X1-B6

/ \ / \ M1-M2-M3-M4-M4-M5-M6-M7

... where X1 is the merge back from main to the side-branch. That doesn't look a lot more complex, but once there are many side-branches, and the master branch ends up merging a whole bunch of side-branches between M4 and M5 above, the commits in B* that add actual new work are split between a point before the X1 merge-back and after it. This can make tracking down what exactly will get merged into M* when B* is re-merged a bit trickier. git log M..B can probably show it fine, but if you start looking at gitk it'll look quite complex.

I don't use gitk much, but wouldn't it just show the mergeback as another edge in the graph (plus the merge commit itself of course)? It doesn't seem like a big deal.

...

...

...

...

Or often when u-boot/master has made a complete new release does:

b) Creates a new branch based on the latest rc or release from u-boot/master.

That's a rebase. How is that better than a (likely fast-forward)

merge?

No, that's not a rebase.

Fine, a reset. Or a new branch name, which in practice is the same thing except with a much more cluttered branch list, unless you have a specific need to refer to an older iteration of this process.

Rebase is when you take some existing commits based on one commit and apply them to a different baseline commit instead. If you're creating a new branch to take commits for a new release, you're simply not applying the commits for release N+1 until there's a branch ready to take them.

There is a branch ready to take them -- the custodian tree. Why wait until there's an upstream tree ready to pull?

...

...

...

...

(in practice, downstream branches typically end up with

something

...

...

...

...

like for-3.5 based on v3.4-rcN, for-3.6 based on v3.5-rcN, for-3.7 based on v3.6-rcN, some running in parallel containing either important bugfixes for the release or new development as determined by the current state of the various releases in the mainline tree).

I thought you said your way was less work? :-)

And I believe it is; no rebasing required. The difference probably isn't that big though I admit. Still, creating a fresh branch from scratch for each release one time and only then applying patches for that release seems a lot simpler that constantly rebasing stuff all over the place.

We've already dropped the rebasing. I'd rather do merges than constantly create new branches.

...

...

but with the Linux model, you simply:

a) Send pull request.

Admittedly the recipient then might need to resolve some merge conflicts. However, hopefully people have been planning for these

and

...

...

have avoided them.

How do you plan for them and avoid them, and how is that less work

that

...

what we do now?

People have to be aware what is going on.

If you're submitting a bunch of patches which depend on each-other, the submitter had better call that out when sending the patch series.

Sometimes it's not obvious in advance. Not all conflicts (whether they be merge conflicts, build breaks, or runtime failures) come from intrusive tree-wide changes.

...

I especially do not want to have to work with some artificially

chosen

...

old tree as my base. I also do not want to create a bunch of named branches for each

Linux requires that branches be based only on rc or release tags in the equivalent of u-boot/master.

So how do you sort out dependencies? If I have a patch that depends on some patches that have been already applied by custodian X, and others that have already been applied by custodian Y (yes, this happened recently), I have to wait until the next release to deal with them?

U-Boot could choose to be different, and

U-Boot is already different in what RCs mean -- the merge window ends well before rc1. So I couldn't even use my tree to send fixes for issues caused by the merge, until shortly before release. Or rather, I'd have to awkwardly backport the fixes to an old tree and hope that they make it OK when merged into the real tree.

-Scott

On 10/10/2012 10:55:33 AM, Stephen Warren wrote:

On 10/09/2012 06:20 PM, Scott Wood wrote:

...

I don't use gitk much, but wouldn't it just show the mergeback as another edge in the graph (plus the merge commit itself of

course)? It

...

doesn't seem like a big deal.

One big problem is the ability to look back and see easily what's been merged and what work has been committed since then.

If I run "git log u-boot-xxx/master", and look backwards through the list of commits, looking for those committed by the owner of u-boot-xxx, and stop when I see a commit by someone else, I'll end up stopping at the merge commit where u-boot/master was merged back down.

Very often that ends up being a fast forward.

Plus, what about commits that were merged from a tree futher downstream?

If there commits to u-boot-xxx/master between the point where u-boot/master merged u-boot-xxx/master and where u-boot-xxx/master merged u-boot/master back down, then they'll be hidden before that merge point.

Now, I probably could just do git log u-boot/master..u-boot-xxx/master, but that's a but more complicated (and I can never remember which order to give the two branches).

I have a hard time remembering the order too (it seems backwards from what's intuitive), but it's usually obvious when I get the order wrong. And it's faster and more robust than scanning for a commit that looks like it came from somewhere else.

...

...

Rebase is when you take some existing commits based on one commit and apply them to a different baseline commit instead. If you're creating a new branch to take commits for a new release, you're simply not applying the commits for release N+1

until

...

...

there's a branch ready to take them.

There is a branch ready to take them -- the custodian tree. Why

wait

...

until there's an upstream tree ready to pull?

Well, the two possibilities are:

a) Send pull request b) Wait for pull request to be merged c) Here, or later, merge back down u-boot/master. Note this cannot happen before (b) has completed, or the merge of this branch upstream may pick up extra commits that weren't in the pull request. d) Continue merging commits to the same branch without reset/...

or:

a) Send pull request b) Wait for pull request to be merged c) Reset u-boot-xxx/master to u-boot/master or some rc or ... d) Continue merging commits

In either case, one must always wait for the pull request send in (b) to be completely merged so that when the pull request is merged, it doesn't pick up commits to the branch that were added after the pull request was sent. So, there isn't a tree ready to take new patches until a pull request has been completely processed.

Ideally once a pull request happens the pull happens quickly. If that doesn't happen, you could reply to the pull request asking that it be ignored in favor of a new pull request, or create a new temporary branch. IMHO pull requests ought to request a pull of a specific SHA1 along with the branch name (it'd be nice if the command to pull could verify that the SHA1 is in the history of the branch name, and include the branch name in the commit message, but merge based on the SHA1).

...

...

...

...

...

> (in practice, downstream branches typically end up with

something

...

...

...

...

...

> like for-3.5 based on v3.4-rcN, for-3.6 based on v3.5-rcN, > for-3.7 based on v3.6-rcN, some running in parallel

containing

...

...

...

...

...

> either important bugfixes for the release or new development

as

...

...

...

...

...

> determined by the current state of the various releases in

the

...

...

...

...

...

> mainline tree).

I thought you said your way was less work? :-)

And I believe it is; no rebasing required. The difference probably isn't that big though I admit. Still, creating a fresh branch from

scratch for

...

...

each release one time and only then applying patches for that

release

...

...

seems a lot simpler that constantly rebasing stuff all over the

place.

...

We've already dropped the rebasing.

The whole reason I started this thread was because rebasing was still being used.

Rebasing is no longer supposed to be part of the standard workflow, as you portrayed it. I'm not sure why the rebasing happened in this particular instance.

...

I'd rather do merges than constantly create new branches.

...

...

...

...

but with the Linux model, you simply:

a) Send pull request.

Admittedly the recipient then might need to resolve some merge conflicts. However, hopefully people have been planning for

these and

...

...

...

...

have avoided them.

How do you plan for them and avoid them, and how is that less

work that

...

...

...

what we do now?

People have to be aware what is going on.

If you're submitting a bunch of patches which depend on

each-other, the

...

...

submitter had better call that out when sending the patch series.

Sometimes it's not obvious in advance. Not all conflicts (whether

they

...

be merge conflicts, build breaks, or runtime failures) come from intrusive tree-wide changes.

Since u-boot/next appears to be a rolling/parallel model, where changes can be pulled into it at arbitrary times even when u-boot/master is being used to stabilize a release, the way to resolve conflicts for U-Boot is simply to have the branch containing the dependency push their /next into u-boot/next, then the branch needing the dependency to branch from or merge u-boot/next into their /next, then apply the patch with the dependency.

Again, it's not always obvious in advance. The patches that conflict may have already been applied to their respective custodian trees.

-Scott

Dear Stephen Warren,

In message 5075F48A.2080504@wwwdotorg.org you wrote:

I believe that's (part of) why Linux is tending towards pull requests of a (signed) tag rather than a branch, since the tag always points at a specific commit, and incremental pull requests can just create a new tag name. Of course, the ability to sign tags also also a motivator.

I tend to disagree here. Tags can easily be removed. As such, they are in no way different from or better than a specific commit in a branch that does not get rebased.

Best regards,

Wolfgang Denk

On 10/11/2012 01:19 AM, Wolfgang Denk wrote:

Dear Stephen Warren,

In message 5075F48A.2080504@wwwdotorg.org you wrote:

...

I believe that's (part of) why Linux is tending towards pull requests of a (signed) tag rather than a branch, since the tag always points at a specific commit, and incremental pull requests can just create a new tag name. Of course, the ability to sign tags also also a motivator.

I tend to disagree here. Tags can easily be removed. As such, they are in no way different from or better than a specific commit in a branch that does not get rebased.

True, tags can be moved. However, the point wasn't that they're immutable, but that using them can decouple the pull process from the commit process. For example, I could:

git checkout -b foo bar git am git am git am git tag -s tag1 foo send pull request not wait for pull to complete git am git am git am git tag -s tag2 foo send pull request

So, I can still apply stuff to a branch even in parallel with committing new stuff. IIRC, the point I was originally responding to above was the issue that by sending pull requests for somebody to pull a branch, if you added commits to it before the pull was complete, they'd be pulled in too. With tags, you can avoid that (or by creating a new branch too, but anyway)

Hi Scott,

On Wed, 10 Oct 2012 17:02:18 -0500, Scott Wood scottwood@freescale.com wrote:

Ideally once a pull request happens the pull happens quickly. If that doesn't happen, you could reply to the pull request asking that it be ignored in favor of a new pull request, or create a new temporary branch. IMHO pull requests ought to request a pull of a specific SHA1 along with the branch name (it'd be nice if the command to pull could verify that the SHA1 is in the history of the branch name, and include the branch name in the commit message, but merge based on the SHA1).

Note that for recent enough versions of git, request-pull will indicate the last commit ID to pull, which makes it possible for them to keep on adding to their branch once their PR is out. Not all U-Boot ARM custodians have a recent enough git, mind.

Amicalement,

On 10/11/2012 01:28 AM, Wolfgang Denk wrote:

Dear Stephen Warren,

In message 50759A75.8060105@wwwdotorg.org you wrote:

...

Do note that linux-next doesn't become the next Linux kernel version either; it's just a preview of the merges Linus will do. Linus re-does all the merges based on the pull requests people actually send him. So, the model is very different to u-boot/next.

Yes. And experience has shown that linux-next may be useful for specific testing, but it is utterly useless as base for any development work.

My experience doesn't agree. I almost exclusively use linux-next as the underlying branch for all my development work. Now and then I do suffer from finding the occasional problem, but it's better to find them and fix them early rather than not seeing them until Linus merges everything and having to debug through a couple months of history to find the bug, so overall even that extra effort works out to be a net gain.

When you need a tree which has all the latest and greatest commits (which have already been accepted for inclusion into the next release), then next is not your choice, because it will crumble below your fingers.

(I assume that in "then next is not your choice" above, "next" is referring to a linux-next style model)

Yes, with U-Boot's next model (or what I understand it to be...) a linux-next style branch/model/.. is indeed not needed; just use u-boot/next instead. At least, I think that's what you're saying.

...

If I run "git log u-boot-xxx/master", and look backwards through the list of commits, looking for those committed by the owner of u-boot-xxx,

Who cares about the comitter? This is just a technical act. It makes zero difference whether I apply a patch or somebody else applies the same.

Well, if git tells me someone committed something, it should be right. I could apply just the same argument to writing the code; who cares who wrote it so long as it's in the code-base?

Your assertion is probably coupled with U-Boot's interpretation of the Signed-off-by tag too. Linux requires both the original author and each committer/cherry-picker to sign-off the commit to document the entire path the commit went through, In that scenario, a commit that has the git committer field set to some value yet the last Signed-off-by entry isn't that person is a red flag. Hence, the git committer field is important.

However, U-Boot is reported to only use Signed-off to indicate the original author(s), so I can see how the git committer field isn't considered important in U-Boot. I'm not convinced it's a good idea to be so cavalier about git metadata though; wouldn't it be better to take simple steps (never rebase any other committer's commits) so that the metadata is always correct?

...

and stop when I see a commit by someone else, I'll end up stopping at the merge commit where u-boot/master was merged back down. If there commits to u-boot-xxx/master between the point where u-boot/master merged u-boot-xxx/master and where u-boot-xxx/master merged u-boot/master back down, then they'll be hidden before that merge point.

I have to admit that I never ever cared about any merges before. For me they happen because of that's the way how development goes, i. e. they cannot be avoided, but that's all. I have only been interested in which commits have ben included, not which branch or repository these have been coming from.

Why would I need to know that?

Personally I believe it's a good idea to document the whole commit/merge process fully. That's for all the same reasons that git contains metadata for author, committer, datestamps, log messages, etc. The whole point of source control is to document the development process; otherwise, we could just publish the latest .tar and ignore git entirely.

...

...

Since u-boot/next appears to be a rolling/parallel model, where changes can be pulled into it at arbitrary times even when u-boot/master is being used to stabilize a release, the way to resolve conflicts for U-Boot is simply to have the branch containing the dependency push their /next into u-boot/next, then the branch needing the dependency to branch from or merge u-boot/next into their /next, then apply the patch with the dependency.

Sorry, I cannot parse this.

If you have two commits or series that depend on each-other, e.g. series A introduces a new feature and series B makes use of that new feature, and those two series must be applied to different downstream trees for some reason, then the process would be:

1) commit series A to downstream branch X 2) pull downstream branch X into u-boot/next 3) either re-create downstream branch Y from u-boot/next, or merge u-boot/next into existing downstream branch Y 4) commit series B to downstream branch Y

The thing that makes this work easily in U-Boot is that the pull request into u-boot/next in (2) above can basically happen any time, so it's a very low-latency process. In Linux, there is only a specific time this can happen (during the merge window), so the latency for such a process is very high, and more convoluted processes much be followed for such dependencies. I do like U-Boot's process better in this regard.

Hi Tom,

On Tue, 9 Oct 2012 14:32:08 -0700, Tom Rini trini@ti.com wrote:

On Tue, Oct 09, 2012 at 03:03:28PM -0600, Stephen Warren wrote:

...

On 10/09/2012 08:23 AM, Tom Rini wrote:

...

On Sun, Oct 07, 2012 at 08:49:00PM +0200, Marek Vasut wrote:

...

NOTE: I get a few more size issues with ELDK 4.2 on IXP (that big-endian ARM) after this patchset is applied. I wonder if we shouldn't just throw these away, since they're dead code mostly.

The following changes since commit c7ee66a8222660b565e9240775efa4c82cb348c2:

Merge branch 'next' of git://www.denx.de/git/u-boot-ppc4xx into next (2012-10-02 10:16:40 -0700)

are available in the git repository at:

git://git.denx.de/u-boot-usb.git next

for you to fetch changes up to f0ede0e8305bc3c959862446bce40cb028b36293:

usb.h: Add udc_disconnect prototype to usb.h (2012-10-07 02:08:48 +0200)

I had to rebase this locally to merge (such is next), and now it's applied to u-boot/next, thanks!

Hmm. Can't "git merge" solve merge conflicts just as well as "git rebase"?

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

The short answer is that right now, u-boot/next follows the linux-next model and we rebase as needed.

...

It would be awesome if U-Boot could adopt something more similar to the Linux kernel's git usage model, namely:

• All downstream branches are based off some known stable point in the

master branch (e.g. 2012.10-rc1). Before these branches are merged into any other branch, they can be rebased if absolutely needed, but preferably not.

• Once a downstream branch is merged upwards, the downstream branch

doesn't merge upstream back down into the downstream branch, but either:

a) Keeps adding to the existing branch so that incremental pull requests can be sent.

Or often when u-boot/master has made a complete new release does:

b) Creates a new branch based on the latest rc or release from u-boot/master.

(in practice, downstream branches typically end up with something like for-3.5 based on v3.4-rcN, for-3.6 based on v3.5-rcN, for-3.7 based on v3.6-rcN, some running in parallel containing either important bugfixes for the release or new development as determined by the current state of the various releases in the mainline tree).

• When a branch is merged from a repo to a parent repo, it's always a

git merge --no-ff; never a rebase or fast-forward.

• In order to resolve merge conflicts/dependencies between different

downstream branches, one of the following happens:

a) The first downstream branch gets merged into u-boot/master. b) The second downstream branch creates a new branch starting at an an rc or release in u-boot-master that contains it the required patches. c) The dependent patches are applied to the second downstream branch. d) The second downstream branch gets merged into u-boot/master.

All the patches that would usually be merged through downstream branch 2 actually get ack'd by the maintainer of downstream branch 2 and applied to downstream branch 1 after the patches they depend on. This is simplest, but may cause complications if both branches need to take patches that build on the merged patches they're merged into an rc or release in u-boot/master.

A topic branch is created by one of the downstream maintainers, branched from a u-boot/master rc or release, and containing just the patches that other patches depend on, and this topic branch gets merged into both the two downstream branches for further work.

Yes, this does all take a little bit more thought, planning, and co-ordination, but I think having a simpler and more stable git history is worth it.

Interesting. As this is more work on the custodians end, what does everyone else say?

IIUC the current rule for U-Boot is that master branches do not rebase while next branches can (as Tom said).

Apart from this, I'm not sure why forbidding fast-forward is a good thing, but if there are benefits, why not.

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.

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.

Amicalement,

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,

Hi Stephen,

...

...

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.

The documentation of merge commits seems good to me just in an of itself.

"It's intrinsically good" sounds like a circular or axiomatic justification to me.

...

...

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

Hiding patches within merges wasn't something I'd considered at all; I just assumed that would never happen.

Good. Since we agree that patches never get hidden in merge commits, then each patch always has its own commit with its "by" tags.

...

...

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).

At least the way the Linux kernel uses the tag, both the original author of the patch /and/ anyone who applies the patch, cherry-picks the patch, ... must add their S-o-b line. I think U-Boot isn't using that part of the model.

No, it isn't. IIUC, U-Boot's "Signed-off-by" is supposed to mean "I am (one of) the autor(s) of this patch".

...

...

...

...

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."

Hmmm. That makes custodian trees very multi-purpose; they have to both:

a) Be used for feeding patches upstream. b) Be a useful independent version of U-Boot.

That seems like conflating two pretty different requirements. I'm personally not sure that's a good idea.

Actually, b) is correct but a) is not how I would have expressed it, if only because u-boot itself is a custodian tree, and u-boot-arm is both feeding mainline and fed by other trees. How I would express it is that a custodian tree:

a) feeds *from* patches and pulls (actually, fetches) from other custodian trees;

b) is a useful independent (i.e. functional) U-Boot tree.

Now, obviously the custodian trees should be buildable and run fine on all platforms; patches shouldn't break functionality. But, I don't think that any tree either that u-boot.git should be seen as somewhere you'd expect to be able to use a combination of features from different custodian trees before they're merged into u-boot.git, which is what the description above implies to me.

You're overspecifying the definition I showed you. It does not say custodian trees should provide "a combination of features from different custodian trees before they're merged into u-boot.git"; it only says they should be "independent incarnations of U-Boot source trees which users (note: users, not only developers) with specific needs or interests can refer to". Not a word about having to combine features. This you /can/ do, of course, in a custodian tree; but they are not done for this purpose.

...

...

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.

Yes, using an identical process for next and master makes sense to me.

...

But that's not making the point (IMO) that we should have a flurry of branch names.

True, that's an entirely orthogonal issue. I mainly raised that point as an example from the kernel. What I really started this conversation about was not using rebases in either master or next, and the conversation has started to concentrate more on other things.

However, there are times when rebasing, and reordering even, might be required -- think, for instance, of an important patch that should be placed as early as possible in the next release, or inversely, a patch that was put in next release and now sits in the middle of other commits, but reveals faulty. There would be cause to pick this commit out of the next tree before it becomes master.

Amicalement,

Hi Scott,

On Thu, 11 Oct 2012 11:54:46 -0500, Scott Wood scottwood@freescale.com wrote:

On 10/10/2012 01:40:54 PM, Albert ARIBAUD wrote:

...

...

...

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).

At least the way the Linux kernel uses the tag, both the original

author

...

of the patch /and/ anyone who applies the patch, cherry-picks the

patch,

...

... must add their S-o-b line. I think U-Boot isn't using that part

of

...

the model.

No, it isn't. IIUC, U-Boot's "Signed-off-by" is supposed to mean "I am (one of) the autor(s) of this patch".

Is this documented anywhere?

http://www.denx.de/wiki/U-Boot/DevelopmentProcess says, "U-Boot has adopted the Linux kernel signoff policy".

Please do read the Linux kernel signoff policy as laid out in Documentation/SubmittingPatches. Branch or subsystem maintainers should add their Signed-off-by only if they made modifications to the original patch in the process of applying it.

Then read http://www.denx.de/wiki/U-Boot/Patches: "the Signed-off-by: is a line at the end of the commit message by which the signer certifies that he was involved in the development of the patch and that he accepts the Developer's Certificate of Origin (see SubmittingPatches).

In U-Boot, we typically do not add a Signed-off-by: if we just pass on a patch without any changes".

(the "Certificate of Origin" is laid out in the "SubmittingPatches" documentation file from Linux)

Actual behavior is probably inconsistent between custodians.

I haven't seen such inconsistency and certainly don't want to see any, at least in ARM trees from which I have to pull.

Amicalement,

Hi Stephen,

On Thu, 11 Oct 2012 11:26:45 -0600, Stephen Warren swarren@wwwdotorg.org wrote:

On 10/11/2012 11:16 AM, Albert ARIBAUD wrote:

...

Hi Scott,

On Thu, 11 Oct 2012 11:54:46 -0500, Scott Wood scottwood@freescale.com wrote:

...

On 10/10/2012 01:40:54 PM, Albert ARIBAUD wrote:

...

...

...

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).

At least the way the Linux kernel uses the tag, both the original

author

...

of the patch /and/ anyone who applies the patch, cherry-picks the

patch,

...

... must add their S-o-b line. I think U-Boot isn't using that part

of

...

the model.

No, it isn't. IIUC, U-Boot's "Signed-off-by" is supposed to mean "I am (one of) the autor(s) of this patch".

Is this documented anywhere?

http://www.denx.de/wiki/U-Boot/DevelopmentProcess says, "U-Boot has adopted the Linux kernel signoff policy".

Please do read the Linux kernel signoff policy as laid out in Documentation/SubmittingPatches. Branch or subsystem maintainers should add their Signed-off-by only if they made modifications to the original patch in the process of applying it.

That's certainly not what I understand from reading that document. Can you please point out the part the states that policy?

(The part I think you may be talking about is that when you edit a patch, it is polite to add a note indicating what you changed *in addition* to adding your Signed-off-by tag):

Quoting that doc:

...

If you are a subsystem or branch maintainer, sometimes you need to slightly modify patches you receive in order to merge them, because the code is not exactly the same in your tree and the submitters'. If you stick strictly to rule (c), you should ask the submitter to rediff, but this is a totally counter-productive waste of time and energy. Rule (b) allows you to adjust the code, but then it is very impolite to change one submitter's code and make him endorse your bugs. To solve this problem, it is recommended that you add a line between the last Signed-off-by header and yours, indicating the nature of your changes. While there is nothing mandatory about this, it seems like prepending the description with your mail and/or name, all enclosed in square brackets, is noticeable enough to make it obvious that you are responsible for last-minute changes. Example :

    Signed-off-by: Random J Developer <random@developer.example.org>
    [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h]
    Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org>

and in particular, the following parts of that doc is what tells me that committers should always add S-o-b even if the commit didn't change:

...

    Developer's Certificate of Origin 1.1

    By making a contribution to this project, I certify that:

...

...

    (c) The contribution was provided directly to me by some other
        person who certified (a), (b) or (c) and I have not modified
        it.

...

The Signed-off-by: tag indicates that the signer was involved in the development of the patch, or that he/she was in the patch's delivery path.

My bad. I've indeed misread the Linux doc. However, the U-Boot doc is clearly on the side of "no Signed-off-by from custodians".

Amicalement,

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

On 10/13/12 12:30, Wolfgang Denk wrote:

[snip]

Yes, I am aware it is also possible to interpret this as "anybody in the patch's delivery path" - but even then I cannot derive any obligation for such a passer-on to add his SoB.

While also IANAL (and I try and stay out of these discussions), paging around in the kernel log it sure seems like Linus and akpm both add S-O-B when they git am something in (perhaps why there is git am - --signoff?) but not when it comes via pull request. In other words, up until the point it goes into intended-to-be-pulled-somehow-someway git, whomever is presenting the work adds one (or more) to say I (we) worked on it and yes, applies as well as the person bringing it in (I touch this and believe it to be).

- -- Tom

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

On 10/13/12 15:25, Wolfgang Denk wrote:

Dear Tom,

In message 5079D95E.4070609@ti.com you wrote:

...

While also IANAL (and I try and stay out of these discussions), paging around in the kernel log it sure seems like Linus and akpm both add S-O-B when they git am something in (perhaps why there is git am --signoff?) but not when it comes via pull request. In other words, up until the point it goes into intended-to-be-pulled-somehow-someway git, whomever is presenting the work adds one (or more) to say I (we) worked on it and yes, applies as well as the person bringing it in (I touch this and believe it to be).

Yes, git am has such an option. But git fetch (or pull) does not. I see no technical difference if someone provides me a patch as such, or in form of a git repository with this patch applied so I can just "git fetch" from it. In both cases the result would be exactly the same: I add the patch to my local repository. But in one case I am supposed to sign it (and tools offer me options to do so), but in the other case I cannot do that, even if I wanted?

I will not claim the kernel practice to be 100% consistent, but yes. git am --signoff, git pull/merge and no -s in merge commits seems to be the practice. Perhaps we should stop saying we follow the kernel process, link to it as useful background, but then document what we actually want / do which is only require new S-O-B on code modification, and allow custodians to add their own they want for tracking or otherwise ease of not having to remember a different workflow for kernel vs U-Boot?

- -- Tom

Dear Stephen Warren,

In message 50770155.20700@wwwdotorg.org you wrote:

and in particular, the following parts of that doc is what tells me that committers should always add S-o-b even if the commit didn't change:

...

    Developer's Certificate of Origin 1.1

    By making a contribution to this project, I certify that:

...

...

    (c) The contribution was provided directly to me by some other
        person who certified (a), (b) or (c) and I have not modified
        it.

No, I think you misinterpret this ;-)

This is intended for cases where the original author of the patch shall remain unknown for whatever reasons. Consider some bigger companies doing a lot of their actual development in low-cost countries (say, China). They usually have a ton of developers workignon such stuff, and only one (or very few) people who "interface" ith the community. It is these interface-guys who will add their SoB based on above rule, meaning: yes, I can certify that this is Open Source, and even though the original author shall remain unnamed this can be used freely in this context.

I don't see how you derive fromt hat that a custodian applying a patch without modifications should add a SoB? If so, then please explain where the limits are? Aplying from a mailbox file from a mailing list? Or from some archive (say, patchwork)? Or pulling from some repository provided by the original author? Or pulling from a downstream repository in your own project?

Spinning this to an end consequently, I think we would have to add a new SoB line to all commits for any git pull we are doing (which caanot be the intention, and which cannot work).

Best regards,

Wolfgang Denk

Dear Stephen Warren,

In message 507C3AA4.6050707@wwwdotorg.org you wrote:

Irrespective of the documentation (which I obviously read the way I describe anyway...), the kernel practice is that everyone who writes or commits a patch adds their S-o-b line, and everyone who simply merges a

I'm aware of this.

branch from someone else checks that the provider of the branch added their S-o-b to patches they applied (rather than merged themselves) but does not add their own S-o-b (because it's impossible).

Is such checking really taking place? Are there any tools to support this?

I have often wondered why the merge commits themselves were not signed off by the person performing the merge (which would then logically cover all the commits that got merged). I haven't investigated to find out why though. Doing so would seem to solve your objection about merges.

I'm not only concerned about merges from custodian (or maintainer's, in Linux terminology) trees. For me it also seems reasonable to pull from any other repository instead of using git-am or similar to apply patches from a mailbox file. Herer I also have no way to add any new SoBs.

Best regards,

Wolfgang Denk

On 10/11/2012 12:16:58 PM, Albert ARIBAUD wrote:

Hi Scott,

On Thu, 11 Oct 2012 11:54:46 -0500, Scott Wood scottwood@freescale.com wrote:

...

On 10/10/2012 01:40:54 PM, Albert ARIBAUD wrote:

...

...

...

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).

...

...

...

At least the way the Linux kernel uses the tag, both the

original

...

...

author

...

of the patch /and/ anyone who applies the patch, cherry-picks

the

...

...

patch,

...

... must add their S-o-b line. I think U-Boot isn't using that

part

...

...

of

...

the model.

No, it isn't. IIUC, U-Boot's "Signed-off-by" is supposed to mean

"I

...

...

am (one of) the autor(s) of this patch".

Is this documented anywhere?

http://www.denx.de/wiki/U-Boot/DevelopmentProcess says, "U-Boot has adopted the Linux kernel signoff policy".

Please do read the Linux kernel signoff policy as laid out in Documentation/SubmittingPatches.

You want me to read the Linux policy for documentation of how U-Boot deviates from Linux policy?

Branch or subsystem maintainers should add their Signed-off-by only if they made modifications to the original patch in the process of applying it.

That's not what it says.

Then read http://www.denx.de/wiki/U-Boot/Patches: "the Signed-off-by: is a line at the end of the commit message by which the signer certifies that he was involved in the development of the patch and that he accepts the Developer's Certificate of Origin (see SubmittingPatches).

In U-Boot, we typically do not add a Signed-off-by: if we just pass on a patch without any changes".

Thanks. FWIW I think putting policy documents in a wiki, without any guidance on who's supposed to edit it or how changes get approved, is a bad idea. Why not put policy documents in the git-managed source tree? And changes would be proposed, discussed, and accepted/rejected like any other change. Plus there'd be at least a chance of a commit message showing rationale.

In any case, if this is the policy, we should not be saying that we follow the Linux policy.

(the "Certificate of Origin" is laid out in the "SubmittingPatches" documentation file from Linux)

...

Actual behavior is probably inconsistent between custodians.

I haven't seen such inconsistency and certainly don't want to see any, at least in ARM trees from which I have to pull.

I've been signing off patches I apply to the NAND tree. I recall stopping at one point in the past because someone complained, and then starting again -- not sure if someone else complained about doing it *that* way, or if I just noticed others doing it.

Looking through the history I see others that seems to be doing the same (outside ARMland), though I can't say for sure without investigating whether the patch was "passed on without any changes" in each instance.

-Scott

On 10/11/2012 01:45:02 PM, Albert ARIBAUD wrote:

Hi Scott,

On Thu, 11 Oct 2012 13:13:33 -0500, Scott Wood scottwood@freescale.com wrote:

...

FWIW I think putting policy documents in a wiki, without any guidance on who's supposed to edit it or how changes get approved,

is a

...

bad idea. Why not put policy documents in the git-managed source tree? And changes would be proposed, discussed, and accepted/rejected like any other change.

Plus

...

there'd be at least a chance of a commit message showing rationale.

While I can see the benefits you find in this, is it not based on the unspoken axiom that the project's policies should necessarily be subject to a democratic process?

Process is othogonal to revision control. We could vote on whether a policy patch gets applied, though I do not think U-Boot is currently democraticly run, except to the extent that Wolfgang sometimes changes his mind if enough people complain. I do not know of any existing democratic process for approving a wiki update, and would hesitate to just go make a change.

As for the merits of the policy itself, I find maintainer signoffs to be useful, for example to distinguish a patch that I've applied locally versus one that I've fetched from upstream.

-Scott

On 10/12/2012 05:11:17 AM, Albert ARIBAUD wrote:

Hi Scott,

On Thu, 11 Oct 2012 13:59:31 -0500, Scott Wood scottwood@freescale.com wrote:

...

On 10/11/2012 01:45:02 PM, Albert ARIBAUD wrote:

...

Hi Scott,

On Thu, 11 Oct 2012 13:13:33 -0500, Scott Wood scottwood@freescale.com wrote:

...

FWIW I think putting policy documents in a wiki, without any guidance on who's supposed to edit it or how changes get

approved,

...

...

is a

...

bad idea. Why not put policy documents in the git-managed

source

...

...

...

tree? And changes would be proposed, discussed, and accepted/rejected like any other

change.

...

...

Plus

...

there'd be at least a chance of a commit message showing

rationale.

...

...

While I can see the benefits you find in this, is it not based on the unspoken axiom that the project's policies should necessarily

be

...

...

subject to a democratic process?

Process is othogonal to revision control. We could vote on whether

a

...

policy patch gets applied, though I do not think U-Boot is currently democraticly run, except to the extent that Wolfgang sometimes

changes

...

his mind if enough people complain. I do not know of any existing democratic process for approving a wiki update, and would hesitate

to

...

just go make a change.

My remark was that Stephen took the democracy for granted in the process, not that there was a relationship to be drawn between process and revision control.

OK, I misread what you said. I don't think my comment (I assume you meant mine and not Stephen's, given what it's a reply to) assumes any such thing. Wolfgang is free to NACK any patch that changes policy in ways he doesn't like. With the wiki it's not clear who should make changes to policy documents and under what circumstances, but it doesn't appear to be just Wolfgang, as he has said things like "it's a wiki, go edit it".

http://www.mail-archive.com/u-boot@lists.denx.de/msg87395.html http://www.mail-archive.com/u-boot@lists.denx.de/msg12795.html

...

As for the merits of the policy itself, I find maintainer signoffs

to

...

be useful, for example to distinguish a patch that I've applied

locally

...

versus one that I've fetched from upstream.

This you can see by looking at the upstream branch tip, the patch's committer identity or by doing a git branch -r --contains <commit-id>.

Sure, I was just describing a way in which I found it useful. Are there any benefits to U-Boot's current policy that justify deviating from what Signed-off-by: means in Linux?

-Scott

Dear Scott Wood,

In message 1349974486.6903.5@snotra you wrote:

Is this documented anywhere?

http://www.denx.de/wiki/U-Boot/DevelopmentProcess says, "U-Boot has adopted the Linux kernel signoff policy".

Actual behavior is probably inconsistent between custodians.

This is documented, see previous discussions.

We're referring to linux/Documentation/SubmittingPatches .

Unfortunaltely this leaves room to interpretation, and depending on thier own interpretation people behave differently.

To me the most interesting part of this document is this section:

If you are a subsystem or branch maintainer, sometimes you need to slightly modify patches you receive in order to merge them, because the code is not exactly the same in your tree and the submitters'. If you stick strictly to rule (c), you should ask the submitter to rediff, but this is a totally counter-productive waste of time and energy. Rule (b) allows you to adjust the code, but then it is very impolite to change one submitter's code and make him endorse your bugs. To solve this problem, it is recommended that you add a line between the last Signed-off-by header and yours, indicating the nature of your changes. While there is nothing mandatory about this, it seems like prepending the description with your mail and/or name, all enclosed in square brackets, is noticeable enough to make it obvious that you are responsible for last-minute changes. Example :

Signed-off-by: Random J Developer random@developer.example.org [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h] Signed-off-by: Lucky K Maintainer lucky@maintainer.example.org

This practise is particularly helpful if you maintain a stable branch and want at the same time to credit the author, track changes, merge the fix, and protect the submitter from complaints. Note that under no circumstances can you change the author's identity (the From header), as it is the one which appears in the changelog.

In my interpretation this means that one should add a SoB line of one creates or modifies a patch. Just applying it from some published and recoded form does not include any such modification, so why should I add my SoB when applying a patch from PW for example?

Best regards,

Wolfgang Denk

On Tue, Oct 09, 2012 at 02:32:08PM -0700, Tom Rini wrote:

On Tue, Oct 09, 2012 at 03:03:28PM -0600, Stephen Warren wrote:

[snip]

...

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

The short answer is that right now, u-boot/next follows the linux-next model and we rebase as needed.

I'm going to reply to myself, in hopes of clearing things up. We don't follow the linux-next model, really, I miss-spoke.

History is important. But so is getting the amount of process for the size of the project. The other thing is that we're doing simultaneous development for both the current release and the next release.

So for the master branch of the master repo, it must never rebase. And as Wolfgang encourages users to use the custodian repository of mainline isn't quite up to what they need, custodian repositories must also keep their master branch un-rebased as much as humanly possible (my rule of thumb would be once it's been in the wild for a few days, it's too late).

The next branch however can be rebased, as needed. In the case of post-v2012.10, it will be rebased as we want the commit to change how ARM and unaligned accesses are handled to be the first thing. I don't think "perfect" "changes A-G were done in repository X against tree Y" is the most useful bit of information. When we rebase we may lose that boards 1/2/3 worked at point Y but we gain change D is when board 2 broke as part of being merged with other changes.

On 10/11/2012 11:16 AM, Scott Wood wrote:

On 10/11/2012 11:38:00 AM, Tom Rini wrote:

...

On Tue, Oct 09, 2012 at 02:32:08PM -0700, Tom Rini wrote:

...

...

In the case of post-v2012.10, it will be rebased as we want the commit to change how ARM and unaligned accesses are handled to be the first thing.

Any particular reason, short of telling people whose patches have already been accepted that they need to respin them?

(I'm really replying to Tom here not Scott)

As I understand it, the process for getting u-boot/next into u-boot/master is perhaps:

1) tag u-boot/master for the release 2) merge u-boot/next into u-boot/master (or perhaps you're proposing rebase u-boot/next onto u-boot/master and then fast-forward merge it).

I think the solution to getting patches into u-boot/master first is therefore:

1) tag u-boot/master for the release 2) apply, cherry-pick, or merge a branch with just the early commits into u-boot/master 3) merge u-boot/next into u-boot/master

(hopefully (3) doesn't contain duplicated commits with (2)... the beauty of the Linux kernel's model is that it wouldn't, and even with U-Boot's model it wouldn't with sufficient planning)

That way, everything can still be merges, and hence preserve commit IDs, but we also get any early commits in early.

On 10/11/2012 12:27:57 PM, Tom Rini wrote:

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

On 10/11/12 10:16, Scott Wood wrote:

...

On 10/11/2012 11:38:00 AM, Tom Rini wrote:

...

On Tue, Oct 09, 2012 at 02:32:08PM -0700, Tom Rini wrote:

...

On Tue, Oct 09, 2012 at 03:03:28PM -0600, Stephen Warren wrote:

[snip]

...

...

The problem with rebasing when pulling is that git commit IDs change, so it's much more difficult to determine when a commit is merged into a parent tree; one has to search by commit subject rather than just executing e.g. git branch -a --contains XXX. I thought Albert just agreed to use merges rather than rebases for u-boot-arm for this and perhaps other reasons.

The short answer is that right now, u-boot/next follows the linux-next model and we rebase as needed.

I'm going to reply to myself, in hopes of clearing things up. We don't follow the linux-next model, really, I miss-spoke.

History is important. But so is getting the amount of process for the size of the project. The other thing is that we're doing simultaneous development for both the current release and the next release.

So for the master branch of the master repo, it must never rebase. And as Wolfgang encourages users to use the custodian repository of mainline isn't quite up to what they need, custodian repositories must also keep their master branch un-rebased as much as humanly possible (my rule of thumb would be once it's been in the wild for a few days, it's too late).

The next branch however can be rebased, as needed.

Why is the next branch any different? Users and custodians will both be affected by any rebase, just as if a master branch gets rebased. This hybrid of the Linux approach and what was described in this thread as the U-Boot approach is worse than consistently doing one or the other IMHO.

The number of people working on next should be small and manageable.

More people likely use next than some custodian branches, and those are supposed to be rebase-free...

Wouldn't anyone developing code for the next merge window be working on next, at least when it's not out of date?

But, yes, it bears more thinking if we want the next branch open for longer than it has historically been, if we want that. And we have at least historically been saying that next can and will be rebased.

IMHO it would be nice for the next branch to open immediately after the merge window closes, and be kept up to date except during the merge window. Historically the next branch has opened when someone requests a pull into it, but how do I make a sane pull request when the next branch has been untouched since the last release?

-Scott

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

On 10/11/12 22:29, Stefan Roese wrote:

On 10/11/2012 08:30 PM, Scott Wood wrote:

...

...

But, yes, it bears more thinking if we want the next branch open for longer than it has historically been, if we want that. And we have at least historically been saying that next can and will be rebased.

IMHO it would be nice for the next branch to open immediately after the merge window closes, and be kept up to date except during the merge window. Historically the next branch has opened when someone requests a pull into it, but how do I make a sane pull request when the next branch has been untouched since the last release?

This has been discussed on the U-Boot BoF in Geneva a few months ago. And it was generally agreed upon that opening the next branch earlier (best directly after the merge window) would be a big plus.

IIRC, then Tom did exactly this in this release cycle.

Yes, I got -rc1 out sooner than we do historically. I think there's a few changes I would like to make about how we handle branching, releasing and merge windows. But this thread has reminded me that I need to think very carefully about how we manage branches that are longer lived. And I don't want to make big changes for v2013.01 but prepare everyone to try them for v2013.04 (and .07).

- -- Tom