[RFC] kernel.bbclass: run checkpatch.pl for all patches

Submitted by Mikko Rapeli on April 17, 2020, 12:11 p.m. | Patch ID: 172014

Details

Message ID 20200417121103.27918-1-mikko.rapeli@bmw.de
State New
Headers show

Commit Message

Mikko Rapeli April 17, 2020, 12:11 p.m.
RFC: currently unconditionally, but this may need to be changed because
so many patches are not passing this check. Even upstream backports
from Linus Torvalds are failing the check sometimes.

This is test is important to keep all kernel patches in upstreamable state.
It would help keeping BSP and project specific kernel recipes in better shape.
It will not help if BSP git trees contain 1000's of non-upstreamed
patches. Passing this script is mandatory for patches according to
https://www.kernel.org/doc/html/latest/process/submit-checklist.html

checkpatch.pl catches for example:

 * missing Signed-off-by lines
 * spaces vs. tabs indents, new line issues etc coding style issues
 * some bugs like useless initialization of global variables

Signed-off-by: Mikko Rapeli <mikko.rapeli@bmw.de>
---
 meta/classes/kernel.bbclass | 9 +++++++++
 1 file changed, 9 insertions(+)

Patch hide | download patch | download mbox

diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
index a724645466..b0c0359d38 100644
--- a/meta/classes/kernel.bbclass
+++ b/meta/classes/kernel.bbclass
@@ -547,6 +547,15 @@  do_savedefconfig() {
 do_savedefconfig[nostamp] = "1"
 addtask savedefconfig after do_configure
 
+do_checkpatch() {
+	bbplain "Running checkpatch.pl QA check for all kernel patches"
+	if ls "${S}"/patches/*.patch > /dev/null; then
+		"${S}/scripts/checkpatch.pl" "${S}"/patches/*.patch
+	fi
+}
+do_checkpatch[nostamp] = "1"
+addtask checkpatch after do_configure
+
 inherit cml1
 
 KCONFIG_CONFIG_COMMAND_append = " HOSTLDFLAGS='${BUILD_LDFLAGS}'"

Comments

Armin Kuster April 17, 2020, 4:02 p.m.
On 4/17/20 5:11 AM, Mikko Rapeli wrote:
> RFC: currently unconditionally, but this may need to be changed because
> so many patches are not passing this check. Even upstream backports
> from Linus Torvalds are failing the check sometimes.
>
> This is test is important to keep all kernel patches in upstreamable state.
> It would help keeping BSP and project specific kernel recipes in better shape.
> It will not help if BSP git trees contain 1000's of non-upstreamed
> patches. Passing this script is mandatory for patches according to
> https://www.kernel.org/doc/html/latest/process/submit-checklist.html

I am not opposed to having more checks, I worry about the impact on build.

Maybe have this in a separate class so folks can opt-in or out depending
on final conclusion of this RFC.

- armin
>
> checkpatch.pl catches for example:
>
>  * missing Signed-off-by lines
>  * spaces vs. tabs indents, new line issues etc coding style issues
>  * some bugs like useless initialization of global variables
>
> Signed-off-by: Mikko Rapeli <mikko.rapeli@bmw.de>
> ---
>  meta/classes/kernel.bbclass | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
> index a724645466..b0c0359d38 100644
> --- a/meta/classes/kernel.bbclass
> +++ b/meta/classes/kernel.bbclass
> @@ -547,6 +547,15 @@ do_savedefconfig() {
>  do_savedefconfig[nostamp] = "1"
>  addtask savedefconfig after do_configure
>  
> +do_checkpatch() {
> +	bbplain "Running checkpatch.pl QA check for all kernel patches"
> +	if ls "${S}"/patches/*.patch > /dev/null; then
> +		"${S}/scripts/checkpatch.pl" "${S}"/patches/*.patch
> +	fi
> +}
> +do_checkpatch[nostamp] = "1"
> +addtask checkpatch after do_configure
> +
>  inherit cml1
>  
>  KCONFIG_CONFIG_COMMAND_append = " HOSTLDFLAGS='${BUILD_LDFLAGS}'"
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#137275): https://lists.openembedded.org/g/openembedded-core/message/137275
Mute This Topic: https://lists.openembedded.org/mt/73075458/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Mikko Rapeli April 17, 2020, 5:48 p.m.
On Fri, Apr 17, 2020 at 09:02:30AM -0700, akuster808 wrote:
> 
> 
> On 4/17/20 5:11 AM, Mikko Rapeli wrote:
> > RFC: currently unconditionally, but this may need to be changed because
> > so many patches are not passing this check. Even upstream backports
> > from Linus Torvalds are failing the check sometimes.
> >
> > This is test is important to keep all kernel patches in upstreamable state.
> > It would help keeping BSP and project specific kernel recipes in better shape.
> > It will not help if BSP git trees contain 1000's of non-upstreamed
> > patches. Passing this script is mandatory for patches according to
> > https://www.kernel.org/doc/html/latest/process/submit-checklist.html
> 
> I am not opposed to having more checks, I worry about the impact on build.
> 
> Maybe have this in a separate class so folks can opt-in or out depending
> on final conclusion of this RFC.

Agreed. In theory this would belong to insane.bbclass, or would only be
added if some extra variable or similar is set in the recipes using
kernel.bbclass, or a separate class if you will. I would need some pointers
here.

I'm also playing with sparse so that I can easily do sparse checker builds of
kernels from different BSPs. Will hopefully use similar approach with that too...

-Mikko
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#137276): https://lists.openembedded.org/g/openembedded-core/message/137276
Mute This Topic: https://lists.openembedded.org/mt/73075458/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-