[OE-core] Toolchain library whitelisting: A first pass (preliminary patch/RFC)
Mark Hatle
mark.hatle at windriver.com
Thu Apr 26 22:01:41 UTC 2012
In general I like this.. see a few critiques below:
On 4/26/12 4:08 PM, Peter Seebach wrote:
> On Wed, 25 Apr 2012 20:38:05 -0500
> Peter Seebach<peter.seebach at windriver.com> wrote:
>> This is a followup from some chat in #yocto and elsewhere.
>
> Okay, some more followup.
>
> While testing this, I kept burning myself on perfectly reasonable
> things to get wrong while defining and using multilibs, so I wrote a
> bunch of sanity checks for that.
>
> The intent of this is to validate that tunings (including multilibs)
> are configured in a reasonable way that we would expect to work. This
> includes:
>
> 1. Verifying that no multilib's tuning is the same as DEFAULTTUNE.
> 2. Verifying that no multilib's library name is 'lib', because that
> causes really cryptic error messages parsing recipes.
> 3. For each tuning, verify:
> 3a. The tuning has features.
> 3b. Every feature has a TUNEVALID[x] entry.
> 3c. If the feature has a TUNECONFLICTS[x] entry, no feature listed in
> it is included in the feature list.
> 3d. If the value TUNEABI_WHITELIST exists, the tuning's
> TUNEABI_tune-foo value, or the tuning's name if that doesn't exist, is
> in TUNEABI_WHITELIST, or alternatively, TUNEABI_OVERRIDE is defined.
>
> The whitelist feature is optional, and my intent would be not to define
> any TUNEABI_tune values in oe-core, but just to maintain the hooks so
> that people with custom (and often prebuilt-binary) toolchains can use
> it without all of us writing our own.
>
> I am totally aware that my Python is a little rough, and would be happy
> to improve the legibility or idiom.
>
> Separately, I propose also the following fix:
>
> # Check TARGET_ARCH is set correctly
> - if data.getVar('TARGE_ARCH', e.data, False) == '${TUNE_ARCH}':
> + if data.getVar('TARGET_ARCH', e.data, False) == '${TUNE_ARCH}':
>
> Anyway, the patch:
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 687ddeb..b7f93b5 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 = []
> + 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(' ')
split does a whitespace based split automatically, I'm used to seeing .split()
everywhere. (I won't comment on the other similar split items)
> + validities = data.getVarFlags('TUNEVALID') or ""
"validities"? that a new word? ;)
> + 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 validities:
> + bb.note(" %s: %s" % (feature, validities[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", True) or ""
> + if tuneabi == "" or tuneabi.startswith('$'):
> + 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 != "":
Change the above to:
multilibs = data.getVar("MULTILIBS", True)
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 doesn't work. You need lib32 or lib64.")
I'm surprised, why doesn't 'lib' work? I was under the impression the naming
was completely arbitrary.
Also we can definitely have more then just lib32 or lib64. We already often use
libx32 in testing the x32 layer.
> + else:
> + tune = data.getVar("DEFAULTTUNE_virtclass-multilib-%s" % pairs[1], True)
If the tune isn't defined, then the multilib configuration is invalid. I
thought we already had a check for that somewhere else.. but if not.. it
wouldn't be a bad idea to mention that here for the user.
A simple if not tune: would do it..
> + if tune == deftune:
> + tune_error_set.append("Multilib '%s' (%s) is also the default tuning." % (pairs[1], deftune))
I wonder if this is an error or a warning.. I suspect it would be unintentional,
but I'm not sure it's a failure?
> + 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'
Your whitespace usage looks different.. perhaps that is just my mailer.
--Mark
>
> # 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':
More information about the Openembedded-core
mailing list