linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* linux-next: Signed-off-by missing for commits in the xfs tree
@ 2021-06-20 22:26 Stephen Rothwell
  2021-06-21 17:12 ` Darrick J. Wong
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Rothwell @ 2021-06-20 22:26 UTC (permalink / raw)
  To: Darrick J. Wong, David Chinner, linux-xfs
  Cc: Allison Henderson, Chandan Babu R, Linux Kernel Mailing List,
	Linux Next Mailing List

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-06-20 22:26 linux-next: Signed-off-by missing for commits in the xfs tree Stephen Rothwell
@ 2021-06-21 17:12 ` Darrick J. Wong
  2021-06-21 21:27   ` Stephen Rothwell
  2021-06-21 21:54   ` Dave Chinner
  0 siblings, 2 replies; 9+ messages in thread
From: Darrick J. Wong @ 2021-06-21 17:12 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Chinner, linux-xfs, Allison Henderson, Chandan Babu R,
	Linux Kernel Mailing List, Linux Next Mailing List

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-06-21 17:12 ` Darrick J. Wong
@ 2021-06-21 21:27   ` Stephen Rothwell
  2021-06-21 21:51     ` Darrick J. Wong
  2021-06-21 21:54   ` Dave Chinner
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Rothwell @ 2021-06-21 21:27 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: David Chinner, linux-xfs, Allison Henderson, Chandan Babu R,
	Linux Kernel Mailing List, Linux Next Mailing List

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-06-21 21:27   ` Stephen Rothwell
@ 2021-06-21 21:51     ` Darrick J. Wong
  2021-06-21 22:10       ` Stephen Rothwell
  2021-06-21 23:06       ` Stephen Rothwell
  0 siblings, 2 replies; 9+ messages in thread
From: Darrick J. Wong @ 2021-06-21 21:51 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Chinner, linux-xfs, Allison Henderson, Chandan Babu R,
	Linux Kernel Mailing List, Linux Next Mailing List

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-06-21 17:12 ` Darrick J. Wong
  2021-06-21 21:27   ` Stephen Rothwell
@ 2021-06-21 21:54   ` Dave Chinner
  1 sibling, 0 replies; 9+ messages in thread
From: Dave Chinner @ 2021-06-21 21:54 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Stephen Rothwell, linux-xfs, Allison Henderson, Chandan Babu R,
	Linux Kernel Mailing List, Linux Next Mailing List

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-06-21 21:51     ` Darrick J. Wong
@ 2021-06-21 22:10       ` Stephen Rothwell
  2021-06-21 23:06       ` Stephen Rothwell
  1 sibling, 0 replies; 9+ messages in thread
From: Stephen Rothwell @ 2021-06-21 22:10 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: David Chinner, linux-xfs, Allison Henderson, Chandan Babu R,
	Linux Kernel Mailing List, Linux Next Mailing List

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-06-21 21:51     ` Darrick J. Wong
  2021-06-21 22:10       ` Stephen Rothwell
@ 2021-06-21 23:06       ` Stephen Rothwell
  1 sibling, 0 replies; 9+ messages in thread
From: Stephen Rothwell @ 2021-06-21 23:06 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Stephen Rothwell, David Chinner, linux-xfs, Allison Henderson,
	Chandan Babu R, Linux Kernel Mailing List,
	Linux Next Mailing List


[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: linux-next: Signed-off-by missing for commits in the xfs tree
  2021-08-10 21:49 Stephen Rothwell
@ 2021-08-11 19:21 ` Darrick J. Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Chinner, linux-xfs, Linux Kernel Mailing List,
	Linux Next Mailing List

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* linux-next: Signed-off-by missing for commits in the xfs tree
@ 2021-08-10 21:49 Stephen Rothwell
  2021-08-11 19:21 ` Darrick J. Wong
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Rothwell @ 2021-08-10 21:49 UTC (permalink / raw)
  To: Darrick J. Wong, David Chinner, linux-xfs
  Cc: Linux Kernel Mailing List, Linux Next Mailing List

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-11 19:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-20 22:26 linux-next: Signed-off-by missing for commits in the xfs tree Stephen Rothwell
2021-06-21 17:12 ` Darrick J. Wong
2021-06-21 21:27   ` Stephen Rothwell
2021-06-21 21:51     ` Darrick J. Wong
2021-06-21 22:10       ` Stephen Rothwell
2021-06-21 23:06       ` Stephen Rothwell
2021-06-21 21:54   ` Dave Chinner
2021-08-10 21:49 Stephen Rothwell
2021-08-11 19:21 ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).