diff mbox series

[01/55] insane.bbclass: add a SUMMARY/HOMEPAGE check (oe-core recipes only)

Message ID 20230614092918.4065570-1-alex@linutronix.de
State New
Headers show
Series [01/55] insane.bbclass: add a SUMMARY/HOMEPAGE check (oe-core recipes only) | expand

Commit Message

Alexander Kanavin June 14, 2023, 9:28 a.m. UTC
This was done in a selftest, but that is too late and creates
friction in integration as errors are not seen until autobuilder fails.

Bonus fix: SUMMARY check wasn't even working, as in the absence
of one set in the recipe there is a default value set from bitbake.conf.

I left DESCRIPTION check out for now, as many recipes don't actually
have it, and it's set from SUMMARY (plus a dot) if absent.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/classes-global/insane.bbclass         | 26 ++++++++++++++++
 meta/lib/oeqa/selftest/cases/distrodata.py | 36 ----------------------
 2 files changed, 26 insertions(+), 36 deletions(-)

Comments

Richard Purdie June 14, 2023, 11:33 a.m. UTC | #1
On Wed, 2023-06-14 at 11:28 +0200, Alexander Kanavin wrote:
> This was done in a selftest, but that is too late and creates
> friction in integration as errors are not seen until autobuilder fails.
> 
> Bonus fix: SUMMARY check wasn't even working, as in the absence
> of one set in the recipe there is a default value set from bitbake.conf.
> 
> I left DESCRIPTION check out for now, as many recipes don't actually
> have it, and it's set from SUMMARY (plus a dot) if absent.
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>  meta/classes-global/insane.bbclass         | 26 ++++++++++++++++
>  meta/lib/oeqa/selftest/cases/distrodata.py | 36 ----------------------
>  2 files changed, 26 insertions(+), 36 deletions(-)

I think this change is in the right direction but there are a few
things I think we need to tweak here.

a) I don't like the " if not '/meta/recipes-'" check at all. We now
have the groundwork for layer overrides in bitbake so I'd like to
complete that, then we can make some of these WARN_QA entries as layer
specific. This has the added bounus that other layers and more easily
adopt the tests and raise the bar on quality.

b) I really really don't want a do_fetch postfunc doing something that
isn't fetching. I know why you've done this, so it appears early in the
build and so on but it doesn't belong there.

There are other sanity tests at parse time in anonymous python which
also bother me since the increase parsing time and also shouldn't be
there. Those often aren't gated on ERROR_QA/WARN_QA options either so
can't be configured.

Given all this, do we want to consider a new task which covers these
kinds of tests? A new task does have certain overhead but I'm starting
to think we might need to do that. Of course there is then a dillema
about which task it would need to run before.

c) There is an issue that warnings are not restored from sstate. If you
rerun this build after seeing the warnings, the warnings will not show
again. This is going to confuse users and cause issues to get missed on
the autobuilder. There is an open bug for this one, it isn't an easy
fix unfortunately.

Cheers,

Richard
Ross Burton June 14, 2023, 3:02 p.m. UTC | #2
On 14 Jun 2023, at 12:33, Richard Purdie via lists.openembedded.org <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote:
> Given all this, do we want to consider a new task which covers these
> kinds of tests? A new task does have certain overhead but I'm starting
> to think we might need to do that. Of course there is then a dillema
> about which task it would need to run before.

I agree: somewhere lost in my pile of branches is a half-completed implementation of a recipe-scope QA check to collect together these checks which can be ran from just the metadata and don’t depend on the results of do_package.

Ross
diff mbox series

Patch

diff --git a/meta/classes-global/insane.bbclass b/meta/classes-global/insane.bbclass
index 8788f58fc5b..632f738c86d 100644
--- a/meta/classes-global/insane.bbclass
+++ b/meta/classes-global/insane.bbclass
@@ -34,6 +34,7 @@  WARN_QA ?= " libdir xorg-driver-abi buildpaths \
             missing-update-alternatives native-last missing-ptest \
             license-exists license-no-generic license-syntax license-format \
             license-incompatible license-file-missing obsolete-license \
+            missing-metadata \
             "
 ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \
             perms dep-cmp pkgvarcheck perm-config perm-line perm-link \
@@ -1473,6 +1474,28 @@  python do_qa_unpack() {
     unpack_check_src_uri(d.getVar('PN'), d)
 }
 
+python do_qa_fetch() {
+    def test_missing_metadata(d):
+        fn = d.getVar("FILE")
+        if not '/meta/recipes-' in fn:
+            # We are only interested in OE-Core
+            return
+        pn = d.getVar('BPN')
+        srcfile = d.getVar('SRC_URI').split()
+        # Check that SUMMARY is not the same as the default from bitbake.conf
+        if d.getVar('SUMMARY') == d.expand("${PN} version ${PV}-${PR}"):
+            oe.qa.handle_error("missing-metadata", "Recipe {} in {} does not contain a SUMMARY. Please add an entry.".format(pn, fn), d)
+        if not d.getVar('HOMEPAGE'):
+            if srcfile and srcfile[0].startswith('file') or not d.getVar('SRC_URI'):
+                # We are only interested in recipes SRC_URI fetched from external sources
+                pass
+            else:
+                oe.qa.handle_error("missing-metadata", "Recipe {} in {} does not contain a HOMEPAGE. Please add an entry.".format(pn, fn), d)
+
+    test_missing_metadata(d)
+    oe.qa.exit_if_errors(d)
+}
+
 # Check for patch fuzz
 do_patch[postfuncs] += "do_qa_patch "
 
@@ -1484,6 +1507,9 @@  do_configure[postfuncs] += "do_qa_configure "
 # Check does S exist.
 do_unpack[postfuncs] += "do_qa_unpack"
 
+# Check basic recipe metadata (e.g. SUMMARY/HOMEPAGE/RECIPE_MAINTAINER)
+do_fetch[postfuncs] += "do_qa_fetch"
+
 python () {
     import re
     
diff --git a/meta/lib/oeqa/selftest/cases/distrodata.py b/meta/lib/oeqa/selftest/cases/distrodata.py
index c83a3a7bd67..fd262fe3c9e 100644
--- a/meta/lib/oeqa/selftest/cases/distrodata.py
+++ b/meta/lib/oeqa/selftest/cases/distrodata.py
@@ -39,42 +39,6 @@  but their recipes claim otherwise by setting UPSTREAM_VERSION_UNKNOWN. Please re
 """ + "\n".join(regressed_successes)
         self.assertTrue(len(regressed_failures) == 0 and len(regressed_successes) == 0, msg)
 
-    def test_missing_homepg(self):
-        """
-        Summary:     Test for oe-core recipes that don't have a HOMEPAGE or DESCRIPTION
-        Expected:    All oe-core recipes should have a DESCRIPTION entry
-        Expected:    All oe-core recipes should have a HOMEPAGE entry except for recipes that are not fetched from external sources.
-        Product:     oe-core
-        """
-        with bb.tinfoil.Tinfoil() as tinfoil:
-            tinfoil.prepare(config_only=False)
-            no_description = []
-            no_homepage = []
-            for fn in tinfoil.all_recipe_files(variants=False):
-                if not '/meta/recipes-' in fn:
-                    # We are only interested in OE-Core
-                    continue
-                rd = tinfoil.parse_recipe_file(fn, appends=False)
-                pn = rd.getVar('BPN')
-                srcfile = rd.getVar('SRC_URI').split()
-                #Since DESCRIPTION defaults to SUMMARY if not set, we are only interested in recipes without DESCRIPTION or SUMMARY
-                if not (rd.getVar('SUMMARY') or rd.getVar('DESCRIPTION')):
-                    no_description.append((pn, fn))
-                if not rd.getVar('HOMEPAGE'):
-                    if srcfile and srcfile[0].startswith('file') or not rd.getVar('SRC_URI'):
-                        # We are only interested in recipes SRC_URI fetched from external sources
-                        continue
-                    no_homepage.append((pn, fn))
-        if no_homepage:
-            self.fail("""
-The following recipes do not have a HOMEPAGE. Please add an entry for HOMEPAGE in the recipe.
-""" + "\n".join(['%s (%s)' % i for i in no_homepage]))
-
-        if no_description:
-            self.fail("""
-The following recipes do not have a DESCRIPTION. Please add an entry for DESCRIPTION in the recipe.
-""" + "\n".join(['%s (%s)' % i for i in no_description]))
-
     def test_maintainers(self):
         """
         Summary:     Test that oe-core recipes have a maintainer and entries in maintainers list have a recipe