mbox

[U-Boot,PULL] u-boot-usb/next

Message ID 201210072049.00393.marex@denx.de
State Accepted
Delegated to: Tom Rini
Headers show

Pull-request

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

Message

Marek Vasut Oct. 7, 2012, 6:49 p.m. UTC
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)

----------------------------------------------------------------
Lucas Stach (7):
      usb: lowlevel interface change to support multiple controllers
      usb: ehci: rework to take advantage of new lowlevel interface
      usb: add support for multiple usb controllers
      tegra20: port to new ehci interface
      usb: ehci: don't print debug output
      usb: ulpi: add indicator configuration function
      tegra20: add USB ULPI init code

Lukasz Dalek (5):
      usbether: Fixed bug when using with PXA25X chips
      usbether: Define CONFIG_USB_ETH_{CDC,SUBSET}
      usbether: Removed DEV_CONFIG_{CDC,SUBSET}
      pxa25x: Add support for USB ethernet gadget
      usb.h: Add udc_disconnect prototype to usb.h

 README                                        |    3 +
 arch/arm/cpu/arm920t/s3c24x0/usb_ohci.c       |    4 +-
 arch/arm/cpu/armv7/tegra20/usb.c              |  168 ++++--
 arch/arm/include/asm/arch-tegra20/usb.h       |   33 +-
 arch/arm/include/asm/ehci-omap.h              |   10 +-
 arch/mips/cpu/mips32/au1x00/au1x00_usb_ohci.c |    4 +-
 arch/powerpc/cpu/mpc5xxx/usb_ohci.c           |    4 +-
 arch/powerpc/cpu/ppc4xx/usb_ohci.c            |    4 +-
 arch/sparc/cpu/leon3/usb_uhci.c               |    4 +-
 arch/sparc/lib/bootm.c                        |    2 +-
 board/htkw/mcx/mcx.c                          |    6 +-
 board/mpl/common/usb_uhci.c                   |    4 +-
 board/technexion/twister/twister.c            |    6 +-
 board/teejet/mt_ventoux/mt_ventoux.c          |    6 +-
 board/ti/beagle/beagle.c                      |    6 +-
 board/ti/panda/panda.c                        |    6 +-
 common/cmd_usb.c                              |   16 +-
 common/usb.c                                  |  108 ++--
 common/usb_hub.c                              |    2 +-
 common/usb_storage.c                          |    2 +-
 drivers/usb/eth/usb_ether.c                   |    2 +-
 drivers/usb/gadget/Makefile                   |    1 +
 drivers/usb/gadget/ether.c                    |   69 +--
 drivers/usb/gadget/pxa25x_udc.c               | 2059 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/pxa25x_udc.h               |  162 ++++++
 drivers/usb/host/ehci-armada100.c             |   15 +-
 drivers/usb/host/ehci-atmel.c                 |   11 +-
 drivers/usb/host/ehci-core.h                  |   29 --
 drivers/usb/host/ehci-exynos.c                |   15 +-
 drivers/usb/host/ehci-fsl.c                   |   15 +-
 drivers/usb/host/ehci-hcd.c                   |  126 ++---
 drivers/usb/host/ehci-ixp4xx.c                |   15 +-
 drivers/usb/host/ehci-marvell.c               |   15 +-
 drivers/usb/host/ehci-mpc512x.c               |   25 +-
 drivers/usb/host/ehci-mx5.c                   |   11 +-
 drivers/usb/host/ehci-mx6.c                   |   11 +-
 drivers/usb/host/ehci-mxc.c                   |   11 +-
 drivers/usb/host/ehci-mxs.c                   |   20 +-
 drivers/usb/host/ehci-omap.c                  |   10 +-
 drivers/usb/host/ehci-pci.c                   |   15 +-
 drivers/usb/host/ehci-ppc4xx.c                |   11 +-
 drivers/usb/host/ehci-tegra.c                 |   14 +-
 drivers/usb/host/ehci-vct.c                   |    9 +-
 drivers/usb/host/ehci.h                       |    4 +-
 drivers/usb/host/isp116x-hcd.c                |    4 +-
 drivers/usb/host/ohci-hcd.c                   |    4 +-
 drivers/usb/host/r8a66597-hcd.c               |    4 +-
 drivers/usb/host/sl811-hcd.c                  |    4 +-
 drivers/usb/musb/musb_hcd.c                   |    4 +-
 drivers/usb/ulpi/ulpi.c                       |   32 +-
 include/usb.h                                 |   21 +-
 include/usb/mv_udc.h                          |    2 +-
 include/usb/ulpi.h                            |   13 +-
 53 files changed, 2779 insertions(+), 382 deletions(-)
 create mode 100644 drivers/usb/gadget/pxa25x_udc.c
 create mode 100644 drivers/usb/gadget/pxa25x_udc.h
 delete mode 100644 drivers/usb/host/ehci-core.h

Comments

Tom Rini Oct. 9, 2012, 2:23 p.m. UTC | #1
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)
> 
> ----------------------------------------------------------------
> Lucas Stach (7):
>       usb: lowlevel interface change to support multiple controllers
>       usb: ehci: rework to take advantage of new lowlevel interface
>       usb: add support for multiple usb controllers
>       tegra20: port to new ehci interface
>       usb: ehci: don't print debug output
>       usb: ulpi: add indicator configuration function
>       tegra20: add USB ULPI init code
> 
> Lukasz Dalek (5):
>       usbether: Fixed bug when using with PXA25X chips
>       usbether: Define CONFIG_USB_ETH_{CDC,SUBSET}
>       usbether: Removed DEV_CONFIG_{CDC,SUBSET}
>       pxa25x: Add support for USB ethernet gadget
>       usb.h: Add udc_disconnect prototype to usb.h
> 
>  README                                        |    3 +
>  arch/arm/cpu/arm920t/s3c24x0/usb_ohci.c       |    4 +-
>  arch/arm/cpu/armv7/tegra20/usb.c              |  168 ++++--
>  arch/arm/include/asm/arch-tegra20/usb.h       |   33 +-
>  arch/arm/include/asm/ehci-omap.h              |   10 +-
>  arch/mips/cpu/mips32/au1x00/au1x00_usb_ohci.c |    4 +-
>  arch/powerpc/cpu/mpc5xxx/usb_ohci.c           |    4 +-
>  arch/powerpc/cpu/ppc4xx/usb_ohci.c            |    4 +-
>  arch/sparc/cpu/leon3/usb_uhci.c               |    4 +-
>  arch/sparc/lib/bootm.c                        |    2 +-
>  board/htkw/mcx/mcx.c                          |    6 +-
>  board/mpl/common/usb_uhci.c                   |    4 +-
>  board/technexion/twister/twister.c            |    6 +-
>  board/teejet/mt_ventoux/mt_ventoux.c          |    6 +-
>  board/ti/beagle/beagle.c                      |    6 +-
>  board/ti/panda/panda.c                        |    6 +-
>  common/cmd_usb.c                              |   16 +-
>  common/usb.c                                  |  108 ++--
>  common/usb_hub.c                              |    2 +-
>  common/usb_storage.c                          |    2 +-
>  drivers/usb/eth/usb_ether.c                   |    2 +-
>  drivers/usb/gadget/Makefile                   |    1 +
>  drivers/usb/gadget/ether.c                    |   69 +--
>  drivers/usb/gadget/pxa25x_udc.c               | 2059 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/gadget/pxa25x_udc.h               |  162 ++++++
>  drivers/usb/host/ehci-armada100.c             |   15 +-
>  drivers/usb/host/ehci-atmel.c                 |   11 +-
>  drivers/usb/host/ehci-core.h                  |   29 --
>  drivers/usb/host/ehci-exynos.c                |   15 +-
>  drivers/usb/host/ehci-fsl.c                   |   15 +-
>  drivers/usb/host/ehci-hcd.c                   |  126 ++---
>  drivers/usb/host/ehci-ixp4xx.c                |   15 +-
>  drivers/usb/host/ehci-marvell.c               |   15 +-
>  drivers/usb/host/ehci-mpc512x.c               |   25 +-
>  drivers/usb/host/ehci-mx5.c                   |   11 +-
>  drivers/usb/host/ehci-mx6.c                   |   11 +-
>  drivers/usb/host/ehci-mxc.c                   |   11 +-
>  drivers/usb/host/ehci-mxs.c                   |   20 +-
>  drivers/usb/host/ehci-omap.c                  |   10 +-
>  drivers/usb/host/ehci-pci.c                   |   15 +-
>  drivers/usb/host/ehci-ppc4xx.c                |   11 +-
>  drivers/usb/host/ehci-tegra.c                 |   14 +-
>  drivers/usb/host/ehci-vct.c                   |    9 +-
>  drivers/usb/host/ehci.h                       |    4 +-
>  drivers/usb/host/isp116x-hcd.c                |    4 +-
>  drivers/usb/host/ohci-hcd.c                   |    4 +-
>  drivers/usb/host/r8a66597-hcd.c               |    4 +-
>  drivers/usb/host/sl811-hcd.c                  |    4 +-
>  drivers/usb/musb/musb_hcd.c                   |    4 +-
>  drivers/usb/ulpi/ulpi.c                       |   32 +-
>  include/usb.h                                 |   21 +-
>  include/usb/mv_udc.h                          |    2 +-
>  include/usb/ulpi.h                            |   13 +-
>  53 files changed, 2779 insertions(+), 382 deletions(-)
>  create mode 100644 drivers/usb/gadget/pxa25x_udc.c
>  create mode 100644 drivers/usb/gadget/pxa25x_udc.h
>  delete mode 100644 drivers/usb/host/ehci-core.h

I had to rebase this locally to merge (such is next), and now it's
applied to u-boot/next, thanks!
Stephen Warren Oct. 9, 2012, 9:03 p.m. UTC | #2
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.
Tom Rini Oct. 9, 2012, 9:32 p.m. UTC | #3
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:
> 
> 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.

Interesting.  As this is more work on the custodians end, what does
everyone else say?
Stephen Warren Oct. 9, 2012, 10:14 p.m. UTC | #4
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:
>> 
>> 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.
> 
> 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.
Albert ARIBAUD Oct. 9, 2012, 10:19 p.m. UTC | #5
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:
> > 
> > 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.
> 
> 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,
Albert ARIBAUD Oct. 9, 2012, 10:43 p.m. UTC | #6
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. :)

But I suspect the Wiki page is still lagging behind.

Amicalement,
Tom Rini Oct. 9, 2012, 10:59 p.m. UTC | #7
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.
Graeme Russ Oct. 9, 2012, 11:02 p.m. UTC | #8
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
Stephen Warren Oct. 9, 2012, 11:04 p.m. UTC | #9
On 10/09/2012 04:19 PM, Albert ARIBAUD wrote:
> 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:
>>>
>>> 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.
>>
>> 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.

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

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

> 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?

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?

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:

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.

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

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.
Stephen Warren Oct. 9, 2012, 11:07 p.m. UTC | #10
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?
Graeme Russ Oct. 9, 2012, 11:17 p.m. UTC | #11
Hi Tom,

On Wed, Oct 10, 2012 at 9:59 AM, Tom Rini <trini@ti.com> 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.

I'm no git guru, so this may not be sane, but here goes:

u-boot-master and $(repo)-master are parallel branches - $(repo)
maintainer merges u-boot-master regularly and applies $(repo) specific
patches to $(repo)-master during the merge window (and critical fixes
during the -rc cycle). Tom merges the various $(repo)-master's into
u-boot-master regularly.

u-boot-next and $(repo)-next are also parallel branches. $(repo)
maintainer merges u-boot-master and $(repo)-master regularly and
applies $(repo) specific patches to $(repo)-master during the -rc
cycle. Tom merges the various $(repo)-next's into u-boot-next
regularly.

Immediately following a release, Tom merges u-boot-next into u-boot-master

Yes, we get lots of merge commits, but the strategy is very straightforward...

One thing I don't know how to resolve is what happens when a
$(repo)-{master, next} does not merge cleanly into u-boot-{master,
next}...

Regards,

Graeme
Albert ARIBAUD Oct. 10, 2012, 6:15 a.m. UTC | #12
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,
Stephen Warren Oct. 10, 2012, 4:04 p.m. UTC | #13
On 10/10/2012 12:15 AM, Albert ARIBAUD wrote:
> 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.

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

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

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

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

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 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.
Albert ARIBAUD Oct. 10, 2012, 6:40 p.m. UTC | #14
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,
Wolfgang Denk Oct. 11, 2012, 7:17 a.m. UTC | #15
Dear Stephen Warren,

In message <50759C8C.3030509@wwwdotorg.org> you wrote:
>
> The documentation of merge commits seems good to me just in an of itself.

Linus has commented a lot about this for Linux in the past. You may
want to dig this out from the archives.

In general, we should always fast-forward if possible.

Best regards,

Wolfgang Denk
Tom Rini Oct. 11, 2012, 4:38 p.m. UTC | #16
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.
Albert ARIBAUD Oct. 11, 2012, 5:16 p.m. UTC | #17
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,
Stephen Warren Oct. 11, 2012, 5:26 p.m. UTC | #18
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.
Albert ARIBAUD Oct. 11, 2012, 6:30 p.m. UTC | #19
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,
Albert ARIBAUD Oct. 11, 2012, 6:45 p.m. UTC | #20
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? Plus... in order for this process to
be put in place, a process should be defined for discussing this
process... argh. :)


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

Agreed -- see Stephen's reply rightly correcting me re Linux.

Amicalement,
Scott Wood Oct. 11, 2012, 6:59 p.m. UTC | #21
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
Albert ARIBAUD Oct. 12, 2012, 10:11 a.m. UTC | #22
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.

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

> -Scott

Amicalement,
Wolfgang Denk Oct. 13, 2012, 7:17 p.m. UTC | #23
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
Wolfgang Denk Oct. 13, 2012, 7:30 p.m. UTC | #24
Dear Albert ARIBAUD,

In message <20121011203003.02f27b2d@lilith> you wrote:
> 
> > > 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".

Why do you think you misread anything?

The sentence above explains what the SoB _means_. To me it does not
include _any_ information about who should add such a line. _This_ is
explained in other parts of the document, i. e. the "you wrote it or
otherwise have the right to pass it on as an open-source patch".

I have strong reason to believe (by remember, IANAL) that the "or" in
this statement is intended to cover the cases where the original
author(s) cannot or shal not be disclosed for some reason (for
example, because company FOO does not want to disclose who or how many
people exactly are working on a specific task, and/or where these are
located).  In this case it simply means:  yes, I guarantee you this
can be used without restriction as an open-source patch even though I
cannot/will not tell you who actually is/are the author(s).


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.

Best regards,

Wolfgang Denk
Tom Rini Oct. 13, 2012, 9:13 p.m. UTC | #25
-----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 SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQedleAAoJENk4IS6UOR1WuDsP/RO40omO2g/1QBUhwr0rddrS
KocAiHI4txaRxMKaHEn/cISyXpgHBnSMVen9L9iWOQo8HYVfHuusRM500nEYdsZy
BBSgiY0IUSkmINHrztc4WxF6sPadJQcftFeE1Rxi4p5uNNCn/luNyPAK5Hi3N1Kw
ZjmayqU8lmcjh3nt1jpzTtzkOuLmx+HwhKo7V5CuPklbvAidt2CsRI7cUHFUgcx2
Gw//6zQPGbW/Z5NcimuyyvwYa5csz2/lZJdyb4tTGgbTHDwrwmKbbRa6EUA4bEIb
saUqgff4jCY2/NgYuwc6zvz/aBgD3n165libtBTMBZfq3+RNbxzd4SDmsp1tTy24
0ZP40WAL2+0++mW/QptYJU4v7FcnEXjPVJJnxAcxXu+qZSKoYzVf1B+mel7rd1PB
YLLGjxbsH9vN+BWqkjyc+OwhubiLGtdGVq+21JCwR2VZKM311UWXW1MVZ7K3wiyH
5iYhyRPWHUPG9xj4ObVRrOJMtqPqnvc3Qq/u1e/ukzrCvCb2iv1cEVqOJlapCNWU
2wxPCimO9w9zHESBJVPFPqKQCrOAGRta4ouMMx2D1Hti4XYhBxVgB9x/nQ9w+QcS
f0OkEu6dZKtHEpLY+tI+DDiJfvxBGXA/xzrEdmRKQ3WoxV/luu3cAga9uo/BhyyM
HNAILD2siVXVutUPZZYk
=ODaw
-----END PGP SIGNATURE-----
Wolfgang Denk Oct. 13, 2012, 10:25 p.m. UTC | #26
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?

Best regards,

Wolfgang Denk
Stephen Warren Oct. 15, 2012, 4:32 p.m. UTC | #27
On 10/13/2012 01:17 PM, Wolfgang Denk wrote:
> 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.

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

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.
Tom Rini Oct. 15, 2012, 5:56 p.m. UTC | #28
-----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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQfE43AAoJENk4IS6UOR1WKEUP/i2Ez8T+zv872jq6hjuhGhiE
xCdrc2+npXHx/DguOkHIQqFRPQwlKaAbaGgNLXFWdIHipUcuZlUI1sraLDiQJ0un
fToNRqlts0N/nXgwOMMBn13/aihSiGrOy6MvYB0RhFLZ5FVBXxdY3QXc6rOfFrom
45A+60T4VUmghyuoa3I5Oc+H9PEyvPG6BgVFm5JtwB6oPi7KypNOx0pSnv5z7uJ8
JkiLeWDlXag4VJyXFLbf2xOQRFgbFX8EgQcRXOWDjXet1lzXjP5sA9qVnYFMVFHJ
AANtj9XFpQft6CK1Wfyq2+9fVNoqQtdqmvjhNbuqJK1vehZrpL4//tO4O++eynnP
NlYxtSUDFLeWC/qyksdda02V5Gwme7pA3aMGUFU+VBgPrzAV2aCaseiOxKND//ni
MSX+KkBUHy0l8kV7TnwuZtIV+fEvvOkYLD3KMzYxa98h7hbMpUIEa8Ldhkjyw3qx
GrzEQ59xHV6stE2/nw++J30rzlxMrnavUU6ato25GcpSDIH2yPzDargrIBtDSteP
tHFLfsj8buHx8csylaecHc+qlICA8JshbMcYkYQpzOIKYI+6M6sJOWFeXY9xrIdg
b3zjZeJk6MRhU7cdv+q1JfPxmgXKrJ/51taimYJWH1b3saZXS4fEHhshPvbhZ5Sq
5PFd5XVPr+IkrvBKSfFT
=5YMo
-----END PGP SIGNATURE-----
Wolfgang Denk Oct. 15, 2012, 6:55 p.m. UTC | #29
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
Wolfgang Denk Oct. 15, 2012, 7 p.m. UTC | #30
Dear Tom Rini,

In message <507C4E37.8000000@ti.com> you wrote:
>
> 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?

I'm fine with that.

We could (should?) also ask the Linux PTBs about their opinion on this
(and the observed inconsistency).

Best regards,

Wolfgang Denk
Stephen Warren Oct. 15, 2012, 9:42 p.m. UTC | #31
On 10/15/2012 12:55 PM, Wolfgang Denk wrote:
> 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've certainly seen people give feedback on patches that the appropriate
S-o-b lines aren't present. I don't recall if I've explicitly seen
anyone called out for not doing this during a merge (which most likely
means there was never an issue, not that people weren't checking), so I
can't say for 100% certain that everyone is doing this, but they
certainly should be.

git log is probably what I would use to validate this.