v4 or so [PATCH 0/3] sanity.bbclass and friends: toolchain sanity

v4 or so [PATCH 0/3] sanity.bbclass and friends: toolchain sanity

[Peter Seebach]

And yes, I'm aware that "toolchain sanity checks" is sort of a
self-defeating concept.

Okay, cleanups:
* Now checking MULTILIB_VARIANTS, not MULTILIBS.  Simpler and less indented.
* Added a third commit to do the BBCLASSEXTEND checks since those seem like
  a good idea, and it's less work to do it than to create a todo item.
* Did you know that, despite a lot of this being Python, you can't use
  triple-quote strings in documentation.conf?  I didn't, and apparently
  that "fix" went in untested.  (Also the addition of duplicate but indented
  copies of two later lines, which didn't parse.  Looks like I got a bit
  careless with my cut and paste there.)

The following changes since commit 35b5fb2dd2131d4c7dc6635c14c6e08ea6926457:
  Saul Wold (1):
        eglibc-package: remove /var

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib seebs/toolchains
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=seebs/toolchains

Peter Seebach (3):
  conf/machine: Clean up configuration values.
  sanity.bbclass: Implement initial toolchain sanity checks
  sanity.bbclass: Check for bogus values in BBCLASSEXTEND

 meta/classes/sanity.bbclass                      |   86 ++++++++++++++++++++++
 meta/conf/documentation.conf                     |    9 ++
 meta/conf/machine/include/README                 |    4 +
 meta/conf/machine/include/arm/arch-armv5-dsp.inc |    1 +
 meta/conf/machine/include/arm/arch-armv7a.inc    |    2 +-
 meta/conf/machine/include/ia32/arch-ia32.inc     |    2 +-
 meta/conf/machine/include/mips/arch-mips.inc     |    6 +-
 meta/conf/machine/include/tune-c3.inc            |    2 +-
 8 files changed, 106 insertions(+), 6 deletions(-)

[PATCH 1/3] conf/machine: Clean up configuration values.

[Peter Seebach]

This cleans up and/or corrects a few values from machine includes
for consistency with future toolchain sanity checks, and also adds
the TUNEVALID and TUNECONFLICTS to documentation.conf.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 meta/conf/documentation.conf                     |    3 +++
 meta/conf/machine/include/README                 |    4 ++++
 meta/conf/machine/include/arm/arch-armv5-dsp.inc |    1 +
 meta/conf/machine/include/arm/arch-armv7a.inc    |    2 +-
 meta/conf/machine/include/ia32/arch-ia32.inc     |    2 +-
 meta/conf/machine/include/mips/arch-mips.inc     |    6 +++---
 meta/conf/machine/include/tune-c3.inc            |    2 +-
 7 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf
index f4d6241..3e40a77 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -34,6 +34,9 @@ TARGET_CC_ARCH[doc] = "FIXME"
 TARGET_FPU[doc] = "Floating point option (mostly for FPU-less systems), can be 'soft' or empty \
 for hardware floating point instructions."
 
+TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
+TUNECONFLICTS[doc] = "List of conflicting features for a given feature."
+
 ASSUME_PROVIDED[doc] = "List of packages (recipes actually) which are assumed to be implicitly available.\
  These packages won't be built by bitbake."
 ASSUME_SHLIBS[doc] = "List of shlib:package[_version] mappings. Useful for lib packages in ASSUME_PROVIDED,\
diff --git a/meta/conf/machine/include/README b/meta/conf/machine/include/README
index 6a3a63d..e4b59c9 100644
--- a/meta/conf/machine/include/README
+++ b/meta/conf/machine/include/README
@@ -24,6 +24,10 @@ TUNEVALID[feature] - The <feature> is defined with a human readable
 explanation for what it does.  All architectural, cpu, abi, etc tuning 
 features must be defined using TUNEVALID.
 
+TUNECONFLICTS[feature] - A list of features which conflict with <feature>.
+New sanity checks will try to reject combinations in which a single
+tuning ends up with features which conflict with each other.
+
 TUNE_FEATURES - This is automatically defined as TUNE_FEATURES_tune-<tune>.
 See TUNE_FEATURES_tune-<tune> for more information.
 
diff --git a/meta/conf/machine/include/arm/arch-armv5-dsp.inc b/meta/conf/machine/include/arm/arch-armv5-dsp.inc
index 9f03a0f..0f64562 100644
--- a/meta/conf/machine/include/arm/arch-armv5-dsp.inc
+++ b/meta/conf/machine/include/arm/arch-armv5-dsp.inc
@@ -1,4 +1,5 @@
 ARMPKGSFX_DSP = "${@bb.utils.contains("TUNE_FEATURES", [ "armv5", "dsp" ], "e", "", d)}"
+TUNEVALID[dsp] = "ARM DSP functionality"
 
 require conf/machine/include/arm/arch-armv5.inc
 
diff --git a/meta/conf/machine/include/arm/arch-armv7a.inc b/meta/conf/machine/include/arm/arch-armv7a.inc
index 629960d..c90aff5 100644
--- a/meta/conf/machine/include/arm/arch-armv7a.inc
+++ b/meta/conf/machine/include/arm/arch-armv7a.inc
@@ -2,7 +2,7 @@ DEFAULTTUNE ?= "armv7a"
 
 ARMPKGARCH ?= "armv7a"
 
-TUNEVALID[armv7-a] = "Enable instructions for ARMv7-a"
+TUNEVALID[armv7a] = "Enable instructions for ARMv7-a"
 TUNE_CONFLICTS[armv7a] = "armv4 armv5 armv6 armv7"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "armv7a", "-march=armv7-a -fno-tree-vectorize", "", d)}"
 MACHINEOVERRIDES .= "${@bb.utils.contains("TUNE_FEATURES", "armv7a", ":armv7a", "" ,d)}"
diff --git a/meta/conf/machine/include/ia32/arch-ia32.inc b/meta/conf/machine/include/ia32/arch-ia32.inc
index a5dae88..15f67d7 100644
--- a/meta/conf/machine/include/ia32/arch-ia32.inc
+++ b/meta/conf/machine/include/ia32/arch-ia32.inc
@@ -27,7 +27,7 @@ TUNE_ASARGS += "${@bb.utils.contains("TUNE_FEATURES", "mx32", "-x32", "", d)}"
 
 # ELF64 ABI
 TUNEVALID[m64] = "IA32e (x86_64) ELF64 standard ABI"
-TUNECONFLICT[m64] = "m32 mx32"
+TUNECONFLICTS[m64] = "m32 mx32"
 TUNE_ARCH .= "${@bb.utils.contains("TUNE_FEATURES", "m64", "${X86ARCH64}", "" ,d)}"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "m64", "-m64", "", d)}"
 
diff --git a/meta/conf/machine/include/mips/arch-mips.inc b/meta/conf/machine/include/mips/arch-mips.inc
index 8758ecd..9f12920 100644
--- a/meta/conf/machine/include/mips/arch-mips.inc
+++ b/meta/conf/machine/include/mips/arch-mips.inc
@@ -12,15 +12,15 @@ TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "bigendian", "-meb", "-mel
 
 # ABI flags
 TUNEVALID[o32] = "MIPS o32 ABI"
-TUNECONFLICT[o32] = "n32 n64"
+TUNECONFLICTS[o32] = "n32 n64"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "o32", "-mabi=32", "", d)}"
 
 TUNEVALID[n32] = "MIPS64 n32 ABI"
-TUNECONFLICT[n32] = "o32 n64"
+TUNECONFLICTS[n32] = "o32 n64"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "n32", "-mabi=n32", "", d)}"
 
 TUNEVALID[n64] = "MIPS64 n64 ABI"
-TUNECONFLICT[n64] = "o32 n32"
+TUNECONFLICTS[n64] = "o32 n32"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "n64", "-mabi=64", "", d)}"
 
 # Floating point
diff --git a/meta/conf/machine/include/tune-c3.inc b/meta/conf/machine/include/tune-c3.inc
index 06fac8f..79bb67b 100644
--- a/meta/conf/machine/include/tune-c3.inc
+++ b/meta/conf/machine/include/tune-c3.inc
@@ -1,7 +1,7 @@
 require conf/machine/include/ia32/arch-ia32.inc
 
 TUNEVALID[c3] = "VIA Cyrix III or VIA C3 specific optimizations"
-TUNECONFLICT[c3] = "m64 mx32"
+TUNECONFLICTS[c3] = "m64 mx32"
 TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "c3", "-march=c3 -mtune=c3", "", d)}"
 
 AVAILTUNES += "c3"
-- 
1.7.0.4

[PATCH 2/3] sanity.bbclass: Implement initial toolchain sanity checks

[Peter Seebach]

This introduces a sanity check for the toolchain, which verifies
each tuning (including any multilibs), producing meaningful diagnostics
for problems, and also provides some higher-level tuning features.

The TUNEVALID and TUNECONFLICT/TUNECONFLICTS settings were not
implemented.  Listed one or two missing features in TUNEVALID,
also (in a previous patch) fixed the references to
features which didn't exist.

This patch also provides a whitelisting mechanism (which is completely
unused) to allow vendors providing prebuilt toolchain components to
restrict tunings to those based on or compatible with a particular ABI.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 meta/classes/sanity.bbclass  |   73 ++++++++++++++++++++++++++++++++++++++++++
 meta/conf/documentation.conf |    6 +++
 2 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 687ddeb..635049e 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -11,6 +11,76 @@ def raise_sanity_error(msg):
     
     %s""" % msg)
 
+# Check a single tune for validity.
+def check_toolchain_tune(data, tune, multilib):
+    tune_errors = []
+    if not tune:
+        return "No tuning found for %s multilib." % multilib
+    bb.debug(2, "Sanity-checking tuning '%s' (%s) features:" % (tune, multilib))
+    features = (data.getVar("TUNE_FEATURES_tune-%s" % tune, True) or "").split()
+    if not features:
+        return "Tuning '%s' has no defined features, and cannot be used." % tune
+    valid_tunes = data.getVarFlags('TUNEVALID') or {}
+    conflicts = data.getVarFlags('TUNECONFLICTS') or {}
+    # [doc] is the documentation for the variable, not a real feature
+    if 'doc' in valid_tunes:
+        del valid_tunes['doc']
+    if 'doc' in conflicts:
+        del conflicts['doc']
+    for feature in features:
+        if feature in conflicts:
+            for conflict in conflicts[feature].split():
+                if conflict in features:
+                    tune_errors.append("Feature '%s' conflicts with '%s'." %
+                        (feature, conflict))
+        if feature in valid_tunes:
+            bb.debug(2, "  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)
+    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
+    override = data.getVar("TUNEABI_OVERRIDE", True) or ''
+    if whitelist:
+        tuneabi = data.getVar("TUNEABI_tune-%s" % tune, True) or ''
+        if not tuneabi:
+            tuneabi = tune
+        if True not in [x in whitelist.split() for x in tuneabi.split()]:
+            tune_errors.append("Tuning '%s' (%s) cannot be used with any supported tuning/ABI." %
+                (tune, tuneabi))
+    if tune_errors:
+        return "Tuning '%s' has the following errors:\n" + '\n'.join(tune_errors)
+
+def check_toolchain(data):
+    tune_error_set = []
+    deftune = data.getVar("DEFAULTTUNE", True)
+    tune_errors = check_toolchain_tune(data, deftune, 'default')
+    if tune_errors:
+        tune_error_set.append(tune_errors)
+
+    multilibs = (data.getVar("MULTILIB_VARIANTS", True) or "").split()
+    if multilibs:
+        seen_libs = []
+        seen_tunes = []
+        for lib in multilibs:
+            if lib in seen_libs:
+                tune_error_set.append("The multilib '%s' appears more than once." % lib)
+            else:
+                seen_libs.append(lib)
+            tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % lib, True)
+            if tune in seen_tunes:
+                tune_error_set.append("The tuning '%s' appears in more than one multilib." % tune)
+            else:
+                seen_libs.append(tune)
+            if tune == deftune:
+                tune_error_set.append("Multilib '%s' (%s) is also the default tuning." % (lib, deftune))
+            else:
+                tune_errors = check_toolchain_tune(data, tune, lib)
+            if tune_errors:
+                tune_error_set.append(tune_errors)
+    if tune_error_set:
+        return "Toolchain tunings invalid:\n" + '\n'.join(tune_error_set)
+
+    return ""
+
 def check_conf_exists(fn, data):
     bbpath = []
     fn = data.expand(fn)
@@ -327,6 +397,9 @@ def check_sanity(e):
         messages = messages + pseudo_msg + '\n'
 
     check_supported_distro(e)
+    toolchain_msg = check_toolchain(e.data)
+    if toolchain_msg != "":
+        messages = messages + toolchain_msg + '\n'
 
     # Check if DISPLAY is set if IMAGETEST is set
     if not data.getVar( 'DISPLAY', e.data, True ) and data.getVar( 'IMAGETEST', e.data, True ) == 'qemu':
diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf
index 3e40a77..004a16c 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -36,6 +36,12 @@ for hardware floating point instructions."
 
 TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
 TUNECONFLICTS[doc] = "List of conflicting features for a given feature."
+TUNEABI[doc] = "An underlying ABI used by a particular tuning in a given \
+toolchain layer.  This feature allows providers using prebuilt \
+libraries to check compatibility of a tuning against their selection \
+of libraries."
+TUNEABI_WHITELIST[doc] = "A whitelist of permissible TUNEABI values; if unset, all are allowed."
+TUNEABI_OVERRIDE[doc] = "If set, ignores TUNEABI_WHITELIST."
 
 ASSUME_PROVIDED[doc] = "List of packages (recipes actually) which are assumed to be implicitly available.\
  These packages won't be built by bitbake."
-- 
1.7.0.4

[PATCH 3/3] sanity.bbclass: Check for bogus values in BBCLASSEXTEND

[Peter Seebach]

Suggestion that came out of my sanity-checks of multilibs; verify
that stuff in BBCLASSEXTEND is valid.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 meta/classes/sanity.bbclass |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 635049e..9f3f1d5 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -81,6 +81,15 @@ def check_toolchain(data):
 
     return ""
 
+def check_bbclassextend(data):
+    allowed = ('native', 'nativesdk', 'multilib', 'cross')
+    extends = (data.getVar("BBCLASSEXTEND", True) or "").split()
+    errors = []
+    for pair in [x.split(':') for x in extends]:
+        if pair[0] not in allowed:
+            errors.append('BBCLASSEXTEND has invalid value: %s' % pair[0])
+    return '\n'.join(errors)
+
 def check_conf_exists(fn, data):
     bbpath = []
     fn = data.expand(fn)
@@ -401,6 +410,10 @@ def check_sanity(e):
     if toolchain_msg != "":
         messages = messages + toolchain_msg + '\n'
 
+    bbclassextend_msg = check_bbclassextend(e.data)
+    if bbclassextend_msg != "":
+        messages = messages + bbclassextend_msg + '\n'
+
     # Check if DISPLAY is set if IMAGETEST is set
     if not data.getVar( 'DISPLAY', e.data, True ) and data.getVar( 'IMAGETEST', e.data, True ) == 'qemu':
         messages = messages + 'qemuimagetest needs a X desktop to start qemu, please set DISPLAY correctly (e.g. DISPLAY=:1.0)\n'
-- 
1.7.0.4

Re: [PATCH 3/3] sanity.bbclass: Check for bogus values in BBCLASSEXTEND

[Richard Purdie]

On Tue, 2012-05-01 at 20:45 -0500, Peter Seebach wrote:
> Suggestion that came out of my sanity-checks of multilibs; verify
> that stuff in BBCLASSEXTEND is valid.
> 
> Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
> ---
>  meta/classes/sanity.bbclass |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 635049e..9f3f1d5 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -81,6 +81,15 @@ def check_toolchain(data):
>  
>      return ""
>  
> +def check_bbclassextend(data):
> +    allowed = ('native', 'nativesdk', 'multilib', 'cross')
> +    extends = (data.getVar("BBCLASSEXTEND", True) or "").split()
> +    errors = []
> +    for pair in [x.split(':') for x in extends]:
> +        if pair[0] not in allowed:
> +            errors.append('BBCLASSEXTEND has invalid value: %s' % pair[0])
> +    return '\n'.join(errors)
> +
>  def check_conf_exists(fn, data):
>      bbpath = []
>      fn = data.expand(fn)
> @@ -401,6 +410,10 @@ def check_sanity(e):
>      if toolchain_msg != "":
>          messages = messages + toolchain_msg + '\n'
>  
> +    bbclassextend_msg = check_bbclassextend(e.data)
> +    if bbclassextend_msg != "":
> +        messages = messages + bbclassextend_msg + '\n'
> +
>      # Check if DISPLAY is set if IMAGETEST is set
>      if not data.getVar( 'DISPLAY', e.data, True ) and data.getVar( 'IMAGETEST', e.data, True ) == 'qemu':
>          messages = messages + 'qemuimagetest needs a X desktop to start qemu, please set DISPLAY correctly (e.g. DISPLAY=:1.0)\n'

FWIW, this isn't going to work. Its checking the sanity in
"configuration" space, pre recipe parse so it won't pick up on the
errors we want it to.

So whilst its well intentioned, it isn't going to work as designed and
we'll have to rethink it. I can't think of a good way to fix it right
now, we don't really have a structure for recipe checks at this point.

Cheers,

Richard

Re: [PATCH 3/3] sanity.bbclass: Check for bogus values in BBCLASSEXTEND

[Peter Seebach]

On Wed, 9 May 2012 21:59:49 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> FWIW, this isn't going to work. Its checking the sanity in
> "configuration" space, pre recipe parse so it won't pick up on the
> errors we want it to.
> 
> So whilst its well intentioned, it isn't going to work as designed and
> we'll have to rethink it. I can't think of a good way to fix it right
> now, we don't really have a structure for recipe checks at this point.

Oh, that has to be per-recipe?  I thought BBCLASSEXTEND was top-level.

Adding recipe checks of some sort seems useful, but that is a bigger
project, so I guess we can drop that.  Note that this was in sgw's
consolidated pull request, too.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.

Re: v4 or so [PATCH 0/3] sanity.bbclass and friends: toolchain sanity

[Saul Wold]

On 05/01/2012 06:45 PM, Peter Seebach wrote:
> And yes, I'm aware that "toolchain sanity checks" is sort of a
> self-defeating concept.
>
> Okay, cleanups:
> * Now checking MULTILIB_VARIANTS, not MULTILIBS.  Simpler and less indented.
> * Added a third commit to do the BBCLASSEXTEND checks since those seem like
>    a good idea, and it's less work to do it than to create a todo item.
> * Did you know that, despite a lot of this being Python, you can't use
>    triple-quote strings in documentation.conf?  I didn't, and apparently
>    that "fix" went in untested.  (Also the addition of duplicate but indented
>    copies of two later lines, which didn't parse.  Looks like I got a bit
>    careless with my cut and paste there.)
>
> The following changes since commit 35b5fb2dd2131d4c7dc6635c14c6e08ea6926457:
>    Saul Wold (1):
>          eglibc-package: remove /var
>
> are available in the git repository at:
>
>    git://git.yoctoproject.org/poky-contrib seebs/toolchains
>    http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=seebs/toolchains
>
> Peter Seebach (3):
>    conf/machine: Clean up configuration values.
>    sanity.bbclass: Implement initial toolchain sanity checks
These 2 have been merged into OE-Core

>    sanity.bbclass: Check for bogus values in BBCLASSEXTEND
>
RP commented on this one.

Thanks
	Sau!


>   meta/classes/sanity.bbclass                      |   86 ++++++++++++++++++++++
>   meta/conf/documentation.conf                     |    9 ++
>   meta/conf/machine/include/README                 |    4 +
>   meta/conf/machine/include/arm/arch-armv5-dsp.inc |    1 +
>   meta/conf/machine/include/arm/arch-armv7a.inc    |    2 +-
>   meta/conf/machine/include/ia32/arch-ia32.inc     |    2 +-
>   meta/conf/machine/include/mips/arch-mips.inc     |    6 +-
>   meta/conf/machine/include/tune-c3.inc            |    2 +-
>   8 files changed, 106 insertions(+), 6 deletions(-)
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>

Re: v4 or so [PATCH 0/3] sanity.bbclass and friends: toolchain sanity

[Peter Seebach]

On Wed, 9 May 2012 16:32:17 -0700
Saul Wold <sgw@linux.intel.com> wrote:

> >    sanity.bbclass: Check for bogus values in BBCLASSEXTEND
> >  
> RP commented on this one.

I'm gonna drop that for now, I think.  I am looking a little at
per-package sanity checking, but it turns out to be harder than I would
ideally like.

-s
-- 
Listen, get this.  Nobody with a good compiler needs to be justified.