linux-next: Signed-off-by missing for commits in the xfs tree

linux-next: Signed-off-by missing for commits in the xfs tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

Hi all,

Commits

  742140d2a486 ("xfs: xfs_log_force_lsn isn't passed a LSN")
  e30fbb337045 ("xfs: Fix CIL throttle hang when CIL space used going backwards")
  feb616896031 ("xfs: journal IO cache flush reductions")
  6a5c6f5ef0a4 ("xfs: remove need_start_rec parameter from xlog_write()")
  d7693a7f4ef9 ("xfs: CIL checkpoint flushes caches unconditionally")
  e45cc747a6fd ("xfs: async blkdev cache flush")
  9b845604a4d5 ("xfs: remove xfs_blkdev_issue_flush")
  25f25648e57c ("xfs: separate CIL commit record IO")
  a6a65fef5ef8 ("xfs: log stripe roundoff is a property of the log")

are missing a Signed-off-by from their committers.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Darrick J. Wong]

On Mon, Jun 21, 2021 at 08:26:56AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commits
> 
>   742140d2a486 ("xfs: xfs_log_force_lsn isn't passed a LSN")
>   e30fbb337045 ("xfs: Fix CIL throttle hang when CIL space used going backwards")
>   feb616896031 ("xfs: journal IO cache flush reductions")
>   6a5c6f5ef0a4 ("xfs: remove need_start_rec parameter from xlog_write()")
>   d7693a7f4ef9 ("xfs: CIL checkpoint flushes caches unconditionally")
>   e45cc747a6fd ("xfs: async blkdev cache flush")
>   9b845604a4d5 ("xfs: remove xfs_blkdev_issue_flush")
>   25f25648e57c ("xfs: separate CIL commit record IO")
>   a6a65fef5ef8 ("xfs: log stripe roundoff is a property of the log")
> 
> are missing a Signed-off-by from their committers.

<sigh> Ok, I'll rebase the branch again to fix the paperwork errors.

For future reference, if I want to continue accepting pull requests from
other XFS developers, what are the applicable standards for adding the
tree maintainer's (aka my) S-o-B tags?  I can't add my own S-o-Bs after
the fact without rewriting the branch history and changing the commit
ids (which would lose the signed tag), so I guess that means the person
sending the pull request has to add my S-o-B for me?  Which also doesn't
make sense?

--D

> -- 
> Cheers,
> Stephen Rothwell

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

Hi Darrick,

On Mon, 21 Jun 2021 10:12:08 -0700 "Darrick J. Wong" <djwong@kernel.org> wrote:
>
> On Mon, Jun 21, 2021 at 08:26:56AM +1000, Stephen Rothwell wrote:
> > 
> > Commits
> > 
> >   742140d2a486 ("xfs: xfs_log_force_lsn isn't passed a LSN")
> >   e30fbb337045 ("xfs: Fix CIL throttle hang when CIL space used going backwards")
> >   feb616896031 ("xfs: journal IO cache flush reductions")
> >   6a5c6f5ef0a4 ("xfs: remove need_start_rec parameter from xlog_write()")
> >   d7693a7f4ef9 ("xfs: CIL checkpoint flushes caches unconditionally")
> >   e45cc747a6fd ("xfs: async blkdev cache flush")
> >   9b845604a4d5 ("xfs: remove xfs_blkdev_issue_flush")
> >   25f25648e57c ("xfs: separate CIL commit record IO")
> >   a6a65fef5ef8 ("xfs: log stripe roundoff is a property of the log")
> > 
> > are missing a Signed-off-by from their committers.  
> 
> <sigh> Ok, I'll rebase the branch again to fix the paperwork errors.
> 
> For future reference, if I want to continue accepting pull requests from
> other XFS developers, what are the applicable standards for adding the
> tree maintainer's (aka my) S-o-B tags?  I can't add my own S-o-Bs after
> the fact without rewriting the branch history and changing the commit
> ids (which would lose the signed tag), so I guess that means the person
> sending the pull request has to add my S-o-B for me?  Which also doesn't
> make sense?

If you want to take a pull request, then use "git pull" (or "git fetch"
followed by "git merge") which will create a merge commit committed by
you.  The above commits were applied to your tree by you as patches (or
rebased) and so need your sign off.  The commits in a branch that you
just merge into your tree only need the SOBs for their author(s) and
committer.

If you then rebase your tree (with merge commits in it), you need to
use "git rebase -r" to preserve the merge commits.  alternatively, you
can rebase the commits you applied as patches and then redo the
pulls/merges manually.  You generally should not rebase other's work.

Of course, you should not really rebase a published tree at all (unless
vitally necessary) - see Documentation/maintainer/rebasing-and-merging.rst

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Darrick J. Wong]

On Tue, Jun 22, 2021 at 07:27:19AM +1000, Stephen Rothwell wrote:
> Hi Darrick,
> 
> On Mon, 21 Jun 2021 10:12:08 -0700 "Darrick J. Wong" <djwong@kernel.org> wrote:
> >
> > On Mon, Jun 21, 2021 at 08:26:56AM +1000, Stephen Rothwell wrote:
> > > 
> > > Commits
> > > 
> > >   742140d2a486 ("xfs: xfs_log_force_lsn isn't passed a LSN")
> > >   e30fbb337045 ("xfs: Fix CIL throttle hang when CIL space used going backwards")
> > >   feb616896031 ("xfs: journal IO cache flush reductions")
> > >   6a5c6f5ef0a4 ("xfs: remove need_start_rec parameter from xlog_write()")
> > >   d7693a7f4ef9 ("xfs: CIL checkpoint flushes caches unconditionally")
> > >   e45cc747a6fd ("xfs: async blkdev cache flush")
> > >   9b845604a4d5 ("xfs: remove xfs_blkdev_issue_flush")
> > >   25f25648e57c ("xfs: separate CIL commit record IO")
> > >   a6a65fef5ef8 ("xfs: log stripe roundoff is a property of the log")
> > > 
> > > are missing a Signed-off-by from their committers.  
> > 
> > <sigh> Ok, I'll rebase the branch again to fix the paperwork errors.
> > 
> > For future reference, if I want to continue accepting pull requests from
> > other XFS developers, what are the applicable standards for adding the
> > tree maintainer's (aka my) S-o-B tags?  I can't add my own S-o-Bs after
> > the fact without rewriting the branch history and changing the commit
> > ids (which would lose the signed tag), so I guess that means the person
> > sending the pull request has to add my S-o-B for me?  Which also doesn't
> > make sense?
> 
> If you want to take a pull request, then use "git pull" (or "git fetch"
> followed by "git merge") which will create a merge commit committed by
> you.  The above commits were applied to your tree by you as patches (or
> rebased) and so need your sign off.  The commits in a branch that you
> just merge into your tree only need the SOBs for their author(s) and
> committer.

I was about to point out all the complaints about when I actually /did/
merge Dave's branch, but I realized that those complaints were actually
because he wasn't consistently signing patches with the same email
address.

Um... do you know if there's a commit hook or something that all of us
can add to spot-check all this stuff?  I would really like to spend my
worry beans on about algorithms and code design, not worrying about how
many signature rules can be bent before LT starts refusing pull requests.

> If you then rebase your tree (with merge commits in it), you need to
> use "git rebase -r" to preserve the merge commits.  alternatively, you
> can rebase the commits you applied as patches and then redo the
> pulls/merges manually.  You generally should not rebase other's work.
> 
> Of course, you should not really rebase a published tree at all (unless
> vitally necessary) - see Documentation/maintainer/rebasing-and-merging.rst

Heh.  That ship has sailed, unfortunately.  If we /really/ care about
maintainers adding their own SoB tags to non-merge commits then I /have/
to rebase.

--D

> 
> -- 
> Cheers,
> Stephen Rothwell

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

Hi Darrick,

On Mon, 21 Jun 2021 14:51:59 -0700 "Darrick J. Wong" <djwong@kernel.org> wrote:
>
> > Of course, you should not really rebase a published tree at all (unless
> > vitally necessary) - see Documentation/maintainer/rebasing-and-merging.rst  
> 
> Heh.  That ship has sailed, unfortunately.  If we /really/ care about
> maintainers adding their own SoB tags to non-merge commits then I /have/
> to rebase.

We do *not* care about maintainers adding their own SOB to non-merge
commits that are in a branch that are all committed by someone else.
As you say, that is not possible without rewriting the whole branch.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Stephen Rothwell]

[-- Attachment #1.1: Type: text/plain, Size: 720 bytes --]

Hi Darrick,

On Mon, 21 Jun 2021 14:51:59 -0700 "Darrick J. Wong" <djwong@kernel.org> wrote:
>
> Um... do you know if there's a commit hook or something that all of us
> can add to spot-check all this stuff?  I would really like to spend my
> worry beans on about algorithms and code design, not worrying about how
> many signature rules can be bent before LT starts refusing pull requests.

Attached are the scripts I run over each tree as I fetch them each day.
The check_commits script calls the check_fixes script.

So, I fetch a maintainer's branch into a local branch and then run

check_commits ^origin/master <old head>..<new head>

origin/master is Linus' tree.
-- 
Cheers,
Stephen Rothwell

[-- Attachment #1.2: check_commits --]
[-- Type: application/x-shellscript, Size: 1694 bytes --]

[-- Attachment #1.3: check_fixes --]
[-- Type: application/x-shellscript, Size: 4440 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Dave Chinner]

On Mon, Jun 21, 2021 at 10:12:08AM -0700, Darrick J. Wong wrote:
> On Mon, Jun 21, 2021 at 08:26:56AM +1000, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Commits
> > 
> >   742140d2a486 ("xfs: xfs_log_force_lsn isn't passed a LSN")
> >   e30fbb337045 ("xfs: Fix CIL throttle hang when CIL space used going backwards")
> >   feb616896031 ("xfs: journal IO cache flush reductions")
> >   6a5c6f5ef0a4 ("xfs: remove need_start_rec parameter from xlog_write()")
> >   d7693a7f4ef9 ("xfs: CIL checkpoint flushes caches unconditionally")
> >   e45cc747a6fd ("xfs: async blkdev cache flush")
> >   9b845604a4d5 ("xfs: remove xfs_blkdev_issue_flush")
> >   25f25648e57c ("xfs: separate CIL commit record IO")
> >   a6a65fef5ef8 ("xfs: log stripe roundoff is a property of the log")
> > 
> > are missing a Signed-off-by from their committers.
> 
> <sigh> Ok, I'll rebase the branch again to fix the paperwork errors.
> 
> For future reference, if I want to continue accepting pull requests from
> other XFS developers, what are the applicable standards for adding the
> tree maintainer's (aka my) S-o-B tags?  I can't add my own S-o-Bs after
> the fact without rewriting the branch history and changing the commit
> ids (which would lose the signed tag), so I guess that means the person
> sending the pull request has to add my S-o-B for me?  Which also doesn't
> make sense?

None of those things. If there's a problem with a branch, you drop
the entire branch and ask the submitter to reformulate the branch
with a new tag and send a new pull request.

So I think the problem here is that you did, in fact, rewrite these
commits. e.g the commit I have in front of me:

https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=25f25648e57c793b4b18b010eac18a4e2f2b3050

Shows that it was committed at:

author		Dave Chinner <dchinner@redhat.com>	2021-06-18 08:21:48 -0700
committer	Darrick J. Wong <djwong@kernel.org>	2021-06-18 08:24:23 -0700

But in my original branch used for the pull request:

author		Dave Chinner <dchinner@redhat.com>	2021-06-03 14:57:24 +1000
committer	Dave Chinner <david@fromorbit.com>	2021-06-03 14:57:24 +1000

And that is what the script is complaining about.

AFAICT, based on the lack of a merge commit in the tree, is that you
rebased the commits out of the branch I originally asked you to pull
from. That resulted in them being rewritten in order into your tree
which meant you are now the committer, not me.

IOWs, if you do anything other than a direct merge of a signed tag,
you need to add your own SOB because you are creating new commits
rather than merging stable commit history from another branch.

I was going to ask you to revert the entire merge and then *maybe*
asking you to pull a smaller, tested branch with none of the
problems in it. That would have given you a clean merge and wouldn't
have lost the signed tag or the description text in the tag, but...

Hindsight says "talk about the plan first as it will save everyone a
lot of unnecessary work".

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

linux-next: Signed-off-by missing for commits in the xfs tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

Hi all,

Commits

  0f3901673631 ("xfs: Rename __xfs_attr_rmtval_remove")
  da8ca45da62e ("xfs: add attr state machine tracepoints")

are missing a Signed-off-by from their committer.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: Signed-off-by missing for commits in the xfs tree

[Darrick J. Wong]

On Wed, Aug 11, 2021 at 07:49:13AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commits
> 
>   0f3901673631 ("xfs: Rename __xfs_attr_rmtval_remove")
>   da8ca45da62e ("xfs: add attr state machine tracepoints")
> 
> are missing a Signed-off-by from their committer.

These should be fixed in today's for-next branch; thanks for letting me
know.

I have also spent this morning fixing some logic errors in my checkpatch
script (which basically does the same as the ones you sent me last time
this happened) and correcting a bug in the pre-push hook that resulted
in it not running. :/

--D

> 
> -- 
> Cheers,
> Stephen Rothwell