[Linux-kernel-mentees] [PATCH] checkpatch: add fix option for MISSING_SIGN_OFF

Lukas Bulwahn lukas.bulwahn at gmail.com
Mon Nov 9 17:50:29 UTC 2020


On Mon, Nov 9, 2020 at 5:35 PM Aditya <yashsri421 at gmail.com> wrote:
>
> On 9/11/20 8:15 pm, Lukas Bulwahn wrote:
> > On Mon, Nov 9, 2020 at 3:33 PM Aditya <yashsri421 at gmail.com> wrote:
> >>
> >> On 9/11/20 7:10 pm, Lukas Bulwahn wrote:
> >>> On Mo., 9. Nov. 2020 at 14:10, Aditya Srivastava <yashsri421 at gmail.com>
> >>> wrote:
> >>>
> >>>> Currently checkpatch warns us if there is no 'Signed-off-by' line
> >>>> for the patch.
> >>>>
> >>>> E.g., running checkpatch on commit f68e7927212f ("Revert
> >>>> "powerpc/book3s32: Reorder _PAGE_XXX flags to simplify TLB handling"")
> >>>> reports this error:
> >>>>
> >>>> ERROR: Missing Signed-off-by: line(s)
> >>>>
> >>>> Provide a fix by adding a Signed-off-by line corresponding to the author
> >>>> of the patch before the start of diff(s)
> >>>>
> >>>
> >>> Can you please provide an evaluation first?
> >>>
> >>> As far as remember, the fix is often not to add a Signed-off-by: tag but it
> >>> requires something completely different.
> >>>
> >>
> >> I have generated the 'git logs' of the commits causing
> >> MISSING_SIGN_OFF at:
> >> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task7/summary_log.txt
> >>
> >> Actually this fix was suggested by Joe at:
> >> https://lore.kernel.org/linux-kernel-mentees/e8fef5fbd71331b010988769b249af6a79048f48.camel@perches.com/
> >>
> >
> > Yes I know. Now look at the reports.
> >
> > The rc and release commit tags do not get a Signed-off-by
> > intentionally. Other than that, reverts (for reverts is probably best
> > to explain how to do a revert... so that a sign-off-by is included)
> > and typos.
> >
> >
> > Lukas
> >
>
> Yes, I agree. But for the typos, the user must get BAD_SIGN_OFF
> warning for "Non-standard signature". So user will have to fix that
> manually. If so, this error is already avoided.
>

But then the fix should not be applied if there is a BAD_SIGN_OFF
warning, right?

Typos can also be fixed automatically by a fuzzy match. That is a
different feature, though and a different (next) patch.

> For the release commits, I think the user(s) may not use checkpatch
>

Agree. You can filter those out of your evaluation.

> For the rest (ie reverts) with no sign-offs, I think that if we just
> add the sign off line, it serves our purpose.
> Of course if we explain them to revert with sign-off, it will be
> probably the same outcome.
>

Then do both. Provide a fix in checkpatch.pl and provide documentation.

So, how many are because of reverts and how many are because of typos?

Also, how many reverts do currently include a proper sign-off?

When the majority does not include a proper sign-off on revert commit,
maybe the convention is to not have a sign-off on reverts.
When we have some data on that, we can discuss this with some central
maintainers in the kernel community.

If that is the case, then the fix is to change checkpatch.pl not to
complain on revert commits because of a missing sign-off.

> Thus, I feel that we can proceed with this fix for this class of error.
> What do you think/suggest?
>

Please continue on the steps above.

Lukas


More information about the Linux-kernel-mentees mailing list