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

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

[Stephen Rothwell]

Hi all,

Commit

  74456cceed0b ("iomap: fix integer truncation issues in the zeroing and dirtying helpers")

is missing a Signed-off-by from its author.

-- 
Cheers,
Stephen Rothwell

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

[Konstantin Ryabitsev]

On Wed, Oct 05, 2022 at 08:04:00AM +1100, Dave Chinner wrote:
> > That said... I think we should get in the habit of asking patch authors
> > to make sure that at least one of the email or name strings match
> > between the From and SOB tags.  I can see how people who grok even less
> > about how Chinese names work than I do (read: lawyers) might get fussy
> > about this kind of thing.
> 
> As per above, the normal solution is an explicit "From: <foo>" line
> that matches the SOB.  It's just annoying that our new-fangled tools
> haven't encoded this long-standing convention to warn us when we
> pull a patch with a from-tag that doesn't match a sob-tag.

This is the case of "there's multiple opinions of what's right" here. The
logic for matching "person tags" is as follows:

- check that entire email matches (a.smith@intel.com == a.smith@intel.com)
- failing that, check that the name is a full match 
  ("Alex Smith" == "Alex Smith")
- failing that, check if there's a comma in the From and swap it around
  ("Smith, Alex" == "Alex Smith")

The last two checks were added based on a request, I'm pretty sure. Before
that we only did full email check and complained about trailer mismatches if
it was failing. If the previous behaviour was "more right" then I'm happy to
roll back or put this up for a "what is more correct" vote.

-K

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

[Willy Tarreau]

On Tue, Oct 04, 2022 at 08:01:26PM -0700, Darrick J. Wong wrote:
> I think Dave means something like this patch of mine:
> https://lore.kernel.org/linux-xfs/166473478893.1083155.2555785331844801316.stgit@magnolia/T/#u
> 
> From:   "Darrick J. Wong" <djwong@kernel.org>
> To:     djwong@kernel.org
> Cc:     linux-xfs@vger.kernel.org
> Date:   Sun, 02 Oct 2022 11:19:48 -0700
> Subject: [PATCH 3/4] xfs: set the buffer type after holding the AG[IF] across trans_roll
> 
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Currently, the only way to lock an allocation group is to hold the AGI
> and AGF buffers.  If repair needs to roll the transaction while
> repairing some AG metadata, it maintains that lock by holding the two
> buffers across the transaction roll and joins them afterwards.
> 
> However, repair is not the same as the other parts of XFS that employ
> this bhold/bjoin sequence, because it's possible that the AGI or AGF
> buffers are not actually dirty before the roll.  In this case, the
> buffer log item can detach from the buffer, which means that we have to
> re-set the buffer type in the bli after joining the buffer to the new
> transaction so that log recovery will know what to do if the fs fails.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> 
> Notice how after the Subject: there is a blank line (which terminates
> the headers) followed by a new From: line in the body?  And the
> name/email in that second From: line matches the SOB later on?

Or maybe we could have a new option to git-am to always use the first
SOB as the From when there's no other explicit From in the message, so
that we never care about the From used to send the e-mail ? That would
also implicitly perform a requirement that an SOB necessarily exists.

Willy

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

[Darrick J. Wong]

On Wed, Oct 05, 2022 at 10:52:05AM +0800, Stephen Zhang wrote:
> Dave Chinner <david@fromorbit.com> 于2022年10月5日周三 05:06写道：
> > As it is, I use the convention of putting an explicit From: tag in
> > the commit message that matches the SOB so tools pulling stuff from
> > mailing lists do the right thing with them (same as any third-party
> > provided patch in a series).
> >
> > > That said... I think we should get in the habit of asking patch authors
> > > to make sure that at least one of the email or name strings match
> > > between the From and SOB tags.  I can see how people who grok even less
> > > about how Chinese names work than I do (read: lawyers) might get fussy
> > > about this kind of thing.
> >
> > As per above, the normal solution is an explicit "From: <foo>" line
> > that matches the SOB.  It's just annoying that our new-fangled tools
> > haven't encoded this long-standing convention to warn us when we
> > pull a patch with a from-tag that doesn't match a sob-tag.
> >
> 
> Sorry, but I'm not sure whether what you mean is adding another "From: " line
> right above the SOB tag like:
> ====
> From: name2 <email address2>
> Date: Mon, 12 Sep 2022 xx:xx:xx +0800
> Subject: [PATCH ] xfs: fix xxx and xxx
> 
> ...
> the commit  message
> ...
> 
> From: name <email address>             //added line
> signed-off-by:  name <email address>
> ...
> ====

I think Dave means something like this patch of mine:
https://lore.kernel.org/linux-xfs/166473478893.1083155.2555785331844801316.stgit@magnolia/T/#u

From:   "Darrick J. Wong" <djwong@kernel.org>
To:     djwong@kernel.org
Cc:     linux-xfs@vger.kernel.org
Date:   Sun, 02 Oct 2022 11:19:48 -0700
Subject: [PATCH 3/4] xfs: set the buffer type after holding the AG[IF] across trans_roll

From: Darrick J. Wong <djwong@kernel.org>

Currently, the only way to lock an allocation group is to hold the AGI
and AGF buffers.  If repair needs to roll the transaction while
repairing some AG metadata, it maintains that lock by holding the two
buffers across the transaction roll and joins them afterwards.

However, repair is not the same as the other parts of XFS that employ
this bhold/bjoin sequence, because it's possible that the AGI or AGF
buffers are not actually dirty before the roll.  In this case, the
buffer log item can detach from the buffer, which means that we have to
re-set the buffer type in the bli after joining the buffer to the new
transaction so that log recovery will know what to do if the fs fails.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---

Notice how after the Subject: there is a blank line (which terminates
the headers) followed by a new From: line in the body?  And the
name/email in that second From: line matches the SOB later on?

--D

> 
> Thanks,
> Stephen.

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

[Stephen Zhang]

Dave Chinner <david@fromorbit.com> 于2022年10月5日周三 05:06写道：
> As it is, I use the convention of putting an explicit From: tag in
> the commit message that matches the SOB so tools pulling stuff from
> mailing lists do the right thing with them (same as any third-party
> provided patch in a series).
>
> > That said... I think we should get in the habit of asking patch authors
> > to make sure that at least one of the email or name strings match
> > between the From and SOB tags.  I can see how people who grok even less
> > about how Chinese names work than I do (read: lawyers) might get fussy
> > about this kind of thing.
>
> As per above, the normal solution is an explicit "From: <foo>" line
> that matches the SOB.  It's just annoying that our new-fangled tools
> haven't encoded this long-standing convention to warn us when we
> pull a patch with a from-tag that doesn't match a sob-tag.
>

Sorry, but I'm not sure whether what you mean is adding another "From: " line
right above the SOB tag like:
====
From: name2 <email address2>
Date: Mon, 12 Sep 2022 xx:xx:xx +0800
Subject: [PATCH ] xfs: fix xxx and xxx

...
the commit  message
...

From: name <email address>             //added line
signed-off-by:  name <email address>
...
====

Thanks,
Stephen.

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

[Dave Chinner]

On Tue, Oct 04, 2022 at 08:57:34AM -0700, Darrick J. Wong wrote:
> On Tue, Oct 04, 2022 at 10:50:12PM +1100, Stephen Rothwell wrote:
> > Hi Dave,
> > 
> > On Tue, 4 Oct 2022 09:21:03 +1100 Dave Chinner <david@fromorbit.com> wrote:
> > >
> > > The commit matches exactly what was sent to the list. It's just
> > > that the patch was sent from a personal email address with a
> > > corporate signoff.
> > > 
> > > Since when has that been an issue?  I -personally- have been doing
> > > this for well over a decade and I'm pretty sure there are lots of
> > > other people who also do this.
> > 
> > If you are happy (as the maintainer), then fine.

Not really, I don't like it when our tools don't do the right thing,
are entirely silent about it and then I get surprised by custom
checks other people run.

> > My script just could
> > not connect those 2 email addresses.  I check for matches between the
> > address itself (the part between the <>) or a match between the "name"
> > part (before the <>).  If either matches (or it is obvious) then I
> > don't report it.

Yup, during development of the patches the names started out
matching. The SOB stayed the same but the name on the personal email
address got anglicised, hence nothing matched by the time I pulled
it with b4.

> > I have reported very few of these.
> 
> My checkpatch is happier if the whole "name <email>" string matches, but
> it'll accept name matches.  This ofc rests upon the assumption that
> I can spot the deepcake'd Dave Chinners hawking phones in Russia or
> whatever. ;)

If someone wants to fake me and do my work for me so I don't have to
do anything, I'm all for it. :)

As it is, I use the convention of putting an explicit From: tag in
the commit message that matches the SOB so tools pulling stuff from
mailing lists do the right thing with them (same as any third-party
provided patch in a series). 

> That said... I think we should get in the habit of asking patch authors
> to make sure that at least one of the email or name strings match
> between the From and SOB tags.  I can see how people who grok even less
> about how Chinese names work than I do (read: lawyers) might get fussy
> about this kind of thing.

As per above, the normal solution is an explicit "From: <foo>" line
that matches the SOB.  It's just annoying that our new-fangled tools
haven't encoded this long-standing convention to warn us when we
pull a patch with a from-tag that doesn't match a sob-tag.

And, yes, I know about git hooks - forcing every maintainer to
implement their own custom git hooks to catch errors the tool they
are all using can easily catch is not a reliable or scalable
solution. We use common tools for a reason.

-Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[Darrick J. Wong]

On Tue, Oct 04, 2022 at 10:50:12PM +1100, Stephen Rothwell wrote:
> Hi Dave,
> 
> On Tue, 4 Oct 2022 09:21:03 +1100 Dave Chinner <david@fromorbit.com> wrote:
> >
> > The commit matches exactly what was sent to the list. It's just
> > that the patch was sent from a personal email address with a
> > corporate signoff.
> > 
> > Since when has that been an issue?  I -personally- have been doing
> > this for well over a decade and I'm pretty sure there are lots of
> > other people who also do this.
> 
> If you are happy (as the maintainer), then fine.  My script just could
> not connect those 2 email addresses.  I check for matches between the
> address itself (the part between the <>) or a match between the "name"
> part (before the <>).  If either matches (or it is obvious) then I
> don't report it.
> 
> I have reported very few of these.

My checkpatch is happier if the whole "name <email>" string matches, but
it'll accept name matches.  This ofc rests upon the assumption that
I can spot the deepcake'd Dave Chinners hawking phones in Russia or
whatever. ;)

That said... I think we should get in the habit of asking patch authors
to make sure that at least one of the email or name strings match
between the From and SOB tags.  I can see how people who grok even less
about how Chinese names work than I do (read: lawyers) might get fussy
about this kind of thing.

--D

> > Hence if this is wrong, then we've got a tooling problem with b4.
> > Why does b4 allow this rather than warn/fail if it's not actually
> > allowed in the linux-next tree?
> 
> These reports are more of "is this right/was this a slipup?" rather
> than "this is not allowed" i.e.. there are circumstances under which
> the actual author does not (or cannot) provide a Signed-off-by and that
> is OK.
> -- 
> Cheers,
> Stephen Rothwell

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

[Stephen Rothwell]

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

Hi Dave,

On Tue, 4 Oct 2022 09:21:03 +1100 Dave Chinner <david@fromorbit.com> wrote:
>
> The commit matches exactly what was sent to the list. It's just
> that the patch was sent from a personal email address with a
> corporate signoff.
> 
> Since when has that been an issue?  I -personally- have been doing
> this for well over a decade and I'm pretty sure there are lots of
> other people who also do this.

If you are happy (as the maintainer), then fine.  My script just could
not connect those 2 email addresses.  I check for matches between the
address itself (the part between the <>) or a match between the "name"
part (before the <>).  If either matches (or it is obvious) then I
don't report it.

I have reported very few of these.

> Hence if this is wrong, then we've got a tooling problem with b4.
> Why does b4 allow this rather than warn/fail if it's not actually
> allowed in the linux-next tree?

These reports are more of "is this right/was this a slipup?" rather
than "this is not allowed" i.e.. there are circumstances under which
the actual author does not (or cannot) provide a Signed-off-by and that
is OK.
-- 
Cheers,
Stephen Rothwell

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

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

[Dave Chinner]

On Tue, Oct 04, 2022 at 07:23:02AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Commits
> 
>   ece9d1c54c23 ("xfs: rearrange the logic and remove the broken comment for xfs_dir2_isxx")
>   7ee7a280ea9d ("xfs: trim the mapp array accordingly in xfs_da_grow_inode_int")
> 
> are missing a Signed-off-by from their author.

Say what?

I just pulled them w/ b4 via their msg-ids. Have a look at the email
here:

https://lore.kernel.org/linux-xfs/20220918065026.1207016-1-zhangshida@kylinos.cn/

And the commit information from the XFS tree says:

author	Stephen Zhang <starzhangzsd@gmail.com>	2022-09-26 10:36:11 +1000
committer	Dave Chinner <david@fromorbit.com>	2022-09-26 10:36:11 +1000
commit	ece9d1c54c23c316219c19c4c7091495007e149b (patch)
tree	f4bc7747e2b604cf7718c584126e3e9bc8d5a51a
parent	7ee7a280ea9d3208c075151b06190630b8c20775 (diff)
download xfs-linux-ece9d1c54c23c316219c19c4c7091495007e149b.tar.gz

xfs: rearrange the logic and remove the broken comment for xfs_dir2_isxx

xfs_dir2_isleaf is used to see if the directory is a single-leaf
form directory instead, as commented right above the function.

Besides getting rid of the broken comment, we rearrange the logic by
converting everything over to standard formatting and conventions,
at the same time, to make it easier to understand and self
documenting.

Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Dave Chinner <david@fromorbit.com>

----

The commit matches exactly what was sent to the list. It's just
that the patch was sent from a personal email address with a
corporate signoff.

Since when has that been an issue?  I -personally- have been doing
this for well over a decade and I'm pretty sure there are lots of
other people who also do this.

Hence if this is wrong, then we've got a tooling problem with b4.
Why does b4 allow this rather than warn/fail if it's not actually
allowed in the linux-next tree?

-Dave.
-- 
Dave Chinner
david@fromorbit.com

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

[Stephen Rothwell]

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

Hi all,

Commits

  ece9d1c54c23 ("xfs: rearrange the logic and remove the broken comment for xfs_dir2_isxx")
  7ee7a280ea9d ("xfs: trim the mapp array accordingly in xfs_da_grow_inode_int")

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

-- 
Cheers,
Stephen Rothwell

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

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

[Stephen Rothwell]

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

Hi all,

Commit

  9f059beac967 ("xfs: cleanup error handling in xfs_buf_get_map")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell

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

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

[Stephen Rothwell]

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

Hi all,

Commit

  ea52eff66dcd ("xfs: preserve inode versioning across remounts")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell

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

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

[Darrick J. Wong]

On Tue, Jun 05, 2018 at 08:03:40AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   1e7fb5893364 ("xfs_repair: check and repair quota metadata")
> 
> is missing a Signed-off-by from its committer.

Oops, yeah, I just backported that over from xfsprogs, will fix it up.

Thanks for catching this.

--D

> 
> -- 
> Cheers,
> Stephen Rothwell

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

[Stephen Rothwell]

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

Hi all,

Commit

  1e7fb5893364 ("xfs_repair: check and repair quota metadata")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell

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