Workflows Archive on lore.kernel.org
 help / color / Atom feed
* script to check "Fixes:" tags
@ 2019-09-26 19:51 Greg KH
  2019-09-27  7:08 ` Krzysztof Kozlowski
  2019-09-27  7:49 ` Geert Uytterhoeven
  0 siblings, 2 replies; 10+ messages in thread
From: Greg KH @ 2019-09-26 19:51 UTC (permalink / raw)
  To: workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-26 19:51 script to check "Fixes:" tags Greg KH
@ 2019-09-27  7:08 ` Krzysztof Kozlowski
  2019-09-27  7:49 ` Geert Uytterhoeven
  1 sibling, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2019-09-27  7:08 UTC (permalink / raw)
  To: Greg KH; +Cc: workflows

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


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

* Re: script to check "Fixes:" tags
  2019-09-26 19:51 script to check "Fixes:" tags Greg KH
  2019-09-27  7:08 ` Krzysztof Kozlowski
@ 2019-09-27  7:49 ` Geert Uytterhoeven
  2019-09-27 12:01   ` Rob Herring
  2019-09-27 18:01   ` Greg KH
  1 sibling, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2019-09-27  7:49 UTC (permalink / raw)
  To: Greg KH; +Cc: workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27  7:49 ` Geert Uytterhoeven
@ 2019-09-27 12:01   ` Rob Herring
  2019-09-27 18:03     ` Greg KH
  2019-09-27 18:01   ` Greg KH
  1 sibling, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-09-27 12:01 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Greg KH, workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27  7:49 ` Geert Uytterhoeven
  2019-09-27 12:01   ` Rob Herring
@ 2019-09-27 18:01   ` Greg KH
  2019-09-27 18:06     ` Geert Uytterhoeven
  1 sibling, 1 reply; 10+ messages in thread
From: Greg KH @ 2019-09-27 18:01 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27 12:01   ` Rob Herring
@ 2019-09-27 18:03     ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2019-09-27 18:03 UTC (permalink / raw)
  To: Rob Herring; +Cc: Geert Uytterhoeven, workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27 18:01   ` Greg KH
@ 2019-09-27 18:06     ` Geert Uytterhoeven
  2019-09-27 18:35       ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2019-09-27 18:06 UTC (permalink / raw)
  To: Greg KH; +Cc: workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27 18:06     ` Geert Uytterhoeven
@ 2019-09-27 18:35       ` Greg KH
  2019-09-27 19:00         ` Geert Uytterhoeven
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2019-09-27 18:35 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27 18:35       ` Greg KH
@ 2019-09-27 19:00         ` Geert Uytterhoeven
  2019-09-27 19:46           ` Theodore Y. Ts'o
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2019-09-27 19:00 UTC (permalink / raw)
  To: Greg KH; +Cc: workflows

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

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

* Re: script to check "Fixes:" tags
  2019-09-27 19:00         ` Geert Uytterhoeven
@ 2019-09-27 19:46           ` Theodore Y. Ts'o
  0 siblings, 0 replies; 10+ messages in thread
From: Theodore Y. Ts'o @ 2019-09-27 19:46 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Greg KH, workflows

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

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

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26 19:51 script to check "Fixes:" tags Greg KH
2019-09-27  7:08 ` Krzysztof Kozlowski
2019-09-27  7:49 ` Geert Uytterhoeven
2019-09-27 12:01   ` Rob Herring
2019-09-27 18:03     ` Greg KH
2019-09-27 18:01   ` Greg KH
2019-09-27 18:06     ` Geert Uytterhoeven
2019-09-27 18:35       ` Greg KH
2019-09-27 19:00         ` Geert Uytterhoeven
2019-09-27 19:46           ` Theodore Y. Ts'o

Workflows Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/workflows/0 workflows/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 workflows workflows/ https://lore.kernel.org/workflows \
		workflows@vger.kernel.org
	public-inbox-index workflows

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.workflows


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git