[RFC] Handling of reviewed patch series

[RFC] Handling of reviewed patch series

[Dave Chinner]

Hi folks,

I'd like to make a proposal and see what people think abou tit for
managing patch series that I have been reviewed.

What I'd like to do is publish the reviewed and signed-off topic
branches that I built as in my local git tree as part of the review
and test process. For example, I currently have two such branches
right now - one for Christoph's log format changes, and another for
Jeff's icluster cleanups.

That is:

$ git shortlog xfs-oss/master..guilt/tip-log-format
Christoph Hellwig (10):
      xfs: remove duplicate code in xlog_cil_insert_format_items
      xfs: refactor xfs_buf_item_format_segment
      xfs: refactor xfs_inode_item_size
      xfs: refactor xfs_inode_item_format
      xfs: introduce xlog_copy_iovec
      xfs: format log items write directly into the linear CIL buffer
      xfs: format logged extents directly into the CIL
      xfs: remove the inode log format from the inode log item
      xfs: remove the dquot log format from the dquot log item
      xfs: remove the quotaoff log format from the quotaoff log item

$ git shortlog xfs-oss/master..guilt/tip-icluster-factor 
Jie Liu (8):
      xfs: get rid of XFS_IALLOC_INODES macros
      xfs: get rid of XFS_INODE_CLUSTER_SIZE macros
      xfs: get rid of XFS_IALLOC_BLOCKS macros
      xfs: introduce a common helper xfs_icluster_size_fsb
      xfs: use xfs_icluster_size_fsb in xfs_bulkstat
      xfs: use xfs_icluster_size_fsb in xfs_ialloc_inode_init
      xfs: use xfs_icluster_size_fsb in xfs_ifree_cluster
      xfs: use xfs_icluster_size_fsb in xfs_imap

These branches are all based on the master branch as it currently
stands. They have had each patch individually compile tested, they
have been individually tested, and they've been merged into a test
tree and tested along with every other branch I'm currently working
on.

What I'd like to do is publish each of the reviewed branch in the
XFS git repos on oss.sgi.com as completed and ready-to-go code. This
is a bit different to what we do now - normally Ben would come along
and merge the series straight into the master branch, then update
the for-next branch to point at it.

The issue with this is that it then makes our lives more difficult
when we get a bug fix that has to go into the master branch and be
sent to linus for an -rc kernel before all the development code that
is already checked into the master branch.

What I propose is that the xfs-oss/master branch tracks the Linus
-rc1 releases, and we never check code directly into the master
branch except in exceptional circumstances. i.e.  we try to only
ever pull back down from Linus into it. Exceptional circumstances
would be work that causes widespread rebasing, like all of the
structural rework that we've done recently. The structural work
would go into the master branch immediately after a -rc1 update, and
the next cycle's work and topic branches then based on top of that.

What this means is that the "for-next" branch is no longer based
on the master branch - it becomes the development branch we work on,
and is effectively a merge of all the topic branches. i.e like the
-next tree, it is a branch that can be rebased without impacting the
history of the code in the topic branches because it's just a merge
target.

What this means is that development can be done against the master
branch without fear of conflicting with other changes that are being
done. Testing, however, can target the for-next branch, and local
integration testing can be done simply by merging a local topic
branch into a local for-next branch....

Lines of development that overlap will generate conflicts at
the for-next branch merge, and at that point we can decide how to
deal with the problem. e.g. turn the conflicting topic branches into
a single, larger topic branch, live with it, etc.

When it comes to sending code upstream to Linus, we can either send
a pull request per topic branch - Linus often likes to do merges
himself - or we can merge them all into a single branch and ask
Linus to pull that. The deciding factor may well be Linus himself...

However, what this structure means is that urgent bugs fixes canbe
placed into their own topic branches - an "urgent-*" prefix is often
used for these by other maintainers - and Linus can pull directly
from the topic branch without us having to worry about
cherry-picking out of the middle of the master branch and all the
pain that entails.

It also allows us much more flexibility in managing the
code and how it is send upstream. It also makes it possible to push
code earlier for wider testing without being stuck with that code
forever - it's trivial to drop a topic branch if it causes
unexpected regressions or problems without leaving nasty reverts all
over the history.

Yes, this is a bit of a change in the way we do things, but it
aligns much more closely to the distributed nature of how we develop
the code. I'm going to push the two topic branches I mentioned above
into the oss repository so people can have a look at them and get a
feel for how such a process might work and so we can reference them
during the discussion.

Essentially, I want to speed up the rate at which we get code
integrated without the risk of making a big mess by committing to
code too quickly. Keeping code in topic branches like this solves
that problem, and the fact that it closely aligns to my normal
workflow makes it very appealing to me as a Maintainer....

Keep in mind that I want to do the same thing for major pieces of
work with xfstests and xfsprogs - keep work in topic branches until
they are tested and ready to go, then merge everything into the master
branch and cut a release. This would enable a much faster xfsprogs
release schedule because we make a release at any point in time
without having to worry about whether we have work in progress in
the master branch that needs completing before a release is done...

Anyway, have a think and discuss - I'm going to push the branches I
mentioned above....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Christoph Hellwig]

I'm a big fan of the for-next vs for-linux split for next release work
vs bugfixes which we've not applied yet.  The whole topic branches
scheme makes sense for large changes like the crc work, but seems
utterly confusing if applied to every little change, as now the amount
of branches you can conflict againt multiplies.  I'm defintively in
favour of a model that has less active branches.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Dave Chinner]

On Fri, Dec 13, 2013 at 03:19:59AM -0800, Christoph Hellwig wrote:
> 
> I'm a big fan of the for-next vs for-linux split for next release work
> vs bugfixes which we've not applied yet.  The whole topic branches
> scheme makes sense for large changes like the crc work, but seems
> utterly confusing if applied to every little change, as now the amount
> of branches you can conflict againt multiplies.  I'm defintively in
> favour of a model that has less active branches.

I certainly wouldn't want to create topic branches for every
standalone patch - that doesn't make any sense from a management
overhead point of view. I'd keep a "miscellaneous" topic branch
specifically for aggregating standalone and small fixes, and that
keeps the number of topic branches under control.

The way I see it from a developer POV is that after the topic branch
is created you can check that it matches your local changes, then
just ignore it. You continue to work from the for-next branch (which
now includes your work from the topic branch), or continue to target
the unchanging master branch or the for-next merge target branch,
which would be the same as what you work from now.

In the case that you have work that is dependent on a specific topic
branch, we can add the work to the end of that topic branch rather
than create a new one. Or if you have dependencies across
everything, then you develop against for-next and we simply make
that the last topic branch to be merged into for-next.

i.e. as a developer, you really don't need to care that much about
individual topic branches and how they are managed....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Brian Foster]

On 12/13/2013 12:36 AM, Dave Chinner wrote:
> Hi folks,
> 
...
> What I'd like to do is publish each of the reviewed branch in the
> XFS git repos on oss.sgi.com as completed and ready-to-go code. This
> is a bit different to what we do now - normally Ben would come along
> and merge the series straight into the master branch, then update
> the for-next branch to point at it.
> 
> The issue with this is that it then makes our lives more difficult
> when we get a bug fix that has to go into the master branch and be
> sent to linus for an -rc kernel before all the development code that
> is already checked into the master branch.
> 
> What I propose is that the xfs-oss/master branch tracks the Linus
> -rc1 releases, and we never check code directly into the master
> branch except in exceptional circumstances. i.e.  we try to only
> ever pull back down from Linus into it. Exceptional circumstances
> would be work that causes widespread rebasing, like all of the
> structural rework that we've done recently. The structural work
> would go into the master branch immediately after a -rc1 update, and
> the next cycle's work and topic branches then based on top of that.
> 
> What this means is that the "for-next" branch is no longer based
> on the master branch - it becomes the development branch we work on,
> and is effectively a merge of all the topic branches. i.e like the
> -next tree, it is a branch that can be rebased without impacting the
> history of the code in the topic branches because it's just a merge
> target.
> 

Is the merge of a topic branch into the for-next branch a maintainer
duty formality here, or another "phase" we're introducing into the
development process? IOW, a developer submits patches and said patches
are reviewed and merged into a topic branch. What/when/how is said topic
branch merged into the for-next for impending upstream merge?

I don't see the existence of such branches as a big issue. In fact, it
would be nice to see a sort of a high/feature level index of what kind
of things are outstanding for a release, or to be able to narrow down a
regression into a particular set, etc. A more granular workflow doesn't
necessarily hurt if the maintainer(s) see it as worth the extra steps.

On the flipside, topic branches alone make it harder for a
non-maintainer to actually test the whole of a release. Is it up to the
developer to merge all of the topic branches to actually test everything
and actively maintain that relationship as things change on the
maintainer end? Or are these patches pulled into a topic branch and
merged onto for-next at the same time?

Brian

> What this means is that development can be done against the master
> branch without fear of conflicting with other changes that are being
> done. Testing, however, can target the for-next branch, and local
> integration testing can be done simply by merging a local topic
> branch into a local for-next branch....
> 
> Lines of development that overlap will generate conflicts at
> the for-next branch merge, and at that point we can decide how to
> deal with the problem. e.g. turn the conflicting topic branches into
> a single, larger topic branch, live with it, etc.
> 
> When it comes to sending code upstream to Linus, we can either send
> a pull request per topic branch - Linus often likes to do merges
> himself - or we can merge them all into a single branch and ask
> Linus to pull that. The deciding factor may well be Linus himself...
> 
> However, what this structure means is that urgent bugs fixes canbe
> placed into their own topic branches - an "urgent-*" prefix is often
> used for these by other maintainers - and Linus can pull directly
> from the topic branch without us having to worry about
> cherry-picking out of the middle of the master branch and all the
> pain that entails.
> 
> It also allows us much more flexibility in managing the
> code and how it is send upstream. It also makes it possible to push
> code earlier for wider testing without being stuck with that code
> forever - it's trivial to drop a topic branch if it causes
> unexpected regressions or problems without leaving nasty reverts all
> over the history.
> 
> Yes, this is a bit of a change in the way we do things, but it
> aligns much more closely to the distributed nature of how we develop
> the code. I'm going to push the two topic branches I mentioned above
> into the oss repository so people can have a look at them and get a
> feel for how such a process might work and so we can reference them
> during the discussion.
> 
> Essentially, I want to speed up the rate at which we get code
> integrated without the risk of making a big mess by committing to
> code too quickly. Keeping code in topic branches like this solves
> that problem, and the fact that it closely aligns to my normal
> workflow makes it very appealing to me as a Maintainer....
> 
> Keep in mind that I want to do the same thing for major pieces of
> work with xfstests and xfsprogs - keep work in topic branches until
> they are tested and ready to go, then merge everything into the master
> branch and cut a release. This would enable a much faster xfsprogs
> release schedule because we make a release at any point in time
> without having to worry about whether we have work in progress in
> the master branch that needs completing before a release is done...
> 
> Anyway, have a think and discuss - I'm going to push the branches I
> mentioned above....
> 
> Cheers,
> 
> Dave.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Ben Myers]

Hey Dave,

On Fri, Dec 13, 2013 at 04:36:11PM +1100, Dave Chinner wrote:
> I'd like to make a proposal and see what people think abou tit for
> managing patch series that I have been reviewed.
> 
> What I'd like to do is publish the reviewed and signed-off topic
> branches that I built as in my local git tree as part of the review
> and test process. For example, I currently have two such branches
> right now - one for Christoph's log format changes, and another for
> Jeff's icluster cleanups.
> 
> That is:
> 
> $ git shortlog xfs-oss/master..guilt/tip-log-format
> Christoph Hellwig (10):
>       xfs: remove duplicate code in xlog_cil_insert_format_items
>       xfs: refactor xfs_buf_item_format_segment
>       xfs: refactor xfs_inode_item_size
>       xfs: refactor xfs_inode_item_format
>       xfs: introduce xlog_copy_iovec
>       xfs: format log items write directly into the linear CIL buffer
>       xfs: format logged extents directly into the CIL
>       xfs: remove the inode log format from the inode log item
>       xfs: remove the dquot log format from the dquot log item
>       xfs: remove the quotaoff log format from the quotaoff log item
> 
> $ git shortlog xfs-oss/master..guilt/tip-icluster-factor 
> Jie Liu (8):
>       xfs: get rid of XFS_IALLOC_INODES macros
>       xfs: get rid of XFS_INODE_CLUSTER_SIZE macros
>       xfs: get rid of XFS_IALLOC_BLOCKS macros
>       xfs: introduce a common helper xfs_icluster_size_fsb
>       xfs: use xfs_icluster_size_fsb in xfs_bulkstat
>       xfs: use xfs_icluster_size_fsb in xfs_ialloc_inode_init
>       xfs: use xfs_icluster_size_fsb in xfs_ifree_cluster
>       xfs: use xfs_icluster_size_fsb in xfs_imap
> 
> These branches are all based on the master branch as it currently
> stands. They have had each patch individually compile tested, they
> have been individually tested, and they've been merged into a test
> tree and tested along with every other branch I'm currently working
> on.
> 
> What I'd like to do is publish each of the reviewed branch in the
> XFS git repos on oss.sgi.com as completed and ready-to-go code. This
> is a bit different to what we do now - normally Ben would come along
> and merge the series straight into the master branch, then update
> the for-next branch to point at it.

Yes, normally I'd merge those series right about now.  ;)

> The issue with this is that it then makes our lives more difficult
> when we get a bug fix that has to go into the master branch and be
> sent to linus for an -rc kernel before all the development code that
> is already checked into the master branch.

Yeah, that's the same issue we have during the merge window.  You can't pull
stuff into the master branch until after Linus has pulled and you've pulled
back if you want a fast-forward merge the next time around.  If you do pull
stuff in during the window you get a merge commit that screws up the history
graph.  The workaround in later -rc requests is to cherry-pick (AKA rebase) the
bugfixes back onto the latest commit that Linus has pulled, and then there are
duplicate commits that we pull back from mainline after the next merge window
closes.  It's not the prettiest thing, but better than screwing up the graph.

> What I propose is that the xfs-oss/master branch tracks the Linus
> -rc1 releases, and we never check code directly into the master
> branch except in exceptional circumstances. i.e.  we try to only
> ever pull back down from Linus into it. Exceptional circumstances
> would be work that causes widespread rebasing, like all of the
> structural rework that we've done recently. The structural work
> would go into the master branch immediately after a -rc1 update, and
> the next cycle's work and topic branches then based on top of that.
> 
> What this means is that the "for-next" branch is no longer based
> on the master branch - it becomes the development branch we work on,
> and is effectively a merge of all the topic branches. i.e like the
> -next tree, it is a branch that can be rebased without impacting the
> history of the code in the topic branches because it's just a merge
> target.
> 
> What this means is that development can be done against the master
> branch without fear of conflicting with other changes that are being
> done. Testing, however, can target the for-next branch, and local
> integration testing can be done simply by merging a local topic
> branch into a local for-next branch....

I'm not too keen on rebasing a published branch, mostly because I tend to log
test results by commit id.  If there is a way to keep the initial commit id
stable and in the repo so it can be referenced later it would be better.  e.g.
In the [unlikely] event that the for-next branch does need to be rebased, tag
it first.

> Lines of development that overlap will generate conflicts at
> the for-next branch merge, and at that point we can decide how to
> deal with the problem. e.g. turn the conflicting topic branches into
> a single, larger topic branch, live with it, etc.
> 
> When it comes to sending code upstream to Linus, we can either send
> a pull request per topic branch - Linus often likes to do merges
> himself - or we can merge them all into a single branch and ask
> Linus to pull that. The deciding factor may well be Linus himself...

If you take a look at merges into mainline using gitweb they look like this:

Merge branch 'for-linus' of git://git./linux/kernel... 

I suggest that the topic branches start with xfs.

e.g. 'xfs-refactor-icluster-macros' would be better than 'tip-icluster-factor'.

> However, what this structure means is that urgent bugs fixes canbe
> placed into their own topic branches - an "urgent-*" prefix is often
> used for these by other maintainers - and Linus can pull directly
> from the topic branch without us having to worry about
> cherry-picking out of the middle of the master branch and all the
> pain that entails.

Cherry-picking is not all that painful, but a a 'misc' branch, or 'urgent'
seems fine to me too.  I do prefer an xfs- prefix and to send signed tags in
pull requests...
 
> It also allows us much more flexibility in managing the
> code and how it is send upstream. It also makes it possible to push
> code earlier for wider testing without being stuck with that code
> forever - it's trivial to drop a topic branch if it causes
> unexpected regressions or problems without leaving nasty reverts all
> over the history.
> 
> Yes, this is a bit of a change in the way we do things, but it
> aligns much more closely to the distributed nature of how we develop
> the code. I'm going to push the two topic branches I mentioned above
> into the oss repository so people can have a look at them and get a
> feel for how such a process might work and so we can reference them
> during the discussion.

Lets see if I understand correctly (and I'll take some of my own advice from
above).  I'll pull 'tip-icluster-factor' into a local branch named
'xfs-refactor-icluster-macros', and merge it into our for-next branch along
with the other series.  This will get them into -next, and we can still toss it
later if it's not what you had in mind.

> Essentially, I want to speed up the rate at which we get code
> integrated without the risk of making a big mess by committing to
> code too quickly. Keeping code in topic branches like this solves
> that problem, and the fact that it closely aligns to my normal
> workflow makes it very appealing to me as a Maintainer....
> 
> Keep in mind that I want to do the same thing for major pieces of
> work with xfstests and xfsprogs - keep work in topic branches until
> they are tested and ready to go, then merge everything into the master
> branch and cut a release. This would enable a much faster xfsprogs
> release schedule because we make a release at any point in time
> without having to worry about whether we have work in progress in
> the master branch that needs completing before a release is done...

Yeah, that's interesting in light of Christoph's suggestion for a 3.1.12
release.  Probably we should try this on the kernel repo for awhile to see how it
goes.

> Anyway, have a think and discuss - I'm going to push the branches I
> mentioned above....

I've been tracking message id and patchwork id in git notes along with commits
for awhile.  I'm hoping this will become useful later for cross referencing the
list, patchworks, and test results.  If you wouldn't mind also doing so I'd
appreciate it.  Maybe it could be done with a post-commit script or something.

Regards,
Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Dave Chinner]

On Fri, Dec 13, 2013 at 08:42:25AM -0500, Brian Foster wrote:
> On 12/13/2013 12:36 AM, Dave Chinner wrote:
> > Hi folks,
> > 
> ...
> > What I'd like to do is publish each of the reviewed branch in the
> > XFS git repos on oss.sgi.com as completed and ready-to-go code. This
> > is a bit different to what we do now - normally Ben would come along
> > and merge the series straight into the master branch, then update
> > the for-next branch to point at it.
> > 
> > The issue with this is that it then makes our lives more difficult
> > when we get a bug fix that has to go into the master branch and be
> > sent to linus for an -rc kernel before all the development code that
> > is already checked into the master branch.
> > 
> > What I propose is that the xfs-oss/master branch tracks the Linus
> > -rc1 releases, and we never check code directly into the master
> > branch except in exceptional circumstances. i.e.  we try to only
> > ever pull back down from Linus into it. Exceptional circumstances
> > would be work that causes widespread rebasing, like all of the
> > structural rework that we've done recently. The structural work
> > would go into the master branch immediately after a -rc1 update, and
> > the next cycle's work and topic branches then based on top of that.
> > 
> > What this means is that the "for-next" branch is no longer based
> > on the master branch - it becomes the development branch we work on,
> > and is effectively a merge of all the topic branches. i.e like the
> > -next tree, it is a branch that can be rebased without impacting the
> > history of the code in the topic branches because it's just a merge
> > target.
> > 
> 
> Is the merge of a topic branch into the for-next branch a maintainer
> duty formality here, or another "phase" we're introducing into the
> development process? IOW, a developer submits patches and said patches
> are reviewed and merged into a topic branch. What/when/how is said topic
> branch merged into the for-next for impending upstream merge?

It's purely a maintainer thing. Instead of committing the reviewed
series to the master branch, it gets pushed to a topic branch, and
then the topic branch is merged into the for-next branch soon after.

So, really, you don't even need to know that the topic branch exists
;)

> I don't see the existence of such branches as a big issue. In fact, it
> would be nice to see a sort of a high/feature level index of what kind
> of things are outstanding for a release, or to be able to narrow down a
> regression into a particular set, etc. A more granular workflow doesn't
> necessarily hurt if the maintainer(s) see it as worth the extra steps.

Quite a few maintainers use this workflow as it means we don't have
to fix the order of the code that is to be send to Linus weeks
before the merge window. The code is in the repository and so when
it was committed is recorded and that is maintained across merges.
Hence Linus can see that the code has been in the repo well before
the merge window and hence been subject to -next testing.

The win from the maintainer POV is that we can send bug fixes to
Linus in the -rc window without having to cherry pick bug fix
patches out of a fixed history in the master branch - we just get
linus to pull the fix directly, and then rebase the for-next branch
by merging those bug fix topic branches first..

> On the flipside, topic branches alone make it harder for a
> non-maintainer to actually test the whole of a release. Is it up to the
> developer to merge all of the topic branches to actually test everything
> and actively maintain that relationship as things change on the
> maintainer end? Or are these patches pulled into a topic branch and
> merged onto for-next at the same time?

No, what it means is that if you want the current dev tree, you work
from the for-next branch rather than from the master branch.

Ideally, developers work against and send patches against the master
branch rather than the for-next branch, but test against the
for-next branch. To demonstrate, here's my typical workflow (the way
I use guilt to manage individual feature branchs is ommitted here):

# add XFS repo as a remote to a cloned Linus tree
$ git remote add xfs-oss git://oss.sgi.com/xfs/xfs
$ git remote update
....

# create permanent test branch
$ git checkout -b testing xfs-oss/for-next
....

# create a new feature branch
$ git checkout -b feature1 xfs-oss/master
....
$ git commit
$ <build for feature test cycle>

# Integration testing:
$ git checkout testing
$ git reset --hard xfs-oss/for-next
$ git merge feature1
$ git merge feature2
....
$ git merge featureN
$ <build and test>

# bug fixes needed
$ git checkout feature1
<do work, build, test>
$
# Integration testing, need to reset branch to remove previous state
$ git checkout testing
$ git reset --hard xfs-oss/for-next
$ git merge feature1
$ git merge feature2
....

And so the cycle goes. This is how I keep several different lines of
development separate at the same time, yet I'm easily able to test
them all together. You can also see how quickly and easily
integrating 3rd party patches from the mailing list is when they are
based on the master branch:

$ git checkout -b tmp xfs-oss/master
$ git am hch-log-format
....
$ git checkout -b tip-log-format xfs-oss/master
$ guilt init
$ guilt import-commit xfs-oss/master..tmp
$ git branch -d tmp
$ <use guilt to walk patches, build test each, add s-o-b and commit>

# now I have a complete 3rd party branch, test:

# Integration testing:
$ git checkout testing
$ git reset --hard xfs-oss/for-next
$ git merge guilt/tip-log-format
$ git merge feature1
....
$ <build, test>

3rd party patchset good, so the only extra thing I need to do now to
make it available for everyone else is this:

$ git checkout -b for-next xfs-oss/for-next
$ git merge guilt/tip-log-format
$ <build, smoke test>
$ git push .... guilt/tip-log-format:tip-log-format
$ git push .... for-next:for-next
$ guilt branch -d for-next

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Dave Chinner]

On Fri, Dec 13, 2013 at 12:56:18PM -0600, Ben Myers wrote:
> Hey Dave,
> 
> On Fri, Dec 13, 2013 at 04:36:11PM +1100, Dave Chinner wrote:
> > I'd like to make a proposal and see what people think abou tit for
> > managing patch series that I have been reviewed.
> > 
> > What I'd like to do is publish the reviewed and signed-off topic
> > branches that I built as in my local git tree as part of the review
> > and test process. For example, I currently have two such branches
> > right now - one for Christoph's log format changes, and another for
> > Jeff's icluster cleanups.
....
> > These branches are all based on the master branch as it currently
> > stands. They have had each patch individually compile tested, they
> > have been individually tested, and they've been merged into a test
> > tree and tested along with every other branch I'm currently working
> > on.
> > 
> > What I'd like to do is publish each of the reviewed branch in the
> > XFS git repos on oss.sgi.com as completed and ready-to-go code. This
> > is a bit different to what we do now - normally Ben would come along
> > and merge the series straight into the master branch, then update
> > the for-next branch to point at it.
> 
> Yes, normally I'd merge those series right about now.  ;)
> 
> > The issue with this is that it then makes our lives more difficult
> > when we get a bug fix that has to go into the master branch and be
> > sent to linus for an -rc kernel before all the development code that
> > is already checked into the master branch.
> 
> Yeah, that's the same issue we have during the merge window.  You can't pull
> stuff into the master branch until after Linus has pulled and you've pulled
> back if you want a fast-forward merge the next time around.  If you do pull
> stuff in during the window you get a merge commit that screws up the history
> graph.  The workaround in later -rc requests is to cherry-pick (AKA rebase) the
> bugfixes back onto the latest commit that Linus has pulled, and then there are
> duplicate commits that we pull back from mainline after the next merge window
> closes.  It's not the prettiest thing, but better than screwing up the graph.

Agreed, and even better is avoiding the problem of having to
cherry-pick altogether :)

> > What I propose is that the xfs-oss/master branch tracks the
> > Linus -rc1 releases, and we never check code directly into the
> > master branch except in exceptional circumstances. i.e.  we try
> > to only ever pull back down from Linus into it. Exceptional
> > circumstances would be work that causes widespread rebasing,
> > like all of the structural rework that we've done recently. The
> > structural work would go into the master branch immediately
> > after a -rc1 update, and the next cycle's work and topic
> > branches then based on top of that.
> > 
> > What this means is that the "for-next" branch is no longer based
> > on the master branch - it becomes the development branch we work
> > on, and is effectively a merge of all the topic branches. i.e
> > like the -next tree, it is a branch that can be rebased without
> > impacting the history of the code in the topic branches because
> > it's just a merge target.
> > 
> > What this means is that development can be done against the
> > master branch without fear of conflicting with other changes
> > that are being done. Testing, however, can target the for-next
> > branch, and local integration testing can be done simply by
> > merging a local topic branch into a local for-next branch....
> 
> I'm not too keen on rebasing a published branch, mostly because I
> tend to log test results by commit id.  If there is a way to keep
> the initial commit id stable and in the repo so it can be
> referenced later it would be better.  e.g.  In the [unlikely]
> event that the for-next branch does need to be rebased, tag it
> first.

Well, I'd be surprised if we have to rebase the for-next branch very
often. If we plan things correctly (e.g. delay disruptive topic
branchs to the next release, and merge them immediately after an
-rc1 update) I think we can effectively avoid rebases. The
difference is that if we ever need to do a rebase, we can.

It also means that we can commit the disruptive series to a topic
branch once it is complete, but we don't have to merge it into
for-next immediately - it can sit there and wait for the next -rc1
update to roll around. That way we can rebase the topic branch with
bug fixes, merge conflict fixes, etc right up to the point that we
"stabilise" it by merging it into for-next....

> > Lines of development that overlap will generate conflicts at the
> > for-next branch merge, and at that point we can decide how to
> > deal with the problem. e.g. turn the conflicting topic branches
> > into a single, larger topic branch, live with it, etc.
> > 
> > When it comes to sending code upstream to Linus, we can either
> > send a pull request per topic branch - Linus often likes to do
> > merges himself - or we can merge them all into a single branch
> > and ask Linus to pull that. The deciding factor may well be
> > Linus himself...
> 
> If you take a look at merges into mainline using gitweb they look
> like this:
> 
> Merge branch 'for-linus' of git://git./linux/kernel... 
> 
> I suggest that the topic branches start with xfs.
> 
> e.g. 'xfs-refactor-icluster-macros' would be better than
> 'tip-icluster-factor'.

I used the "tip" prefix simply because that is what is used in other
trees for branches of this purpose. It's good to be able to just
look at the branch and know from the prefix that it is a feature
branch pending for the next merge window, as opposed to some
development branch we are using to stage other work....

I'm open to other suggestions - having an "xfs-" prefix of
some kind is definitely a good idea. Perhaps something like
"xfs-14-..." where the 14 is an indicator of the merge window we are
queuing it for (i.e. 3.14)? That way we end up with "xfs-13-rc6-*"
as the branch prefix for bug fixes that need to be sent to linus in
the 3.13-rc6 cycle....

Any other ideas?

> > However, what this structure means is that urgent bugs fixes
> > canbe placed into their own topic branches - an "urgent-*"
> > prefix is often used for these by other maintainers - and Linus
> > can pull directly from the topic branch without us having to
> > worry about cherry-picking out of the middle of the master
> > branch and all the pain that entails.
> 
> Cherry-picking is not all that painful, but a a 'misc' branch, or
> 'urgent' seems fine to me too.  I do prefer an xfs- prefix and to
> send signed tags in pull requests...

It's more that cherry picking screws up the history once we merge
back down from Linus' tree. Remember all the duplicate commits that
resulted from sending all those bug fixes in the 3.10-rc cycle? :/

> > It also allows us much more flexibility in managing the code and
> > how it is send upstream. It also makes it possible to push code
> > earlier for wider testing without being stuck with that code
> > forever - it's trivial to drop a topic branch if it causes
> > unexpected regressions or problems without leaving nasty reverts
> > all over the history.
> > 
> > Yes, this is a bit of a change in the way we do things, but it
> > aligns much more closely to the distributed nature of how we
> > develop the code. I'm going to push the two topic branches I
> > mentioned above into the oss repository so people can have a
> > look at them and get a feel for how such a process might work
> > and so we can reference them during the discussion.
> 
> Lets see if I understand correctly (and I'll take some of my own
> advice from above).  I'll pull 'tip-icluster-factor' into a local
> branch named 'xfs-refactor-icluster-macros', and merge it into our
> for-next branch along with the other series.  This will get them
> into -next, and we can still toss it later if it's not what you
> had in mind.

Well, ideally when one of us pushes out or appends to a topic
branch, we merge it into for-next at the same time. If we need to
rebase the for-next branch, then we need to discuss it first and
take appropriate actions...

> > Essentially, I want to speed up the rate at which we get code
> > integrated without the risk of making a big mess by committing
> > to code too quickly. Keeping code in topic branches like this
> > solves that problem, and the fact that it closely aligns to my
> > normal workflow makes it very appealing to me as a
> > Maintainer....
> > 
> > Keep in mind that I want to do the same thing for major pieces
> > of work with xfstests and xfsprogs - keep work in topic branches
> > until they are tested and ready to go, then merge everything
> > into the master branch and cut a release. This would enable a
> > much faster xfsprogs release schedule because we make a release
> > at any point in time without having to worry about whether we
> > have work in progress in the master branch that needs completing
> > before a release is done...
> 
> Yeah, that's interesting in light of Christoph's suggestion for a
> 3.1.12 release.  Probably we should try this on the kernel repo
> for awhile to see how it goes.

Yup, sounds like a plan...

> > Anyway, have a think and discuss - I'm going to push the
> > branches I mentioned above....
> 
> I've been tracking message id and patchwork id in git notes along
> with commits for awhile.  I'm hoping this will become useful later
> for cross referencing the list, patchworks, and test results.  If
> you wouldn't mind also doing so I'd appreciate it.  Maybe it could
> be done with a post-commit script or something.

There's no notes in the repo of oss.sgi.com, so I'm not sure what
you are doing here.

As it is, patchworks is not something I use or want to use. I
capture and track patches with procmail and mutt - I really don't
want to have to use patchworks just to find some arbitrary ID number
that some 3rd party tool generates and add it to notes attached to a
commit.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Dave Chinner]

On Sat, Dec 14, 2013 at 10:14:01AM +1100, Dave Chinner wrote:
> On Fri, Dec 13, 2013 at 12:56:18PM -0600, Ben Myers wrote:
> > On Fri, Dec 13, 2013 at 04:36:11PM +1100, Dave Chinner wrote:
> > > like the -next tree, it is a branch that can be rebased without
> > > impacting the history of the code in the topic branches because
> > > it's just a merge target.
> > > 
> > > What this means is that development can be done against the
> > > master branch without fear of conflicting with other changes
> > > that are being done. Testing, however, can target the for-next
> > > branch, and local integration testing can be done simply by
> > > merging a local topic branch into a local for-next branch....
> > 
> > I'm not too keen on rebasing a published branch, mostly because I
> > tend to log test results by commit id.  If there is a way to keep
> > the initial commit id stable and in the repo so it can be
> > referenced later it would be better.  e.g.  In the [unlikely]
> > event that the for-next branch does need to be rebased, tag it
> > first.
> 
> Well, I'd be surprised if we have to rebase the for-next branch very
> often. If we plan things correctly (e.g. delay disruptive topic
> branchs to the next release, and merge them immediately after an
> -rc1 update) I think we can effectively avoid rebases. The
> difference is that if we ever need to do a rebase, we can.

FWIW, I just realised that this isn't a huge problem. Rebasing the
for-next branch by remerging topic branches is not going to change
the commit IDs of the commits in the topic branches - it only
changes the commit ID of the merge commits. Hence if you are
tracking commit IDs of the patches rather than the merges, a
for-next rebase won't affect your tracking at all.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Ben Myers]

Hi Dave,

On Sat, Dec 14, 2013 at 10:14:01AM +1100, Dave Chinner wrote:
> On Fri, Dec 13, 2013 at 12:56:18PM -0600, Ben Myers wrote:
> > On Fri, Dec 13, 2013 at 04:36:11PM +1100, Dave Chinner wrote:
> > > Lines of development that overlap will generate conflicts at the
> > > for-next branch merge, and at that point we can decide how to
> > > deal with the problem. e.g. turn the conflicting topic branches
> > > into a single, larger topic branch, live with it, etc.
> > > 
> > > When it comes to sending code upstream to Linus, we can either
> > > send a pull request per topic branch - Linus often likes to do
> > > merges himself - or we can merge them all into a single branch
> > > and ask Linus to pull that. The deciding factor may well be
> > > Linus himself...
> > 
> > If you take a look at merges into mainline using gitweb they look
> > like this:
> > 
> > Merge branch 'for-linus' of git://git./linux/kernel... 
> > 
> > I suggest that the topic branches start with xfs.
> > 
> > e.g. 'xfs-refactor-icluster-macros' would be better than
> > 'tip-icluster-factor'.
> 
> I used the "tip" prefix simply because that is what is used in other
> trees for branches of this purpose. It's good to be able to just
> look at the branch and know from the prefix that it is a feature
> branch pending for the next merge window, as opposed to some
> development branch we are using to stage other work....
> 
> I'm open to other suggestions - having an "xfs-" prefix of
> some kind is definitely a good idea. Perhaps something like
> "xfs-14-..." where the 14 is an indicator of the merge window we are
> queuing it for (i.e. 3.14)? That way we end up with "xfs-13-rc6-*"
> as the branch prefix for bug fixes that need to be sent to linus in
> the 3.13-rc6 cycle....
> 
> Any other ideas?

Just 'xfs-' is good enough IMO.  I think that the various topic branches don't
have to stay around forever after they have been merged.

> > Lets see if I understand correctly (and I'll take some of my own
> > advice from above).  I'll pull 'tip-icluster-factor' into a local
> > branch named 'xfs-refactor-icluster-macros', and merge it into our
> > for-next branch along with the other series.  This will get them
> > into -next, and we can still toss it later if it's not what you
> > had in mind.
> 
> Well, ideally when one of us pushes out or appends to a topic
> branch, we merge it into for-next at the same time. If we need to
> rebase the for-next branch, then we need to discuss it first and
> take appropriate actions...

Sounds fine.  As you pointed out elsewhere in this thread, we probably don't
need to rebase for-next very often.

> > > Anyway, have a think and discuss - I'm going to push the
> > > branches I mentioned above....
> > 
> > I've been tracking message id and patchwork id in git notes along
> > with commits for awhile.  I'm hoping this will become useful later
> > for cross referencing the list, patchworks, and test results.  If
> > you wouldn't mind also doing so I'd appreciate it.  Maybe it could
> > be done with a post-commit script or something.
> 
> There's no notes in the repo of oss.sgi.com, so I'm not sure what
> you are doing here.

I'm just trying to track the message id along with patchwork id.  Maybe later I
can script it up so that test results are cross referenced with the list archives.

commit c91c46c12768daac8486dff0f74bc52c2ec974cd
Author: Christoph Hellwig <hch@infradead.org>
Date:   Mon Nov 18 05:10:52 2013 -0800

    xfs: add xfs_setattr_time
    
    Split out a xfs_setattr_time helper to share code between truncate and
    regular setattr similar to xfs_setattr_mode.  I might also have another
    caller growing for this in the near future.
    
    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Brian Foster <bfoster@redhat.com>
    Signed-off-by: Ben Myers <bpm@sgi.com>

Notes:
    X-Patchwork-ID: 6941
    
    Message-Id: <20131118131052.GB21649@infradead.org>

> As it is, patchworks is not something I use or want to use. I
> capture and track patches with procmail and mutt - I really don't
> want to have to use patchworks just to find some arbitrary ID number
> that some 3rd party tool generates and add it to notes attached to a
> commit.

I'm a mutt user as well.  I'm not necessarily the biggest fan of patchwork
either, but it does turn out to be helpful sometimes.  I don't know what your
workflow is like.  Is there any chance you can get the message id in there?

messageid=$(formail -X Message-Id: < $patch | awk '{print $2}')
git notes append -m "Message-Id: $messageid"

Thanks,
Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [RFC] Handling of reviewed patch series

[Dave Chinner]

On Mon, Dec 16, 2013 at 05:39:40PM -0600, Ben Myers wrote:
> Hi Dave,
> 
> On Sat, Dec 14, 2013 at 10:14:01AM +1100, Dave Chinner wrote:
> > On Fri, Dec 13, 2013 at 12:56:18PM -0600, Ben Myers wrote:
> > > > Anyway, have a think and discuss - I'm going to push the
> > > > branches I mentioned above....
> > > 
> > > I've been tracking message id and patchwork id in git notes along
> > > with commits for awhile.  I'm hoping this will become useful later
> > > for cross referencing the list, patchworks, and test results.  If
> > > you wouldn't mind also doing so I'd appreciate it.  Maybe it could
> > > be done with a post-commit script or something.
> > 
> > There's no notes in the repo of oss.sgi.com, so I'm not sure what
> > you are doing here.
> 
> I'm just trying to track the message id along with patchwork id.  Maybe later I
> can script it up so that test results are cross referenced with the list archives.
> 
> commit c91c46c12768daac8486dff0f74bc52c2ec974cd
> Author: Christoph Hellwig <hch@infradead.org>
> Date:   Mon Nov 18 05:10:52 2013 -0800
> 
>     xfs: add xfs_setattr_time
>     
>     Split out a xfs_setattr_time helper to share code between truncate and
>     regular setattr similar to xfs_setattr_mode.  I might also have another
>     caller growing for this in the near future.
>     
>     Signed-off-by: Christoph Hellwig <hch@lst.de>
>     Reviewed-by: Brian Foster <bfoster@redhat.com>
>     Signed-off-by: Ben Myers <bpm@sgi.com>
> 
> Notes:
>     X-Patchwork-ID: 6941
>     
>     Message-Id: <20131118131052.GB21649@infradead.org>

There are no notes in the oss repository that I can find. AFAIK,
notes stay in the local repo and don't get pushed. Basically, if you
want notes to be pushed/pulled, everyone has to use a non-standard
repository config to tell git to push/pull refs/notes.

Really, if it's necessary information, it should be in the commit
message. I've never used message IDs or patchworks IDs in my life,
so you can guess how necessary I consider that information to be....

> > As it is, patchworks is not something I use or want to use. I
> > capture and track patches with procmail and mutt - I really don't
> > want to have to use patchworks just to find some arbitrary ID number
> > that some 3rd party tool generates and add it to notes attached to a
> > commit.
> 
> I'm a mutt user as well.  I'm not necessarily the biggest fan of patchwork
> either, but it does turn out to be helpful sometimes.  I don't know what your
> workflow is like.  Is there any chance you can get the message id in there?
> 
> messageid=$(formail -X Message-Id: < $patch | awk '{print $2}')
> git notes append -m "Message-Id: $messageid"

Doesn't work for my workflow, because I don't keep individual
patches around - I pushed everything to mbox format files and then
bulk git-am the file. And that still doesn't solve the patchwork id
problem, or the fact non standard repo configs are needed to manage
it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs