[PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Holger Freyther]

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


Make conf/bitbake.conf load conf/insane.conf that will INHERIT the
insane.bbclass. Remove the INHERIT from the distros currently using
insane.bbclass. Change insane.bbclass to check QA_FATAL to see if errors
are allowed to be fatal, make SHR/Kaelios and Angstrom use that new
flag to continue having fatal QA Errors.

Signed-off-by: Holger Hans Peter Freyther <zecke@selfish.org>
---
 classes/insane.bbclass               |    5 ++++-
 conf/bitbake.conf                    |    1 +
 conf/distro/include/angstrom.inc     |    4 ++--
 conf/distro/include/kaeilos-2010.inc |    4 ++--
 conf/distro/include/kaeilos.inc      |    4 ++--
 conf/distro/shr.conf                 |    3 +--
 conf/insane.conf                     |    2 ++
 7 files changed, 14 insertions(+), 9 deletions(-)
 create mode 100644 conf/insane.conf



[-- Attachment #2: 0001-insane.bbclass-Enable-the-insane.bbclass-for-everyon.patch --]
[-- Type: text/x-patch, Size: 3420 bytes --]

diff --git a/classes/insane.bbclass b/classes/insane.bbclass
index 4696c25..d137dfc 100644
--- a/classes/insane.bbclass
+++ b/classes/insane.bbclass
@@ -106,12 +106,15 @@ def package_qa_clean_path(path,d):
     """ Remove the common prefix from the path. In this case it is the TMPDIR"""
     return path.replace(bb.data.getVar('TMPDIR',d,True),"")
 
-def package_qa_make_fatal_error(error_class, name, path,d):
+def package_qa_make_fatal_error(error_class, name, path, d):
     """
     decide if an error is fatal
 
     TODO: Load a whitelist of known errors
     """
+
+    if not bb.data.getVar("QA_FATAL", d):
+        return False
     return not error_class in [0, 5, 7]
 
 def package_qa_write_error(error_class, name, path, d):
diff --git a/conf/bitbake.conf b/conf/bitbake.conf
index b099499..bc8eea0 100644
--- a/conf/bitbake.conf
+++ b/conf/bitbake.conf
@@ -701,6 +701,7 @@ require conf/sanity.conf
 require conf/abi_version.conf
 require conf/enterprise.conf
 require conf/compatibility-providers.conf
+require conf/insane.conf
 
 ##################################################################
 # Weak variables (usually to retain backwards compatibility)
diff --git a/conf/distro/include/angstrom.inc b/conf/distro/include/angstrom.inc
index 8983c86..030ef3b 100644
--- a/conf/distro/include/angstrom.inc
+++ b/conf/distro/include/angstrom.inc
@@ -85,10 +85,10 @@ INHERIT += "devshell"
 #use angstrom source mirrors to combat 404s
 INHERIT += "angstrom-mirrors" 
 
-#run QA tests on builds and packages and log them  
-INHERIT += "insane"
 # INHERIT += "packagehistory"
+#log QA errors
 QA_LOG = "1"
+QA_FATAL = "1"
 
 #run QA tests on recipes
 INHERIT += "recipe_sanity"
diff --git a/conf/distro/include/kaeilos-2010.inc b/conf/distro/include/kaeilos-2010.inc
index 2fb30ce..7061aee 100644
--- a/conf/distro/include/kaeilos-2010.inc
+++ b/conf/distro/include/kaeilos-2010.inc
@@ -53,9 +53,9 @@ INHERIT += "sanity"
 #make devshell available as task
 INHERIT += "devshell" 
 
-#run QA tests on builds and packages and log them  
-INHERIT += "insane"
+#log QA errors
 QA_LOG = "1"
+QA_FATAL = "1"
 
 #run QA tests on recipes
 INHERIT += "recipe_sanity"
diff --git a/conf/distro/include/kaeilos.inc b/conf/distro/include/kaeilos.inc
index edbad69..af8eca3 100644
--- a/conf/distro/include/kaeilos.inc
+++ b/conf/distro/include/kaeilos.inc
@@ -84,10 +84,10 @@ INHERIT += "devshell"
 #use angstrom source mirrors to combat 404s
 INHERIT += "angstrom-mirrors" 
 
-#run QA tests on builds and packages and log them  
-INHERIT += "insane"
 # INHERIT += "packagehistory"
+#log QA errors
 QA_LOG = "1"
+QA_FATAL = "1"
 
 #run QA tests on recipes
 INHERIT += "recipe_sanity"
diff --git a/conf/distro/shr.conf b/conf/distro/shr.conf
index 9dd1c3e..aa94f95 100644
--- a/conf/distro/shr.conf
+++ b/conf/distro/shr.conf
@@ -40,10 +40,9 @@ INHERIT += "testlab"
 #activate the angstrom blacklister
 INHERIT += "angstrom"
 
-#run QA tests on builds and packages and log them
-INHERIT += "insane"
 # INHERIT += "packagehistory"
 QA_LOG = "1"
+QA_FATAL = "1"
 
 #make devshell available as task
 INHERIT += "devshell"
diff --git a/conf/insane.conf b/conf/insane.conf
new file mode 100644
index 0000000..5324529
--- /dev/null
+++ b/conf/insane.conf
@@ -0,0 +1,2 @@
+# Run insane for everyone
+INHERIT += "insane"

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Koen Kooi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12-09-10 11:52, Holger Freyther wrote:
> 
> Make conf/bitbake.conf load conf/insane.conf that will INHERIT the
> insane.bbclass. Remove the INHERIT from the distros currently using
> insane.bbclass. Change insane.bbclass to check QA_FATAL to see if errors
> are allowed to be fatal, make SHR/Kaelios and Angstrom use that new
> flag to continue having fatal QA Errors.
> 
> Signed-off-by: Holger Hans Peter Freyther <zecke@selfish.org>

Looks good to me, but a slight nit-pick:

+#log QA errors
 QA_LOG = "1"
+QA_FATAL = "1"

Can you change the comment to something like:

# Log QA errors and abort the build when encountering them

So that it's clear what the 2 vars do?

regards,

Koen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFMjL6GMkyGM64RGpERAu6nAJ9YVJtjYLE4RJGT8vJaFtCUUpNHLwCfQUXn
bWPfFuQSJMkJMmjkg1j7gAY=
=Chyt
-----END PGP SIGNATURE-----

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Frans Meulenbroeks]

2010/9/12 Holger Freyther <holger+oe@freyther.de>:
>
> Make conf/bitbake.conf load conf/insane.conf that will INHERIT the
> insane.bbclass. Remove the INHERIT from the distros currently using
> insane.bbclass. Change insane.bbclass to check QA_FATAL to see if errors
> are allowed to be fatal, make SHR/Kaelios and Angstrom use that new
> flag to continue having fatal QA Errors.
>
> Signed-off-by: Holger Hans Peter Freyther <zecke@selfish.org>

I am not really in favour of this.

One of the disadvantages of insane.bbclass is that it has:
PACKAGE_DEPENDS += "pax-utils-native desktop-file-utils-native"
This drags in several packages (e.g. sqlite3-native glib-2.0-native)
only to check if the .desktop file is valid.

By making this mandatory quite some additional build time is needed
(on a clean build), which is kind-of a nuisance and does not bring
anything if you are working on a display-less system (e.g. a NAS).

I''d be more in favour to split off the .desktop test and use that
only for those packages that generate .desktop files. (and make the
rest of insane.bbclass mandatory).
Alternately perhaps it is possible to create a var or so by which
people in their local or distro .conf can turn insane.bbclass (or the
.desktop part) off.

Frans

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Leon Woestenberg]

Hello,

On Sun, Sep 12, 2010 at 2:32 PM, Frans Meulenbroeks
<fransmeulenbroeks@gmail.com> wrote:
> 2010/9/12 Holger Freyther <holger+oe@freyther.de>:
>>
>> Make conf/bitbake.conf load conf/insane.conf that will INHERIT the
>> insane.bbclass. Remove the INHERIT from the distros currently using
>> insane.bbclass. Change insane.bbclass to check QA_FATAL to see if errors
>> are allowed to be fatal, make SHR/Kaelios and Angstrom use that new
>> flag to continue having fatal QA Errors.
>>
>> Signed-off-by: Holger Hans Peter Freyther <zecke@selfish.org>
>
> I am not really in favour of this.
>
> One of the disadvantages of insane.bbclass is that it has:
> PACKAGE_DEPENDS += "pax-utils-native desktop-file-utils-native"
> This drags in several packages (e.g. sqlite3-native glib-2.0-native)
> only to check if the .desktop file is valid.
>
Good point.

Could the insane class be more configurable to what it has to check,
and have it's dependency generated from that configuration?

Regards,
-- 
Leon

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Holger Freyther]

On 09/12/2010 07:50 PM, Koen Kooi wrote:


> Can you change the comment to something like:
> 
> # Log QA errors and abort the build when encountering them
> 
> So that it's clear what the 2 vars do?


true.

BTW: why is packagehistory disabled? Was it ever enabled?

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Holger Freyther]

On 09/12/2010 08:32 PM, Frans Meulenbroeks wrote:
> 2010/9/12 Holger Freyther <holger+oe@freyther.de>:

>> Signed-off-by: Holger Hans Peter Freyther <zecke@selfish.org>
> 
> I am not really in favour of this.

This is an outstanding item from the TSC, I am not sure how to proceed from
here, as we will go in circles.

I am happy as long as Angstrom is using this option, as this way I will not
have a unzip package with the wrong binaries ever again.

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Chris Larson]

On Sun, Sep 12, 2010 at 8:00 AM, Holger Freyther
<holger+oe@freyther.de<holger%2Boe@freyther.de>
> wrote:

> On 09/12/2010 08:32 PM, Frans Meulenbroeks wrote:
> > 2010/9/12 Holger Freyther <holger+oe@freyther.de<holger%2Boe@freyther.de>
> >:
>
> >> Signed-off-by: Holger Hans Peter Freyther <zecke@selfish.org>
> >
> > I am not really in favour of this.
>
> This is an outstanding item from the TSC, I am not sure how to proceed from
> here, as we will go in circles.
>
> I am happy as long as Angstrom is using this option, as this way I will not
> have a unzip package with the wrong binaries ever again.
> <http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel>
>

I think we should reorganize insane.bbclass and split up its functionality,
either into multiple classes or configurable via variables, as others have
suggested in this thread.  If that's done, we can enable the core by
default, but leave the bits that require additional dependencies as
optional.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Koen Kooi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12-09-10 16:06, Holger Freyther wrote:
> On 09/12/2010 07:50 PM, Koen Kooi wrote:
> 
> 
>> Can you change the comment to something like:
>>
>> # Log QA errors and abort the build when encountering them
>>
>> So that it's clear what the 2 vars do?
> 
> 
> true.
> 
> BTW: why is packagehistory disabled? Was it ever enabled?

I disabled it when it turned out to invalidate lots of stammps, but that
was a while ago, it might behave differently nowadays.
Or I'm confusing it with lockdown...

regards,

Koen
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)

iD8DBQFMjSEFMkyGM64RGpERAjpIAJ91WsBuYCwKxNQ1ekB7kBHubH3kagCfUzvg
pX6MEM1QfiBXFvW7UkIXG6A=
=QfO3
-----END PGP SIGNATURE-----

Re: [PATCH] insane.bbclass: Enable the insane.bbclass for everyone in a warning mode

[Frans Meulenbroeks]

2010/9/12 Chris Larson <clarson@kergoth.com>:

>
> I think we should reorganize insane.bbclass and split up its functionality,
> either into multiple classes or configurable via variables, as others have
> suggested in this thread.  If that's done, we can enable the core by
> default, but leave the bits that require additional dependencies as
> optional.
> --

Agree.

One of the ways to do so is to add more semantics to SECTION where
whatever is in SECTION triggers a dependency on the sanity tester part
for that section.
Alternately we could decide that every lib file has an inherit lib
clause or something like that.
Disadvantage is that we need to check/update all recipes and there is
a chance that someone forgets to add such a line.
(of course we could work around that by saying that e.g. SECTION is
mandatory and can contain only a predefined number of values. Still
that does not beat the case that someone just uses a wrong value).

BTW I feel that whether (QA or other) tests are executed should be
ultimately controllable by the person who builds (e.g. in the
local.conf). He/she is the one who has to pay for these (in terms of
time and cycles and he/she gets the benefits (or gets burned) by using
it.

Frans