Bug 5427 - Extend and tidy up layer sanity checking
Summary: Extend and tidy up layer sanity checking
Status: RESOLVED DUPLICATE of bug 10596
Alias: None
Product: OE-Core
Classification: Build System, Metadata & Runtime
Component: core (show other bugs)
Version: 1.6
Hardware: All Multiple
: Medium enhancement
Target Milestone: 2.3 M3
Assignee: Aníbal Limón
QA Contact:
URL:
Whiteboard:
Depends on:
Blocks: 10596
  Show dependency tree
 
Reported: 2013-10-30 17:38 UTC by Paul Eggleton
Modified: 2017-02-06 22:59 UTC (History)
9 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: Yes (doc changes required)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Eggleton 2013-10-30 17:38:04 UTC
Provide a script or inheritable class that will check if a layer is doing things it shouldn't; these would be more stringent checks than we could enable by default in sanity.bbclass/insane.bbclass. Suggested checks:

* Check that a BSP layer isn't making any changes when the layer is included but MACHINE is not set to point to one of the conf/machine/*.conf that the layer provides
* Similarly for DISTRO?
* Check that conf/distro and conf/machine don't appear in the same layer
* Check for ${PN} or ${P} usage in SRC_URI or S (should use ${BPN} or ${BP} instead to avoid breaking multilib)
* Check that BSP/distro layers don't provide their own linux-libc-headers
* Check that distro variables such as DISTRO_FEATURES are not being set in machine conf files
* Check that all patches have Signed-off-by and Upstream-Status ?

We do have recipe_sanity.bbclass and oelint.bbclass - these should be tidied up in the process.
Comment 1 Chong Lu 2014-06-20 09:30:41 UTC
Hi Paul,

I will fix oelint.bbclass and make it work.
And I will remove three checks there are LICENSE, MAINTAINER and PRIORITY.
LICENSE check is included in license.bbclass.
MAINTAINER and PRIORITY checks are unneeded.
I add two checks in this oelint.bbclass:

* Check for ${PN} or ${P} usage in SRC_URI or S (should use ${BPN} or ${BP} instead to avoid breaking multilib)
* Check that all patches have Signed-off-by and Upstream-Status


I don't understand what do you mean about three checks. Can you describe in details?

* Check that a BSP layer isn't making any changes when the layer is included but MACHINE is not set to point to one of the conf/machine/*.conf that the layer provides
* Similarly for DISTRO?
* Check that conf/distro and conf/machine don't appear in the same layer

Best Regards
Chong
Comment 2 Paul Eggleton 2014-12-09 14:30:03 UTC
So to update the status of this bug, some of the changes have now been made:

http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=7c85585bf6d436ee874e63cf375d4262ebe204a9
http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=207e94d4e1ffb7cae5310c16efc6224a7f0ce056
http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=268688a489add5ac3f378cf69d39f6c8c769549a

Chong and I have discussed the outstanding checks via email, here is an excerpt of one of my earlier messages answering the questions above:

----------- snip -----------

> * Check that a BSP layer isn't making any changes when the layer is
> included but MACHINE is not set to point to one of the
> conf/machine/*.conf that the layer provides

If a layer providing a machine is included but the machine is not selected in
MACHINE, there should be no changes to the configuration of the system; i.e.
any bbappends should have:

VARIABLENAME_machine = "value"

not:

VARIABLENAME = "value"

How you check this is a good question. You could examine it at a file level,
but it might be better to look through the available recipes and check the
variable history to see if anything within the layer is making a change to the
values. Here is an example of using tinfoil to look at variable values per
recipe:

  https://gist.github.com/bluelightning/6138980

You'd need to look elsewhere for examples on how to look at the variable
history, perhaps in the layer index:

http://git.yoctoproject.org/cgit/cgit.cgi/layerindex-web/tree/layerindex/recipeparse.py

> * Similarly for DISTRO?

Same thing as above but for distro layers.

> * Check that conf/distro and conf/machine don't appear in the same layer

A layer shouldn't be providing both a distro and a machine, those should be in
separate layers.

> * Check that BSP/distro layers don't provide their own linux-libc-headers

Probably the check here would be to look for a recipe that has 
linux-libc-headers in its PROVIDES.

----------- snip -----------
Comment 3 Scott Rifenbark 2015-06-02 15:39:42 UTC
Note about the docs - Not quite implemented but the likelihood is it'll be an additional class and then we would add something to the section where we talk about creating layers on how to validate them using this functionality - From Paul

Scott
Comment 4 Christopher Larson 2015-09-25 17:17:12 UTC
There've been a couple of creations of standalone scripts to check the machine / distro aspect of this, though I'm not sure if a standalone script is the ideal method or not.

I can't seem to find a link to the one I didn't write, but I had a simple one at https://github.com/kergoth/oe-test-scripts, see 'leaky-layer-tests' -- it just gathers a list of the appends from a layer, does a -S none on those with the layer not included, then does a -S printdiff on each (with empty SSTATE_DIR, to ensure it uses the sigdata in STAMPS_DIR) with the layer included. Previously, it did a bitbake-whatchanged. I only mention this for reference or possible inspiration, I doubt it'd be worth including as is.
Comment 5 Christopher Larson 2015-09-25 17:49:56 UTC
(In reply to comment #4)
> There've been a couple of creations of standalone scripts to check the
> machine / distro aspect of this, though I'm not sure if a standalone script
> is the ideal method or not.
> 
> I can't seem to find a link to the one I didn't write, but I had a simple
> one at https://github.com/kergoth/oe-test-scripts, see 'leaky-layer-tests'
> -- it just gathers a list of the appends from a layer, does a -S none on
> those with the layer not included, then does a -S printdiff on each (with
> empty SSTATE_DIR, to ensure it uses the sigdata in STAMPS_DIR) with the
> layer included. Previously, it did a bitbake-whatchanged. I only mention
> this for reference or possible inspiration, I doubt it'd be worth including
> as is.

Found it. For reference, Chong Lu submitted a 'check-layers.py' script to do some of this, I had some concerns with it. Also Chen Qi submitted layer_extra_sanity.bbclass as a version which wasn't as separate script, which I think probably wasn't ideal, but is worth mentioning for reference and posterity in this issue.
Comment 6 Scott Rifenbark 2015-09-25 20:04:07 UTC
For the documentation component of this bug, is it sufficient to document the layer_extra_sanity.bbclass in the reference manual?

Scott
Comment 7 Christopher Larson 2015-09-25 20:39:31 UTC
(In reply to comment #6)
> For the documentation component of this bug, is it sufficient to document
> the layer_extra_sanity.bbclass in the reference manual?

I don't think any of those mechanisms (the checks of the BSP and DISTRO layers) have been merged, so it wouldn't be appropriate to document them, but once an official mechanism makes it into oe-core, yes, we should definitely document how to use it. I wouldn't know which document would be most appropriate, I expect that depends on the final implementation selected, I only mentioned thme here for reference.
Comment 8 Scott Rifenbark 2015-09-25 20:45:33 UTC
Christopher, 

Thanks for the clarification and the heads up.  I will monitor.

Scott
Comment 9 Richard Purdie 2017-02-02 16:21:11 UTC
Anibal: I think this is related to another bug you're now working on. Could you look through this one and see if there are useful pieces to combine into that work, then make this one depend on that one and/or close it as appropriate please?
Comment 10 Aníbal Limón 2017-02-02 16:32:43 UTC
(In reply to comment #9)
> Anibal: I think this is related to another bug you're now working on. Could
> you look through this one and see if there are useful pieces to combine into
> that work, then make this one depend on that one and/or close it as
> appropriate please?

Sure, i'll take into account this bug.
Comment 11 Aníbal Limón 2017-02-02 17:14:53 UTC
Currently i'm working on standalone script (#10596) to make compatibility checking of layers, this will include the rules described in the Yocto Project Compabile page [1].

The unique difference with this bug is about the implementation as a bbclass instead of standalone script, but for the use case is better to have a standalone script because the checking needs to be made only at time of the process to be Yocto Project Compatible. I'm open to add some layer checking into insane class but needs to be discuss.

I will close this bug as duplicated, objections?

[1] https://www.yoctoproject.org/webform/yocto-project-compatible-registration
Comment 12 Scott Rifenbark 2017-02-02 18:34:44 UTC
Hi Anibal, 

Can you make sure that I am on the CC list for the bug that you are going to use as this issue?  I am assuming that it still could affect documentation.

thanks,
Scott
Comment 13 Aníbal Limón 2017-02-02 18:36:39 UTC
(In reply to comment #12)
> Hi Anibal, 
> 
> Can you make sure that I am on the CC list for the bug that you are going to
> use as this issue?  I am assuming that it still could affect documentation.
> 
> thanks,
> Scott

Yes Scott, np.
Comment 14 Paul Eggleton 2017-02-02 20:24:14 UTC
Standalone script is fine - but one thing I do want the script to be able to do is check more than just what's in YP compatible - i.e. there's a standard mode and "YP compatible mode" where what goes into the YP compatible checks is more strictly defined (and documented), and the standard mode is a superset of that. That way the script can live in OE-core and it will be easier to accept patches to add additional checks that are outside the scope of the YP compatible programme.
Comment 15 Aníbal Limón 2017-02-06 22:59:06 UTC
Resolve as duplicated in working on #10596.

*** This bug has been marked as a duplicate of bug 10596 ***