Patchwork [bitbake-devel] bitbake/ConfHandler: Be more strict about variable quoting

login
register
mail settings
Submitter Richard Purdie
Date Feb. 26, 2012, 1:04 p.m.
Message ID <1330261465.31855.4.camel@ted>
Download mbox | patch
Permalink /patch/22083/
State New
Headers show

Comments

Richard Purdie - Feb. 26, 2012, 1:04 p.m.
[cross posted to OE-Core since this is a major change in behaviour but
in my opinion, probably a good one]

Currently, bitbake will accept variables in the forms:

X = 1
X = '1 \

X = "1"
X = '1'

which will all set X=1. This patch removes the first two possibilities
and makes quoting mandatory. There is little metadata out there which
doesn't quote properly and bitbake will exit with an error about the
exact line number and file with any problem so users can easily identify
and fix issues. OE-Core has already been checked/fixed.

The motivation for this is being able to give sane errors if a user
does something like:

IMAGE_INSTALL += # tslib mtd-utils"

which currently gives a really nasty failure.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Martin Jansa - Feb. 26, 2012, 9:26 p.m.
On Sun, Feb 26, 2012 at 01:04:25PM +0000, Richard Purdie wrote:
> [cross posted to OE-Core since this is a major change in behaviour but
> in my opinion, probably a good one]

Agreed, missing quotes are hard to notice and easy to fix in existing
recipes with resonable error message provided by this.
 
> Currently, bitbake will accept variables in the forms:
> 
> X = 1
> X = '1 \
> 
> X = "1"
> X = '1'
> 
> which will all set X=1. This patch removes the first two possibilities
> and makes quoting mandatory. There is little metadata out there which
> doesn't quote properly and bitbake will exit with an error about the
> exact line number and file with any problem so users can easily identify
> and fix issues. OE-Core has already been checked/fixed.

Not completly true about OE-Core, but I'll send another patch fixing the
rest and also meta-oe and meta-smartphone layers..

Cheers,

> The motivation for this is being able to give sane errors if a user
> does something like:
> 
> IMAGE_INSTALL += # tslib mtd-utils"
> 
> which currently gives a really nasty failure.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> diff --git a/bitbake/lib/bb/parse/parse_py/ConfHandler.py b/bitbake/lib/bb/parse/parse_py/ConfHandler.py
> index 9242632..64b8208 100644
> --- a/bitbake/lib/bb/parse/parse_py/ConfHandler.py
> +++ b/bitbake/lib/bb/parse/parse_py/ConfHandler.py
> @@ -29,8 +29,7 @@ import logging
>  import bb.utils
>  from bb.parse import ParseError, resolve_file, ast, logger
>  
> -#__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}]+)\s*(?P<colon>:)?(?P<ques>\?)?=\s*(?P<apo>['\"]?)(?P<value>.*)(?P=apo)$")
> -__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"]?)(?P<value>.*)(?P=apo)$")
> +__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"])(?P<value>.*)(?P=apo)$")
>  __include_regexp__ = re.compile( r"include\s+(.+)" )
>  __require_regexp__ = re.compile( r"require\s+(.+)" )
>  __export_regexp__ = re.compile( r"export\s+(.+)" )
> 
> 
> 
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/bitbake-devel
Richard Purdie - Feb. 26, 2012, 10:53 p.m.
On Sun, 2012-02-26 at 22:26 +0100, Martin Jansa wrote:
> On Sun, Feb 26, 2012 at 01:04:25PM +0000, Richard Purdie wrote:
> > [cross posted to OE-Core since this is a major change in behaviour but
> > in my opinion, probably a good one]
> 
> Agreed, missing quotes are hard to notice and easy to fix in existing
> recipes with resonable error message provided by this.
>  
> > Currently, bitbake will accept variables in the forms:
> > 
> > X = 1
> > X = '1 \
> > 
> > X = "1"
> > X = '1'
> > 
> > which will all set X=1. This patch removes the first two possibilities
> > and makes quoting mandatory. There is little metadata out there which
> > doesn't quote properly and bitbake will exit with an error about the
> > exact line number and file with any problem so users can easily identify
> > and fix issues. OE-Core has already been checked/fixed.
> 
> Not completly true about OE-Core, but I'll send another patch fixing the
> rest and also meta-oe and meta-smartphone layers..

Right, it looks like I didn't trigger a full reparse. Thanks for the
patches, I've applied the OE-Core one.

Cheers,

Richard

Patch

diff --git a/bitbake/lib/bb/parse/parse_py/ConfHandler.py b/bitbake/lib/bb/parse/parse_py/ConfHandler.py
index 9242632..64b8208 100644
--- a/bitbake/lib/bb/parse/parse_py/ConfHandler.py
+++ b/bitbake/lib/bb/parse/parse_py/ConfHandler.py
@@ -29,8 +29,7 @@  import logging
 import bb.utils
 from bb.parse import ParseError, resolve_file, ast, logger
 
-#__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}]+)\s*(?P<colon>:)?(?P<ques>\?)?=\s*(?P<apo>['\"]?)(?P<value>.*)(?P=apo)$")
-__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"]?)(?P<value>.*)(?P=apo)$")
+__config_regexp__  = re.compile( r"(?P<exp>export\s*)?(?P<var>[a-zA-Z0-9\-_+.${}/]+)(\[(?P<flag>[a-zA-Z0-9\-_+.]+)\])?\s*((?P<colon>:=)|(?P<lazyques>\?\?=)|(?P<ques>\?=)|(?P<append>\+=)|(?P<prepend>=\+)|(?P<predot>=\.)|(?P<postdot>\.=)|=)\s*(?P<apo>['\"])(?P<value>.*)(?P=apo)$")
 __include_regexp__ = re.compile( r"include\s+(.+)" )
 __require_regexp__ = re.compile( r"require\s+(.+)" )
 __export_regexp__ = re.compile( r"export\s+(.+)" )