| 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
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.
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
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.
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,
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":
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> ---