[PATCH 0/1] BBClass checker script (14235)

[PATCH 0/1] BBClass checker script (14235)

[Saul Wold]

This is the first pass at a script for addressing 14235 [0]:
bbclass file name convention is not consistent wrt dash and underscore

This is could be a contrib script or added to bitbake parsing somehow, 
I am open to suggestions for where to add this or if other tests are
needed.

[0] https://bugzilla.yoctoproject.org/show_bug.cgi?id=14235

Saul Wold (1):
  check-bbclasses: add new script to check bbclasses

 scripts/check-bbclasses | 109 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100755 scripts/check-bbclasses

-- 
2.34.1

[PATCH 1/1] check-bbclasses: add new script to check bbclasses

[Saul Wold]

FIXES [YOCTO #14235]

This script is a starting point for a "linter" for bbclass files.
Currently it will check for '_' in the bbclass filename and '-' in
addtask or EXPORT_FUNCTION. It will print warnings only no errors.

Signed-off-by: Saul Wold <sgw@bigsur.com>
---
 scripts/check-bbclasses | 109 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100755 scripts/check-bbclasses

diff --git a/scripts/check-bbclasses b/scripts/check-bbclasses
new file mode 100755
index 00000000000..ea525b2d118
--- /dev/null
+++ b/scripts/check-bbclasses
@@ -0,0 +1,109 @@
+#!/usr/bin/env python3
+#
+# Copyright OpenEmbedded Contributors
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# This script checks for bbclass like a linter can gives a
+# warning if any of the following issues:
+# * bbclass filename contains a '_' unless in the know list
+# * either an 'addtask' or 'EXPORT_FUNCTION' name contains a '-'
+#
+
+import sys, os, subprocess, re, shutil
+
+# List of known classes with '_' in OE-Core
+oecore_known_classes = (
+	"test_events.bbclass",
+	"migrate_localcount.bbclass",
+	"copyleft_compliance.bbclass",
+	"sign_ipk.bbclass",
+	"multilib_global.bbclass",
+	"useradd_base.bbclass",
+	"rm_work_and_downloads.bbclass",
+	"sign_rpm.bbclass",
+	"rm_work.bbclass",
+	"sign_package_feed.bbclass",
+	"copyleft_filter.bbclass",
+	"relative_symlinks.bbclass",
+	"recipe_sanity.bbclass",
+	"metadata_scm.bbclass",
+	"python_pyo3.bbclass",
+	"multilib_script.bbclass",
+	"multilib_header.bbclass",
+	"compress_doc.bbclass",
+	"populate_sdk.bbclass",
+	"license_image.bbclass",
+	"python_maturin.bbclass",
+	"python_setuptools3_rust.bbclass",
+	"image_types.bbclass",
+	"cargo_c.bbclass",
+	"bin_package.bbclass",
+	"python_poetry_core.bbclass",
+	"distro_features_check.bbclass",
+	"lib_package.bbclass",
+	"python_setuptools_build_meta.bbclass",
+	"populate_sdk_base.bbclass",
+	"features_check.bbclass",
+	"python_pep517.bbclass",
+	"cargo_common.bbclass",
+	"populate_sdk_ext.bbclass",
+	"rootfs_ipk.bbclass",
+	"rootfs_deb.bbclass",
+	"cpan_build.bbclass",
+	"rootfs_rpm.bbclass",
+	"python_flit_core.bbclass",
+	"python_hatchling.bbclass",
+	"image_types_wic.bbclass",
+	"setuptools3_legacy.bbclass",
+	"package_rpm.bbclass",
+	"package_deb.bbclass",
+	"package_ipk.bbclass",
+	"package_pkgdata.bbclass"
+)
+
+def get_tinfoil():
+    scripts_path = os.path.dirname(os.path.realpath(__file__))
+    lib_path = scripts_path + '/lib'
+    sys.path = sys.path + [lib_path]
+    import scriptpath
+    scriptpath.add_bitbake_lib_path()
+    import bb.tinfoil
+    tinfoil = bb.tinfoil.Tinfoil()
+    tinfoil.prepare()
+    # tinfoil.logger.setLevel(logging.WARNING)
+    return tinfoil
+
+if __name__=='__main__':
+    import argparse, shutil
+
+    parser = argparse.ArgumentParser(description='Sanity checker for bbclasses')
+    parser.add_argument("--verbose", default=False, action="store_true")
+    args = parser.parse_args()
+
+    tinfoil = get_tinfoil()
+
+    bbpath = tinfoil.config_data.getVar('BBPATH').split(':')
+    for path in bbpath:
+        with os.scandir(path) as it:
+            for entry in it:
+                if "classes" in entry.name and entry.is_dir():
+                    with os.scandir(path + "/" + entry.name) as classes:
+                        for c in classes:
+                            #
+                            # Check for underscore in bbclass filename
+                            #
+                            if c.name.endswith(".bbclass") and "_" in c.name and not c.name in oecore_known_classes:
+
+                                print("Warning: BBClass file name contains '_': " + path + "/" + entry.name + "/" + c.name)
+                            #
+                            # Check for '-' in exported functions and tasks
+                            #
+                            with open(path + "/" + entry.name + "/" + c.name) as f:
+                                for line in f.readlines():
+                                    if line.startswith("addtask ") and "-" in line:
+                                        print("Warning: addtask contains '-': " + path + "/" + entry.name + "/" + c.name + ": " + line)
+                                    if line.startswith("EXPORT_FUNCTIONS ") and "-" in line:
+                                        print("Warning: EXPORT_FUNCTIONS contains '-': " + path + "/" + entry.name + "/" + c.name + ": " + line)
+
+    tinfoil.shutdown()
-- 
2.34.1

Re: [OE-core] [PATCH 1/1] check-bbclasses: add new script to check bbclasses

[Jose Quaresma]

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

Saul Wold <sgw@bigsur.com> escreveu (domingo, 28/01/2024 à(s) 00:04):

> FIXES [YOCTO #14235]
>
> This script is a starting point for a "linter" for bbclass files.
> Currently it will check for '_' in the bbclass filename and '-' in
> addtask or EXPORT_FUNCTION. It will print warnings only no errors.
>
> Signed-off-by: Saul Wold <sgw@bigsur.com>
> ---
>  scripts/check-bbclasses | 109 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 109 insertions(+)
>  create mode 100755 scripts/check-bbclasses
>
> diff --git a/scripts/check-bbclasses b/scripts/check-bbclasses
> new file mode 100755
> index 00000000000..ea525b2d118
> --- /dev/null
> +++ b/scripts/check-bbclasses
> @@ -0,0 +1,109 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright OpenEmbedded Contributors
> +#
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# This script checks for bbclass like a linter can gives a
> +# warning if any of the following issues:
> +# * bbclass filename contains a '_' unless in the know list
> +# * either an 'addtask' or 'EXPORT_FUNCTION' name contains a '-'
> +#
> +
> +import sys, os, subprocess, re, shutil
> +
> +# List of known classes with '_' in OE-Core
> +oecore_known_classes = (
> +       "test_events.bbclass",
> +       "migrate_localcount.bbclass",
> +       "copyleft_compliance.bbclass",
> +       "sign_ipk.bbclass",
> +       "multilib_global.bbclass",
> +       "useradd_base.bbclass",
> +       "rm_work_and_downloads.bbclass",
> +       "sign_rpm.bbclass",
> +       "rm_work.bbclass",
> +       "sign_package_feed.bbclass",
> +       "copyleft_filter.bbclass",
> +       "relative_symlinks.bbclass",
> +       "recipe_sanity.bbclass",
> +       "metadata_scm.bbclass",
> +       "python_pyo3.bbclass",
> +       "multilib_script.bbclass",
> +       "multilib_header.bbclass",
> +       "compress_doc.bbclass",
> +       "populate_sdk.bbclass",
> +       "license_image.bbclass",
> +       "python_maturin.bbclass",
> +       "python_setuptools3_rust.bbclass",
> +       "image_types.bbclass",
> +       "cargo_c.bbclass",
> +       "bin_package.bbclass",
> +       "python_poetry_core.bbclass",
> +       "distro_features_check.bbclass",
> +       "lib_package.bbclass",
> +       "python_setuptools_build_meta.bbclass",
> +       "populate_sdk_base.bbclass",
> +       "features_check.bbclass",
> +       "python_pep517.bbclass",
> +       "cargo_common.bbclass",
> +       "populate_sdk_ext.bbclass",
> +       "rootfs_ipk.bbclass",
> +       "rootfs_deb.bbclass",
> +       "cpan_build.bbclass",
> +       "rootfs_rpm.bbclass",
> +       "python_flit_core.bbclass",
> +       "python_hatchling.bbclass",
> +       "image_types_wic.bbclass",
> +       "setuptools3_legacy.bbclass",
> +       "package_rpm.bbclass",
> +       "package_deb.bbclass",
> +       "package_ipk.bbclass",
> +       "package_pkgdata.bbclass"
> +)
>

Hi Saul,

Having this list ordered would make reading easier.

Jose


> +
> +def get_tinfoil():
> +    scripts_path = os.path.dirname(os.path.realpath(__file__))
> +    lib_path = scripts_path + '/lib'
> +    sys.path = sys.path + [lib_path]
> +    import scriptpath
> +    scriptpath.add_bitbake_lib_path()
> +    import bb.tinfoil
> +    tinfoil = bb.tinfoil.Tinfoil()
> +    tinfoil.prepare()
> +    # tinfoil.logger.setLevel(logging.WARNING)
> +    return tinfoil
> +
> +if __name__=='__main__':
> +    import argparse, shutil
> +
> +    parser = argparse.ArgumentParser(description='Sanity checker for
> bbclasses')
> +    parser.add_argument("--verbose", default=False, action="store_true")
> +    args = parser.parse_args()
> +
> +    tinfoil = get_tinfoil()
> +
> +    bbpath = tinfoil.config_data.getVar('BBPATH').split(':')
> +    for path in bbpath:
> +        with os.scandir(path) as it:
> +            for entry in it:
> +                if "classes" in entry.name and entry.is_dir():
> +                    with os.scandir(path + "/" + entry.name) as classes:
> +                        for c in classes:
> +                            #
> +                            # Check for underscore in bbclass filename
> +                            #
> +                            if c.name.endswith(".bbclass") and "_" in
> c.name and not c.name in oecore_known_classes:
> +
> +                                print("Warning: BBClass file name
> contains '_': " + path + "/" + entry.name + "/" + c.name)
> +                            #
> +                            # Check for '-' in exported functions and
> tasks
> +                            #
> +                            with open(path + "/" + entry.name + "/" +
> c.name) as f:
> +                                for line in f.readlines():
> +                                    if line.startswith("addtask ") and
> "-" in line:
> +                                        print("Warning: addtask contains
> '-': " + path + "/" + entry.name + "/" + c.name + ": " + line)
> +                                    if line.startswith("EXPORT_FUNCTIONS
> ") and "-" in line:
> +                                        print("Warning: EXPORT_FUNCTIONS
> contains '-': " + path + "/" + entry.name + "/" + c.name + ": " + line)
> +
> +    tinfoil.shutdown()
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#194436):
> https://lists.openembedded.org/g/openembedded-core/message/194436
> Mute This Topic: https://lists.openembedded.org/mt/104005172/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 9287 bytes --]

Re: [OE-core] [PATCH 1/1] check-bbclasses: add new script to check bbclasses

[Ola x Nilsson]

On Sat, Jan 27 2024, Saul Wold wrote:

> FIXES [YOCTO #14235]
>
> This script is a starting point for a "linter" for bbclass files.
> Currently it will check for '_' in the bbclass filename and '-' in
> addtask or EXPORT_FUNCTION. It will print warnings only no errors.
>
> Signed-off-by: Saul Wold <sgw@bigsur.com>
> ---
> +                            #
> +                            # Check for underscore in bbclass filename
> +                            #
> + if c.name.endswith(".bbclass") and "_" in c.name and not c.name in
> oecore_known_classes:
> +
> + print("Warning: BBClass file name contains '_': " + path + "/" +
> entry.name + "/" + c.name)

Why would '-' in class names be preferred?

Isn't EXPORT_FUNCTION supposed to be used like this:

```
myclass.bbclass:

myclass_do_something() {
 ...
}
addtask something

EXPORT_FUNCTION do_something
```

and the generated script would be

```
myclass_do_something() {
   ...
}

do_something() {
   myclass_do_something()
}

do_something
```

which would not work for shell functions when the class name contains
'-'.

What am I missing here?

-- 
Ola x Nilsson