Patchwork sanity.bbclass: Move back to running at ConfigParsed time

login
register
mail settings
Submitter Richard Purdie
Date Sept. 5, 2012, 2:27 p.m.
Message ID <1346855269.21985.57.camel@ted>
Download mbox | patch
Permalink /patch/35943/
State Accepted
Commit 7b8a68cda7ef8186e834b39e73ee12a55b33f85b
Headers show

Comments

Richard Purdie - Sept. 5, 2012, 2:27 p.m.
If we don't do this, users can get extremely confused errors since the sanity tests
happen too late (after parsing) and don't see the warnings.

Also cleanup messages about merging configuration file changes to give better
hints at where the updated files may be.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Chris Larson - Sept. 5, 2012, 10:21 p.m.
On Wed, Sep 5, 2012 at 7:27 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> If we don't do this, users can get extremely confused errors since the sanity tests
> happen too late (after parsing) and don't see the warnings.

Can you elaborate on this? This commit message is extremely unclear.
If there's an open bug in bugzilla or something that could be referred
to here, that'd be helpful.
Richard Purdie - Sept. 5, 2012, 10:42 p.m.
On Wed, 2012-09-05 at 15:21 -0700, Chris Larson wrote:
> On Wed, Sep 5, 2012 at 7:27 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > If we don't do this, users can get extremely confused errors since the sanity tests
> > happen too late (after parsing) and don't see the warnings.
> 
> Can you elaborate on this? This commit message is extremely unclear.
> If there's an open bug in bugzilla or something that could be referred
> to here, that'd be helpful.

Sorry, I should have elaborated.

Set an invalid MACHINE, try and build and you set all kinds of nasty
warnings and no sensible message about what is wrong.

Change LCONF_VERSION and you don't get the warning unless parsing
completes which it may or may not depending on the kind of change.

and so on.

The sanity.bbclass code is meant to be helpful in these scenarios and
when its most needed, it wasn't working.

FWIW, I think we do need to fix the way sanity is being triggered and
improve this (massively). Unfortunately I didn't have time to do that
right this second but lets file an enhancement request in the bugzilla
and lets see if we can improve things soon?

Cheers,

Richard
Chris Larson - Sept. 5, 2012, 10:49 p.m.
On Wed, Sep 5, 2012 at 3:42 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2012-09-05 at 15:21 -0700, Chris Larson wrote:
>> On Wed, Sep 5, 2012 at 7:27 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > If we don't do this, users can get extremely confused errors since the sanity tests
>> > happen too late (after parsing) and don't see the warnings.
>>
>> Can you elaborate on this? This commit message is extremely unclear.
>> If there's an open bug in bugzilla or something that could be referred
>> to here, that'd be helpful.
>
> Sorry, I should have elaborated.
>
> Set an invalid MACHINE, try and build and you set all kinds of nasty
> warnings and no sensible message about what is wrong.
>
> Change LCONF_VERSION and you don't get the warning unless parsing
> completes which it may or may not depending on the kind of change.

That makes sense, thanks for the clarification.
Martin Jansa - Sept. 11, 2012, 6:39 a.m.
On Thu, Sep 6, 2012 at 12:42 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2012-09-05 at 15:21 -0700, Chris Larson wrote:
>> On Wed, Sep 5, 2012 at 7:27 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > If we don't do this, users can get extremely confused errors since the sanity tests
>> > happen too late (after parsing) and don't see the warnings.
>>
>> Can you elaborate on this? This commit message is extremely unclear.
>> If there's an open bug in bugzilla or something that could be referred
>> to here, that'd be helpful.
>
> Sorry, I should have elaborated.
>
> Set an invalid MACHINE, try and build and you set all kinds of nasty
> warnings and no sensible message about what is wrong.

Today with latest oe-core and bitbake I've set MACHINE=pitz (instead
of spitz) and the output doesn't look very tidy. Shows couple of DEBUG
entries even without debug enabled and then actuall ERROR, see
attached log.

Cheers,
Paul Eggleton - Sept. 11, 2012, 9:43 a.m.
On Tuesday 11 September 2012 08:39:39 Martin Jansa wrote:
> Today with latest oe-core and bitbake I've set MACHINE=pitz (instead
> of spitz) and the output doesn't look very tidy. Shows couple of DEBUG
> entries even without debug enabled and then actuall ERROR, see
> attached log.

I've just sent a patch to the bitbake list that fixes this, as well as a patch 
to the OE-Core list that gets rid of the not particularly useful tune check 
failures when MACHINE is invalid.

Cheers,
Paul

Patch

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 40d8211..8f42fca 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -333,20 +333,20 @@  def check_sanity(sanity_data):
     conf_version =  sanity_data.getVar('LOCALCONF_VERSION', True)
 
     if current_conf != conf_version:
-        messages = messages + "Your version of local.conf was generated from an older version of local.conf.sample and there have been updates made to this file. Please compare the two files and merge any changes before continuing.\nMatching the version numbers will remove this message.\n\"meld conf/local.conf conf/local.conf.sample\" is a good way to visualise the changes.\n"
+        messages = messages + "Your version of local.conf was generated from an older version of local.conf.sample and there have been updates made to this file. Please compare the two files and merge any changes before continuing.\nMatching the version numbers will remove this message.\n\"meld conf/local.conf ${COREBASE}/meta*/conf/local.conf.sample\" is a good way to visualise the changes.\n"
 
     # Check bblayers.conf is valid
     current_lconf = sanity_data.getVar('LCONF_VERSION', True)
     lconf_version = sanity_data.getVar('LAYER_CONF_VERSION', True)
     if current_lconf != lconf_version:
-        messages = messages + "Your version of bblayers.conf was generated from an older version of bblayers.conf.sample and there have been updates made to this file. Please compare the two files and merge any changes before continuing.\nMatching the version numbers will remove this message.\n\"meld conf/bblayers.conf conf/bblayers.conf.sample\" is a good way to visualise the changes.\n"
+        messages = messages + "Your version of bblayers.conf was generated from an older version of bblayers.conf.sample and there have been updates made to this file. Please compare the two files and merge any changes before continuing.\nMatching the version numbers will remove this message.\n\"meld conf/bblayers.conf ${COREBASE}/meta*/conf/bblayers.conf.sample\" is a good way to visualise the changes.\n"
 
     # If we have a site.conf, check it's valid
     if check_conf_exists("conf/site.conf", sanity_data):
         current_sconf = sanity_data.getVar('SCONF_VERSION', True)
         sconf_version = sanity_data.getVar('SITE_CONF_VERSION', True)
         if current_sconf != sconf_version:
-            messages = messages + "Your version of site.conf was generated from an older version of site.conf.sample and there have been updates made to this file. Please compare the two files and merge any changes before continuing.\nMatching the version numbers will remove this message.\n\"meld conf/site.conf conf/site.conf.sample\" is a good way to visualise the changes.\n"
+            messages = messages + "Your version of site.conf was generated from an older version of site.conf.sample and there have been updates made to this file. Please compare the two files and merge any changes before continuing.\nMatching the version numbers will remove this message.\n\"meld conf/site.conf ${COREBASE}/meta*/conf/site.conf.sample\" is a good way to visualise the changes.\n"
 
     assume_provided = sanity_data.getVar('ASSUME_PROVIDED', True).split()
     # Check user doesn't have ASSUME_PROVIDED = instead of += in local.conf
@@ -556,7 +556,7 @@  def check_sanity(sanity_data):
         messages = messages + "Error, you have a space in your COREBASE directory path. Please move the installation to a directory which doesn't include a space."
 
     if messages != "":
-        raise_sanity_error(messages, sanity_data)
+        raise_sanity_error(sanity_data.expand(messages), sanity_data)
 
 # Create a copy of the datastore and finalise it to ensure appends and 
 # overrides are set - the datastore has yet to be finalised at ConfigParsed
@@ -567,7 +567,7 @@  def copy_data(e):
 
 addhandler check_sanity_eventhandler
 python check_sanity_eventhandler() {
-    if bb.event.getName(e) == "BuildStarted" and e.data.getVar("BB_WORKERCONTEXT", True) != "1" and e.data.getVar("DISABLE_SANITY_CHECKS", True) != "1":
+    if bb.event.getName(e) == "ConfigParsed" and e.data.getVar("BB_WORKERCONTEXT", True) != "1" and e.data.getVar("DISABLE_SANITY_CHECKS", True) != "1":
         sanity_data = copy_data(e)
         check_sanity(sanity_data)
     elif bb.event.getName(e) == "SanityCheck":