script to check "Fixes:" tags

script to check "Fixes:" tags

[Greg KH]

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

Since this list was created to share scripts, here's one that I
currently use to test that the "Fixes:" tags are correct on a commit.  I
run it on all patches that I accept into my trees, after getting emails
from Stephen one too many times :)

It's almost entirely based on Stephen's original script, but has been
changed a bit in formatting and usage to be a stand-alone script that
anyone can use.

To use:
	verify_fixes.sh GIT_RANGE

if all is good, script will print nothing out and exit with success.  If
there is a problem it will be printed out and the script will exit with
an error that you can check from any other program.

It it 'shellcheck' clean, but I'm sure that there are other things wrong
with it, so feel free to point out problems.

And should stuff like this be in the kernel tree itself?

thanks,

greg k-h

[-- Attachment #2: verify_fixes.sh --]
[-- Type: application/x-sh, Size: 5494 bytes --]

Re: script to check "Fixes:" tags

[Krzysztof Kozlowski]

On Thu, Sep 26, 2019 at 09:51:43PM +0200, Greg KH wrote:
> Since this list was created to share scripts, here's one that I
> currently use to test that the "Fixes:" tags are correct on a commit.  I
> run it on all patches that I accept into my trees, after getting emails
> from Stephen one too many times :)
> 
> It's almost entirely based on Stephen's original script, but has been
> changed a bit in formatting and usage to be a stand-alone script that
> anyone can use.
> 
> To use:
> 	verify_fixes.sh GIT_RANGE
> 
> if all is good, script will print nothing out and exit with success.  If
> there is a problem it will be printed out and the script will exit with
> an error that you can check from any other program.
> 
> It it 'shellcheck' clean, but I'm sure that there are other things wrong
> with it, so feel free to point out problems.
> 
> And should stuff like this be in the kernel tree itself?

+1

Also a kernel.org repo for maintainer scripts would be useful. Extending
own scripts for generic usage it's a lot of boring and not needed (for
the owner) task. But if someone else does the job...

Best regards,
Krzysztof

Re: script to check "Fixes:" tags

[Geert Uytterhoeven]

Hi Greg,

On Thu, Sep 26, 2019 at 9:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> Since this list was created to share scripts, here's one that I
> currently use to test that the "Fixes:" tags are correct on a commit.  I
> run it on all patches that I accept into my trees, after getting emails
> from Stephen one too many times :)
>
> It's almost entirely based on Stephen's original script, but has been
> changed a bit in formatting and usage to be a stand-alone script that
> anyone can use.
>
> To use:
>         verify_fixes.sh GIT_RANGE
>
> if all is good, script will print nothing out and exit with success.  If
> there is a problem it will be printed out and the script will exit with
> an error that you can check from any other program.
>
> It it 'shellcheck' clean, but I'm sure that there are other things wrong
> with it, so feel free to point out problems.

Thanks!

> And should stuff like this be in the kernel tree itself?

Probably this should be integrated into checkpatch.pl?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: script to check "Fixes:" tags

[Rob Herring]

On Fri, Sep 27, 2019 at 2:50 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Greg,
>
> On Thu, Sep 26, 2019 at 9:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > Since this list was created to share scripts, here's one that I
> > currently use to test that the "Fixes:" tags are correct on a commit.  I
> > run it on all patches that I accept into my trees, after getting emails
> > from Stephen one too many times :)
> >
> > It's almost entirely based on Stephen's original script, but has been
> > changed a bit in formatting and usage to be a stand-alone script that
> > anyone can use.
> >
> > To use:
> >         verify_fixes.sh GIT_RANGE
> >
> > if all is good, script will print nothing out and exit with success.  If
> > there is a problem it will be printed out and the script will exit with
> > an error that you can check from any other program.
> >
> > It it 'shellcheck' clean, but I'm sure that there are other things wrong
> > with it, so feel free to point out problems.
>
> Thanks!
>
> > And should stuff like this be in the kernel tree itself?
>
> Probably this should be integrated into checkpatch.pl?

I believe both checks already are.

I think we're looking at the problem the wrong way around. Stephen
shouldn't have to be checking this, and neither should maintainers. It
could be easily automated. We could have a checkpatch bot that runs
some subset of checkpatch checks we agree are always errors like the
ones here and replies to patches with the errors.

Rob

Re: script to check "Fixes:" tags

[Greg KH]

On Fri, Sep 27, 2019 at 09:49:55AM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Thu, Sep 26, 2019 at 9:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > Since this list was created to share scripts, here's one that I
> > currently use to test that the "Fixes:" tags are correct on a commit.  I
> > run it on all patches that I accept into my trees, after getting emails
> > from Stephen one too many times :)
> >
> > It's almost entirely based on Stephen's original script, but has been
> > changed a bit in formatting and usage to be a stand-alone script that
> > anyone can use.
> >
> > To use:
> >         verify_fixes.sh GIT_RANGE
> >
> > if all is good, script will print nothing out and exit with success.  If
> > there is a problem it will be printed out and the script will exit with
> > an error that you can check from any other program.
> >
> > It it 'shellcheck' clean, but I'm sure that there are other things wrong
> > with it, so feel free to point out problems.
> 
> Thanks!
> 
> > And should stuff like this be in the kernel tree itself?
> 
> Probably this should be integrated into checkpatch.pl?

I can't run checkpatch.pl on everything as sometimes I need to take
patches that do not pass it :)

checkpatch.pl is supposed to check for s-o-b but for some reason I don't
think it does all of the checking that my other script does.  Or at
least I haven't noticed it.

thanks,

greg k-h

Re: script to check "Fixes:" tags

[Greg KH]

On Fri, Sep 27, 2019 at 07:01:17AM -0500, Rob Herring wrote:
> I think we're looking at the problem the wrong way around. Stephen
> shouldn't have to be checking this, and neither should maintainers. It
> could be easily automated. We could have a checkpatch bot that runs
> some subset of checkpatch checks we agree are always errors like the
> ones here and replies to patches with the errors.

Sure, it would be nice to have that, but until someone sets that up to
run on all patches sent to mailing lists, we need something locally to
run to catch these things.

Also, many maintainers mess this up, I run these checks on trees I pull
from and I have caught errors that checkpatch wouldn't have caught due
to maintainers forgetting to sign off on things they are committing
somehow.  email tests wouldn't have caught that.

thanks,

greg k-h

Re: script to check "Fixes:" tags

[Geert Uytterhoeven]

Hi Greg,

On Fri, Sep 27, 2019 at 8:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Sep 27, 2019 at 09:49:55AM +0200, Geert Uytterhoeven wrote:
> > On Thu, Sep 26, 2019 at 9:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > Since this list was created to share scripts, here's one that I
> > > currently use to test that the "Fixes:" tags are correct on a commit.  I
> > > run it on all patches that I accept into my trees, after getting emails
> > > from Stephen one too many times :)
> > >
> > > It's almost entirely based on Stephen's original script, but has been
> > > changed a bit in formatting and usage to be a stand-alone script that
> > > anyone can use.
> > >
> > > To use:
> > >         verify_fixes.sh GIT_RANGE
> > >
> > > if all is good, script will print nothing out and exit with success.  If
> > > there is a problem it will be printed out and the script will exit with
> > > an error that you can check from any other program.
> > >
> > > It it 'shellcheck' clean, but I'm sure that there are other things wrong
> > > with it, so feel free to point out problems.
> >
> > Thanks!
> >
> > > And should stuff like this be in the kernel tree itself?
> >
> > Probably this should be integrated into checkpatch.pl?
>
> I can't run checkpatch.pl on everything as sometimes I need to take
> patches that do not pass it :)

Running checkpatch and taking patches are not mutually exclusive ;-)

> checkpatch.pl is supposed to check for s-o-b but for some reason I don't
> think it does all of the checking that my other script does.  Or at
> least I haven't noticed it.

May be true, but it doesn't help to fragment the checking script space.
Just like we just want to run get_maintainer.pl, we just want to run
checkpatch.pl, don't we?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: script to check "Fixes:" tags

[Greg KH]

On Fri, Sep 27, 2019 at 08:06:18PM +0200, Geert Uytterhoeven wrote:
> Hi Greg,
> 
> On Fri, Sep 27, 2019 at 8:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Sep 27, 2019 at 09:49:55AM +0200, Geert Uytterhoeven wrote:
> > > On Thu, Sep 26, 2019 at 9:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > Since this list was created to share scripts, here's one that I
> > > > currently use to test that the "Fixes:" tags are correct on a commit.  I
> > > > run it on all patches that I accept into my trees, after getting emails
> > > > from Stephen one too many times :)
> > > >
> > > > It's almost entirely based on Stephen's original script, but has been
> > > > changed a bit in formatting and usage to be a stand-alone script that
> > > > anyone can use.
> > > >
> > > > To use:
> > > >         verify_fixes.sh GIT_RANGE
> > > >
> > > > if all is good, script will print nothing out and exit with success.  If
> > > > there is a problem it will be printed out and the script will exit with
> > > > an error that you can check from any other program.
> > > >
> > > > It it 'shellcheck' clean, but I'm sure that there are other things wrong
> > > > with it, so feel free to point out problems.
> > >
> > > Thanks!
> > >
> > > > And should stuff like this be in the kernel tree itself?
> > >
> > > Probably this should be integrated into checkpatch.pl?
> >
> > I can't run checkpatch.pl on everything as sometimes I need to take
> > patches that do not pass it :)
> 
> Running checkpatch and taking patches are not mutually exclusive ;-)
> 
> > checkpatch.pl is supposed to check for s-o-b but for some reason I don't
> > think it does all of the checking that my other script does.  Or at
> > least I haven't noticed it.
> 
> May be true, but it doesn't help to fragment the checking script space.
> Just like we just want to run get_maintainer.pl, we just want to run
> checkpatch.pl, don't we?

do we?

Again, I can't do it in my "workflow" at the moment.  This list was for
people to post what they do and get comments.

And a comment of "don't use the tool you have, use something else that
doesn't do what you really need to do", isn't all that helpful :)

I just noticed that checkpatch can handle a git range, so I'll revisit
it and see if I can "just test one thing" with it or not, next week.

thanks,

greg k-h

Re: script to check "Fixes:" tags

[Geert Uytterhoeven]

Hi Greg,

On Fri, Sep 27, 2019 at 8:36 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Sep 27, 2019 at 08:06:18PM +0200, Geert Uytterhoeven wrote:
> > On Fri, Sep 27, 2019 at 8:01 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Fri, Sep 27, 2019 at 09:49:55AM +0200, Geert Uytterhoeven wrote:
> > > > On Thu, Sep 26, 2019 at 9:51 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > Since this list was created to share scripts, here's one that I
> > > > > currently use to test that the "Fixes:" tags are correct on a commit.  I
> > > > > run it on all patches that I accept into my trees, after getting emails
> > > > > from Stephen one too many times :)
> > > > >
> > > > > It's almost entirely based on Stephen's original script, but has been
> > > > > changed a bit in formatting and usage to be a stand-alone script that
> > > > > anyone can use.
> > > > >
> > > > > To use:
> > > > >         verify_fixes.sh GIT_RANGE
> > > > >
> > > > > if all is good, script will print nothing out and exit with success.  If
> > > > > there is a problem it will be printed out and the script will exit with
> > > > > an error that you can check from any other program.
> > > > >
> > > > > It it 'shellcheck' clean, but I'm sure that there are other things wrong
> > > > > with it, so feel free to point out problems.
> > > >
> > > > Thanks!
> > > >
> > > > > And should stuff like this be in the kernel tree itself?
> > > >
> > > > Probably this should be integrated into checkpatch.pl?
> > >
> > > I can't run checkpatch.pl on everything as sometimes I need to take
> > > patches that do not pass it :)
> >
> > Running checkpatch and taking patches are not mutually exclusive ;-)
> >
> > > checkpatch.pl is supposed to check for s-o-b but for some reason I don't
> > > think it does all of the checking that my other script does.  Or at
> > > least I haven't noticed it.
> >
> > May be true, but it doesn't help to fragment the checking script space.
> > Just like we just want to run get_maintainer.pl, we just want to run
> > checkpatch.pl, don't we?
>
> do we?

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#style-check-your-changes

> Again, I can't do it in my "workflow" at the moment.  This list was for
> people to post what they do and get comments.

You're applying patches from a mailbox folder, right?
So you can run checkpatch against all individual patches in the folder
using:

    formail -s scripts/checkpatch.pl < /path/to/the/mbox/to/apply

BTW, that also applies to patchwork bundles, which is what I started
using recently.

> And a comment of "don't use the tool you have, use something else that
> doesn't do what you really need to do", isn't all that helpful :)

Actually that also applies in reverse, I do use checkpatch.pl ;-)

Use and improve the shared tool that comes with the kernel, instead of
using the one in your cathedral? Unless I'm missing something, that's
one of the general ideas behind this...

> I just noticed that checkpatch can handle a git range, so I'll revisit
> it and see if I can "just test one thing" with it or not, next week.

That assumes the patches have been applied already...
Which is fine for maintainers taking pull requests.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: script to check "Fixes:" tags

[Theodore Y. Ts'o]

On Fri, Sep 27, 2019 at 09:00:37PM +0200, Geert Uytterhoeven wrote:
> > I just noticed that checkpatch can handle a git range, so I'll revisit
> > it and see if I can "just test one thing" with it or not, next week.
> 
> That assumes the patches have been applied already...
> Which is fine for maintainers taking pull requests.

Or for maintainers who pull patches into a git repository for review
purposes.  For complex patches, or long patch series, it can be easier
to use Gerrit than reviewing patches in your inbox, because there's
not enough context in e-mailed diff.  So what I do get around this
problem when the patch series is sent via e-mail is to apply them onto
a temporary git branch.  That means I can look at the patch series in
the context of a whole tree.  It also means I can launch a smoke test
while I'm looking at the changes.

With modern review systems, the automatic build test, static code
checker runs, running uninit tests, etc., happen in parallel and
automatically the moment the patch(es) are submitted for review, even
before a human starts to pay attention to it.

Right now we're trying to simulate this by having the zero-day bot try
to guess what commit against which the patch or patch series are
applied.  Unnfortunately, sometimes the zero-day bot gets it wrong,
and recently it can take 4-5 days before the zero day bot goes through
its backlog, and today, the zero day bot doesn't know when a patch
series has been superceeded by a newer version, so it wastes CPU time
and generates noise on the mailing list.

So I can imagine a scheme where (optionally) a patch submitter would
push their patch series to some review queue, and the system would
e-mail the patches to the mailing list on behalf of the user.  In
parallel, build tests, regression tests could be run, checkpatch,
etc. could be run, etc., against that temporary git branch.

This has a number of advantages; newcomers won't have to fight with
their corporate e-mail systems to avoid white space damaging.  (This
can be quite challenging, and if their company is using Lotus Notes,
impossible.  :-)  It would also make it simpler for systems to run
tests against the branch, and could also enable some simplicity for
the test runners to be able to tell when the V20 version of a patch
series has been superceded by V21.

					- Ted