[v3] [PATCH 0/2] toolchain sanity checks, revised

[v3] [PATCH 0/2] toolchain sanity checks, revised

[Peter Seebach]

While preparing to argue persuasively that the conflict check was as
simple as it could reasonably be, I discovered that this was in fact
totally wrong.  It's now (I hope) a lot clearer, and probably even
more efficient.

The following changes since commit b8dc9182ed8959ce5a6aa285c3c7f78df2432ac3:
  Richard Purdie (1):
        qt-4.8.1: Fix compile failures on qemux86

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 (2):
  tune-sh4.inc: Fix spelling of big-endian feature set
  sanity.bbclass: Implement initial toolchain sanity checks

 meta/classes/sanity.bbclass                      |   67 ++++++++++++++++++++++
 meta/conf/documentation.conf                     |    6 ++
 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 +-
 meta/conf/machine/include/tune-sh3.inc           |    2 +-
 meta/conf/machine/include/tune-sh4.inc           |    4 +-
 meta/conf/machine/include/tune-xscale.inc        |    2 +-
 11 files changed, 88 insertions(+), 10 deletions(-)

[PATCH 1/2] tune-sh4.inc: Fix spelling of big-endian feature set

[Peter Seebach]

In tune-sh3, tune-xscale, and tune-sh4, several FEATURES lines referred
to nonexistent features like "sh3eb" when they should have referred to "sh3
bigendian" or the like.  Caught by the TUNEVALID sanity check.

Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
---
 meta/conf/machine/include/tune-sh3.inc    |    2 +-
 meta/conf/machine/include/tune-sh4.inc    |    4 ++--
 meta/conf/machine/include/tune-xscale.inc |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/meta/conf/machine/include/tune-sh3.inc b/meta/conf/machine/include/tune-sh3.inc
index 816c9c4..70bb10b 100644
--- a/meta/conf/machine/include/tune-sh3.inc
+++ b/meta/conf/machine/include/tune-sh3.inc
@@ -11,7 +11,7 @@ TUNE_ARCH_tune-sh3 = "sh3"
 TUNE_PKGARCH_tune-sh3 = "sh3"
 PACKAGE_EXTRA_ARCHS_tune-sh3 = "sh sh3"
 
-TUNE_FEATURES_tune-sh3eb = "sh3eb"
+TUNE_FEATURES_tune-sh3eb = "sh3 bigendian"
 TUNE_ARCH_tune-sh3eb = "sh3eb"
 TUNE_PKGARCH_tune-sh3eb = "sh3eb"
 PACKAGE_EXTRA_ARCHS_tune-sh3eb = "sheb sh3eb"
diff --git a/meta/conf/machine/include/tune-sh4.inc b/meta/conf/machine/include/tune-sh4.inc
index f971f23..00cfc3d 100644
--- a/meta/conf/machine/include/tune-sh4.inc
+++ b/meta/conf/machine/include/tune-sh4.inc
@@ -18,7 +18,7 @@ TUNE_ARCH_tune-sh4 = "sh4"
 TUNE_PKGARCH_tune-sh4 = "sh4"
 PACKAGE_EXTRA_ARCHS_tune-sh4 = "sh sh4"
 
-TUNE_FEATURES_tune-sh4eb = "sh4eb"
+TUNE_FEATURES_tune-sh4eb = "sh4 bigendian"
 TUNE_ARCH_tune-sh4eb = "sh4eb"
 TUNE_PKGARCH_tune-sh4eb = "sh4eb"
 PACKAGE_EXTRA_ARCHS_tune-sh4eb = "sheb sh4eb"
@@ -28,7 +28,7 @@ TUNE_ARCH_tune-sh4a = "sh4"
 TUNE_PKGARCH_tune-sh4a = "sh4a"
 PACKAGE_EXTRA_ARCHS_tune-sh4a = "sh sh4 sh4a"
 
-TUNE_FEATURES_tune-sh4aeb = "sh4aeb"
+TUNE_FEATURES_tune-sh4aeb = "sh4a bigendian"
 TUNE_ARCH_tune-sh4aeb = "sh4eb"
 TUNE_PKGARCH_tune-sh4aeb = "sh4aeb"
 PACKAGE_EXTRA_ARCHS_tune-sh4aeb = "sheb sh4eb sh4aeb"
diff --git a/meta/conf/machine/include/tune-xscale.inc b/meta/conf/machine/include/tune-xscale.inc
index 54f3727..80a3683 100644
--- a/meta/conf/machine/include/tune-xscale.inc
+++ b/meta/conf/machine/include/tune-xscale.inc
@@ -10,7 +10,7 @@ TUNE_FEATURES_tune-xscale = "${TUNE_FEATURES_tune-armv5te} xscale"
 PACKAGE_EXTRA_ARCHS_tune-xscale = "${PACKAGE_EXTRA_ARCHS_tune-armv5te}"
 
 AVAILTUNES += "xscale-be"
-TUNE_FEATURES_tune-xscale-be = "${TUNE_FEATURES_tune-armv5teb} xscale-be"
+TUNE_FEATURES_tune-xscale-be = "${TUNE_FEATURES_tune-armv5teb} xscale bigendian"
 PACKAGE_EXTRA_ARCHS_tune-xscale-be = "${PACKAGE_EXTRA_ARCHS_tune-armv5teb}"
 
 # webkit-gtk has alignment issues with double instructions on armv5 so
-- 
1.7.0.4

[PATCH 2/2] 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, and there were some loose ends (like not knowing how
the conflict one was spelled).  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                      |   67 ++++++++++++++++++++++
 meta/conf/documentation.conf                     |    6 ++
 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, 84 insertions(+), 6 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 687ddeb..c77e675 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -11,6 +11,70 @@ 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 or tune == "":
+        return "No tuning found for %s multilib." % multilib
+    bb.note("Sanity-checking tuning '%s' (%s) features:" % (tune, multilib))
+    features = data.getVar("TUNE_FEATURES_tune-%s" % tune, True) or ""
+    if features == '':
+        return "Tuning '%s' has no defined features, and cannot be used." % tune
+    features = features.split()
+    valid_tunes = data.getVarFlags('TUNEVALID') or ""
+    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
+    split_conflicts = {}
+    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.note("  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)
+    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
+    if whitelist != '':
+        override = data.getVar("TUNEABI_OVERRIDE", True) or ''
+        if not override:
+            tuneabi = data.getVar("TUNEABI_tune-%s" % tune, True) or ""
+            if 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("MULTILIBS", True) or ""
+    if multilibs != "":
+        for pairs in [x.split(':') for x in multilibs.split()]:
+            if pairs[0] != 'multilib':
+                bb.warn("Got an unexpected '%s' in MULTILIBS." % pairs[0])
+            else:
+                if pairs[1] == 'lib':
+                    tune_error_set.append("The multilib 'lib' was specified, but that causes parse errors.")
+                else:
+                    tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], True)
+                    if tune == deftune:
+                        bb.warn("Multilib '%s' (%s) is also the default tuning." % (pairs[1], deftune))
+                    else:
+                        tune_errors = check_toolchain_tune(data, tune, pairs[1])
+                    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 +391,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 f4d6241..99c16fb 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -34,6 +34,12 @@ 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."
 
+# WARNING:  Because the flags on these have semantic implecations,
+# they must not actually be defined.
+#TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
+#TUNECONFLICTS[doc] = "List of conflicting features for a given feature."
+TUNEABI[doc] = "A base ABI used by a given tuning, used with prebuilt binaries."
+
 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

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

[Mark Hatle]

See comment inline below..

On 4/30/12 3:33 PM, Peter Seebach wrote:
> 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, and there were some loose ends (like not knowing how
> the conflict one was spelled).  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                      |   67 ++++++++++++++++++++++
>   meta/conf/documentation.conf                     |    6 ++
>   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, 84 insertions(+), 6 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 687ddeb..c77e675 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -11,6 +11,70 @@ 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 or tune == "":
> +        return "No tuning found for %s multilib." % multilib
> +    bb.note("Sanity-checking tuning '%s' (%s) features:" % (tune, multilib))
> +    features = data.getVar("TUNE_FEATURES_tune-%s" % tune, True) or ""
> +    if features == '':
> +        return "Tuning '%s' has no defined features, and cannot be used." % tune
> +    features = features.split()
> +    valid_tunes = data.getVarFlags('TUNEVALID') or ""
> +    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
> +    split_conflicts = {}
> +    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.note("  %s: %s" % (feature, valid_tunes[feature]))
> +        else:

Is the above debugging?  (I suspect it is)  I suggest the following...

            if feature not in valid_tunes:
> +            tune_errors.append("Feature '%s' is not defined." % feature)
> +    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
> +    if whitelist != '':
> +        override = data.getVar("TUNEABI_OVERRIDE", True) or ''
> +        if not override:
> +            tuneabi = data.getVar("TUNEABI_tune-%s" % tune, True) or ""
> +            if 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("MULTILIBS", True) or ""
> +    if multilibs != "":
> +        for pairs in [x.split(':') for x in multilibs.split()]:
> +            if pairs[0] != 'multilib':
> +                bb.warn("Got an unexpected '%s' in MULTILIBS." % pairs[0])
> +            else:
> +                if pairs[1] == 'lib':
> +                    tune_error_set.append("The multilib 'lib' was specified, but that causes parse errors.")
> +                else:
> +                    tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], True)
> +                    if tune == deftune:
> +                        bb.warn("Multilib '%s' (%s) is also the default tuning." % (pairs[1], deftune))
> +                    else:
> +                        tune_errors = check_toolchain_tune(data, tune, pairs[1])
> +                    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 +391,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 f4d6241..99c16fb 100644
> --- a/meta/conf/documentation.conf
> +++ b/meta/conf/documentation.conf
> @@ -34,6 +34,12 @@ 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."
>
> +# WARNING:  Because the flags on these have semantic implecations,
> +# they must not actually be defined.
> +#TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
> +#TUNECONFLICTS[doc] = "List of conflicting features for a given feature."
> +TUNEABI[doc] = "A base ABI used by a given tuning, used with prebuilt binaries."
> +
>   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"

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

[Peter Seebach]

On Mon, 30 Apr 2012 15:42:44 -0500
Mark Hatle <mark.hatle@windriver.com> wrote:

> Is the above debugging?  (I suspect it is)  I suggest the following...

Actually, that one's intentional.  I originally did it for debugging,
but I thought it was super convenient to actually get a list of the CPU
feature set bitbake thought it was using.  We can't tell you whether
you picked the right features, but we can tell you what they are.

So for instance, with a qemux86 and a core2 lib32:

NOTE: Sanity-checking tuning 'x86-64' (default) features:
NOTE:   checking for conflicts: m64
NOTE:   checking for conflicts with: m32
NOTE:   m64: IA32e (x86_64) ELF64 standard ABI
NOTE: Sanity-checking tuning 'core2' (lib32) features:
NOTE:   m32: IA32 ELF32 standard ABI
NOTE:   core2: Enable core2 specific processor optimizations

I found this really handy, and left it there on purpose.  I could take
it out, and/or move it to somewhere else, but I really do like having
that information appear.

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

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

[Richard Purdie]

On Mon, 2012-04-30 at 15:48 -0500, Peter Seebach wrote:
> On Mon, 30 Apr 2012 15:42:44 -0500
> Mark Hatle <mark.hatle@windriver.com> wrote:
> 
> > Is the above debugging?  (I suspect it is)  I suggest the following...
> 
> Actually, that one's intentional.  I originally did it for debugging,
> but I thought it was super convenient to actually get a list of the CPU
> feature set bitbake thought it was using.  We can't tell you whether
> you picked the right features, but we can tell you what they are.
> 
> So for instance, with a qemux86 and a core2 lib32:
> 
> NOTE: Sanity-checking tuning 'x86-64' (default) features:
> NOTE:   checking for conflicts: m64
> NOTE:   checking for conflicts with: m32
> NOTE:   m64: IA32e (x86_64) ELF64 standard ABI
> NOTE: Sanity-checking tuning 'core2' (lib32) features:
> NOTE:   m32: IA32 ELF32 standard ABI
> NOTE:   core2: Enable core2 specific processor optimizations
> 
> I found this really handy, and left it there on purpose.  I could take
> it out, and/or move it to somewhere else, but I really do like having
> that information appear.

I'd change these to debug messages. Users don't need this on the console
on every run. There is already a summary of the active tune options
displayed in the build configuration banner if I remember correctly.

Cheers,

Richard

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

[Koen Kooi]

Op 1 mei 2012, om 12:21 heeft Richard Purdie het volgende geschreven:

> On Mon, 2012-04-30 at 15:48 -0500, Peter Seebach wrote:
>> On Mon, 30 Apr 2012 15:42:44 -0500
>> Mark Hatle <mark.hatle@windriver.com> wrote:
>> 
>>> Is the above debugging?  (I suspect it is)  I suggest the following...
>> 
>> Actually, that one's intentional.  I originally did it for debugging,
>> but I thought it was super convenient to actually get a list of the CPU
>> feature set bitbake thought it was using.  We can't tell you whether
>> you picked the right features, but we can tell you what they are.
>> 
>> So for instance, with a qemux86 and a core2 lib32:
>> 
>> NOTE: Sanity-checking tuning 'x86-64' (default) features:
>> NOTE:   checking for conflicts: m64
>> NOTE:   checking for conflicts with: m32
>> NOTE:   m64: IA32e (x86_64) ELF64 standard ABI
>> NOTE: Sanity-checking tuning 'core2' (lib32) features:
>> NOTE:   m32: IA32 ELF32 standard ABI
>> NOTE:   core2: Enable core2 specific processor optimizations
>> 
>> I found this really handy, and left it there on purpose.  I could take
>> it out, and/or move it to somewhere else, but I really do like having
>> that information appear.
> 
> I'd change these to debug messages. Users don't need this on the console
> on every run. There is already a summary of the active tune options
> displayed in the build configuration banner if I remember correctly.

indeed:

BB_VERSION        = "1.15.1"
TARGET_ARCH       = "arm"
TARGET_OS         = "linux-gnueabi"
MACHINE           = "beagleboard"
DISTRO            = "angstrom"
DISTRO_VERSION    = "v2012.05-core"
TUNE_FEATURES     = "armv7a vfp neon cortexa8"
TARGET_FPU        = "vfp-neon"

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

[Richard Purdie]

On Mon, 2012-04-30 at 15:33 -0500, Peter Seebach wrote:
> 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, and there were some loose ends (like not knowing how
> the conflict one was spelled).  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                      |   67 ++++++++++++++++++++++
>  meta/conf/documentation.conf                     |    6 ++
>  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, 84 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 687ddeb..c77e675 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -11,6 +11,70 @@ 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 or tune == "":

"if not tune:"

will suffice here ("" equates to false)

> +        return "No tuning found for %s multilib." % multilib
> +    bb.note("Sanity-checking tuning '%s' (%s) features:" % (tune, multilib))

bb.debug(2, xxx) please

> +    features = data.getVar("TUNE_FEATURES_tune-%s" % tune, True) or ""
> +    if features == '':
> +        return "Tuning '%s' has no defined features, and cannot be used." % tune
> +    features = features.split()

We'd usually have

   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 ""

Hmm, shouldn't this be "or {}" ?

> +    split_conflicts = {}
> +    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 ))

whitespace isn't quite right here.

> +        if feature in valid_tunes:
> +            bb.note("  %s: %s" % (feature, valid_tunes[feature]))

We don't print a message every time something happens successfully. The
user would drown if we did so again, this needs to be bb.debug(2, xxx)

> +        else:
> +            tune_errors.append("Feature '%s' is not defined." % feature)
> +    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''

So what is TUNEABI_WHITELIST?


> +    if whitelist != '':

if whitelist:

> +        override = data.getVar("TUNEABI_OVERRIDE", True) or ''

and what is TUNEABI_OVERRIDE? These should probably get the [doc] tags
you add for other things.

> +        if not override:

Might as well make this:

    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
    override = data.getVar("TUNEABI_OVERRIDE", True) or ''
    if whitelist and not override:

> +            tuneabi = data.getVar("TUNEABI_tune-%s" % tune, True) or ""
> +            if 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))

To be honest I'm really tempted to split this tuneabi stuff out into a
different class. I appreciate the OSVs need something but it probably
doesn't make sense with half an implementation sitting in the core like
this, particularly when the code is near impenetrable like the above.
I'm left wondering what TUNEABI is too...

> +    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("MULTILIBS", True) or ""
> +    if multilibs != "":
> +        for pairs in [x.split(':') for x in multilibs.split()]:
> +            if pairs[0] != 'multilib':
> +                bb.warn("Got an unexpected '%s' in MULTILIBS." % pairs[0])

Shouldn't this get added to the errors list?

> +            else:
> +                if pairs[1] == 'lib':
> +                    tune_error_set.append("The multilib 'lib' was specified, but that causes parse errors.")

Hmm, it does?

What we really want to check here is that each pair[1] value is unique.

> +                else:
> +                    tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], True)
> +                    if tune == deftune:
> +                        bb.warn("Multilib '%s' (%s) is also the default tuning." % (pairs[1], deftune))

Again, I'd just add this to the error list. If someone really wants to
do it, they can disable the checks. This should also happen if tune is
used more than once.

We also want to check BASE_LIB_tune- is unique for each multilib.

> +                    else:
> +                        tune_errors = check_toolchain_tune(data, tune, pairs[1])
> +                    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 +391,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 f4d6241..99c16fb 100644
> --- a/meta/conf/documentation.conf
> +++ b/meta/conf/documentation.conf
> @@ -34,6 +34,12 @@ 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."
>  
> +# WARNING:  Because the flags on these have semantic implecations,
> +# they must not actually be defined.
> +#TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
> +#TUNECONFLICTS[doc] = "List of conflicting features for a given feature."
> +TUNEABI[doc] = "A base ABI used by a given tuning, used with prebuilt binaries."

Just set these and skip "doc" in the above tests. Its ugly but I'd
prefer to have them set than unset.

Can you split this patch into two so we can get the bits that were below
in (they look fine) whilst we work on the above.

Cheers,

Richard

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

[Peter Seebach]

On Tue, 1 May 2012 12:25:48 +0200
Koen Kooi <koen@dominion.thruhere.net> wrote:

> TUNE_FEATURES     = "armv7a vfp neon cortexa8"

I'd like to suggest a separate patch which uses the TUNEVALID info
to explain these, and also explores the tunings of any selected
multilibs, because this is under-informative in some cases.

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

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

[Peter Seebach]

On Tue, 1 May 2012 11:47:14 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> > 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.

^-- this will be referred back to later

> > +    if not tune or tune == "":
 
> "if not tune:"

Heh.  I do about 80% of my scripting work in Lua and Ruby these
days.  :)
 
> We'd usually have
> 
>    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

Heh.  And here, again, I'm used to things in which "".split() would
have given me an array with a single empty string back, same as any
other string with no spaces in it.  :)

Looks like I have to do more experiments.
 
> > +    valid_tunes = data.getVarFlags('TUNEVALID') or ""
> > +    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
 
> Hmm, shouldn't this be "or {}" ?

Yes.

> > +    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
 
> So what is TUNEABI_WHITELIST?

See the last paragraph of the above.
 
> and what is TUNEABI_OVERRIDE? These should probably get the [doc] tags
> you add for other things.

Good catch.

> To be honest I'm really tempted to split this tuneabi stuff out into a
> different class. I appreciate the OSVs need something but it probably
> doesn't make sense with half an implementation sitting in the core
> like this, particularly when the code is near impenetrable like the
> above. I'm left wondering what TUNEABI is too...

The big reason I'd advocate for having it in the base stuff is that
otherwise, all the OSVs are going to do their own and do it
incompatibly.  The one thing worse than having one incomprehensible
thing is having many incomprehensible things with subtly different
semantics.
 
> > +                bb.warn("Got an unexpected '%s' in MULTILIBS." %
> > pairs[0])

> Shouldn't this get added to the errors list?

Good question!  I was unable to find anything definite on the topic.
My thought is, if everything has to be multilib:, there's no reason for
the leading multilib: because it adds no information, so presumably
there logically *could* be something else, but I haven't seen any and
don't know about them.

> > +            else:
> > +                if pairs[1] == 'lib':
> > +                    tune_error_set.append("The multilib 'lib' was
> > specified, but that causes parse errors.")

> Hmm, it does?

Yeah, that was the thing I ran into when I tried a default config
someone suggested; I thoughtfully corrected lib32 to lib, and libxcb
exploded with two pages of Python stack traces and diagnostics.  If
that's not intentional, I can drop this check and we can treat it as a
package bug.

> What we really want to check here is that each pair[1] value is
> unique.
> Again, I'd just add this to the error list. If someone really wants to
> do it, they can disable the checks. This should also happen if tune is
> used more than once.

Okay.  Good catch on that, hadn't thought of it.
 
> Just set these and skip "doc" in the above tests. Its ugly but I'd
> prefer to have them set than unset.

Okay.
 
> Can you split this patch into two so we can get the bits that were
> below in (they look fine) whilst we work on the above.

Yup.

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

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

[Richard Purdie]

On Tue, 2012-05-01 at 11:23 -0500, Peter Seebach wrote:
> On Tue, 1 May 2012 11:47:14 +0100
> Richard Purdie <richard.purdie@linuxfoundation.org> wrote: 
> > To be honest I'm really tempted to split this tuneabi stuff out into a
> > different class. I appreciate the OSVs need something but it probably
> > doesn't make sense with half an implementation sitting in the core
> > like this, particularly when the code is near impenetrable like the
> > above. I'm left wondering what TUNEABI is too...
> 
> The big reason I'd advocate for having it in the base stuff is that
> otherwise, all the OSVs are going to do their own and do it
> incompatibly.  The one thing worse than having one incomprehensible
> thing is having many incomprehensible things with subtly different
> semantics.
>  
> > > +                bb.warn("Got an unexpected '%s' in MULTILIBS." %
> > > pairs[0])
> 
> > Shouldn't this get added to the errors list?
> 
> Good question!  I was unable to find anything definite on the topic.
> My thought is, if everything has to be multilib:, there's no reason for
> the leading multilib: because it adds no information, so presumably
> there logically *could* be something else, but I haven't seen any and
> don't know about them.

Ah, I see what's happened here. This variable is used to append to
BBCLASSEXTEND where there are other values like "nativesdk" and
"native". Looking at the code, I think you should process
MULTILIB_VARIANTS.

The naming of the various multilib variables could probably do with
improvement in places :/.

Another good sanity check might be to look at the BBCLASSEXTEND values
and ensure they're in the list "native", "nativesdk", "multilib:xxx",
"cross".

> > > +            else:
> > > +                if pairs[1] == 'lib':
> > > +                    tune_error_set.append("The multilib 'lib' was
> > > specified, but that causes parse errors.")
> 
> > Hmm, it does?
> 
> Yeah, that was the thing I ran into when I tried a default config
> someone suggested; I thoughtfully corrected lib32 to lib, and libxcb
> exploded with two pages of Python stack traces and diagnostics.  If
> that's not intentional, I can drop this check and we can treat it as a
> package bug.

I think its a packaging bug. I'd be interested to see the error.

Cheers,

Richard

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

[Peter Seebach]

On Tue, 1 May 2012 21:17:28 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> Ah, I see what's happened here. This variable is used to append to
> BBCLASSEXTEND where there are other values like "nativesdk" and
> "native". Looking at the code, I think you should process
> MULTILIB_VARIANTS.

Ahh, okay.
 
> Another good sanity check might be to look at the BBCLASSEXTEND values
> and ensure they're in the list "native", "nativesdk", "multilib:xxx",
> "cross".

Hmm.  Adding that as a separate commit.
 
> I think its a packaging bug. I'd be interested to see the error.

I'll try to get it to show.  I'm still getting the x86_64x86_64 thing
first.  :)

Should have cleaned up versions of this "soon".

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

[PATCH 2/2] 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.
---
 meta/classes/sanity.bbclass  |   76 ++++++++++++++++++++++++++++++++++++++++++
 meta/conf/documentation.conf |   10 +++++
 2 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 687ddeb..4cc5a14 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -11,6 +11,79 @@ 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("MULTILIBS", True) or "").split()
+    if multilibs:
+        seen_libs = []
+        seen_tunes = []
+        for pairs in [x.split(':') for x in multilibs]:
+            if pairs[0] != 'multilib':
+                bb.warn("Got an unexpected '%s' in MULTILIBS." % pairs[0])
+            else:
+                if pairs[1] in seen_libs:
+                    tune_error_set.append("The multilib '%s' appears more than once." % pairs[1])
+                else:
+                    seen_libs.append(pairs[1])
+                tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], 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." % (pairs[1], deftune))
+                else:
+                    tune_errors = check_toolchain_tune(data, tune, pairs[1])
+                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 +400,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..d410098 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -36,6 +36,16 @@ 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."
+ ASSUME_SHLIBS[doc] = "List of shlib:package[_version] mappings. Useful for lib packages in ASSUME_PROVIDED,\
 
 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 2/2] 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, and there were some loose ends (like not knowing how
the conflict one was spelled).  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                      |   69 ++++++++++++++++++++++
 meta/conf/documentation.conf                     |    6 ++
 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, 86 insertions(+), 6 deletions(-)

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 687ddeb..ff11fec 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -11,6 +11,72 @@ 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 or tune == "":
+        return "No tuning found for %s multilib." % multilib
+    bb.note("Sanity-checking tuning '%s' (%s) features:" % (tune, multilib))
+    features = data.getVar("TUNE_FEATURES_tune-%s" % tune, True) or ""
+    if features == '':
+        return "Tuning '%s' has no defined features, and cannot be used." % tune
+    features = features.split()
+    valid_tunes = data.getVarFlags('TUNEVALID') or ""
+    conflicts = data.getVarFlags('TUNECONFLICTS') or ""
+    split_conflicts = {}
+    for feature in features:
+        if feature in conflicts:
+            if feature not in split_conflicts:
+                split_conflicts[feature] = conflicts[feature].split()
+            for other in features:
+                if other in split_conflicts[feature]:
+                    tune_errors.append("Feature '%s' conflicts with '%s'." %
+                        ( feature, other ))
+        if feature in valid_tunes:
+            bb.note("  %s: %s" % (feature, valid_tunes[feature]))
+        else:
+            tune_errors.append("Feature '%s' is not defined." % feature)
+    whitelist = data.getVar("TUNEABI_WHITELIST", True) or ''
+    if whitelist != '':
+        override = data.getVar("TUNEABI_OVERRIDE", True) or ''
+        if not override:
+            tuneabi = data.getVar("TUNEABI_tune-%s" % tune, True) or ""
+            if 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("MULTILIBS", True) or ""
+    if multilibs != "":
+        for pairs in [x.split(':') for x in multilibs.split()]:
+            if pairs[0] != 'multilib':
+                bb.warn("Got an unexpected '%s' in MULTILIBS." % pairs[0])
+            else:
+                if pairs[1] == 'lib':
+                    tune_error_set.append("The multilib 'lib' was specified, but that causes parse errors.")
+                else:
+                    tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], True)
+                    if tune == deftune:
+                        bb.warn("Multilib '%s' (%s) is also the default tuning." % (pairs[1], deftune))
+                    else:
+                        tune_errors = check_toolchain_tune(data, tune, pairs[1])
+                    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 +393,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 f4d6241..f41c79f 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -34,6 +34,12 @@ 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."
 
+# WARNING:  Because the flags on these have semantic implecations,
+# they must not actually be defined.
+#TUNEVALID[doc] = "Descriptions of valid tuning features, stored as flags."
+#TUNEVALID[doc] = "List of conflicting features for a given feature."
+TUNEABI[doc] = "A base ABI used by a given tuning, used with prebuilt binaries."
+
 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