be more lenient with whitespace in GYP_DEFINES

treat all whitespace as a delimiter in GYP_DEFINES

gyp itself treats any whitespace as a delimiter in GYP_DEFINES, this script should behave the same.

NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271851

[Mostyn Bramley-Moore, 2014-05-20 12:24:07]

@Lei Zhang, Nico: PTAL

[Mostyn Bramley-Moore, 2014-05-20 13:37:58]

+mithro

[Lei Zhang, 2014-05-20 21:18:46]

lgtm

Add NOTRY=true ?

[Mostyn Bramley-Moore, 2014-05-20 21:30:19]

On 2014/05/20 21:18:46, Lei Zhang wrote:
> lgtm
> 
> Add NOTRY=true ?

Thanks, done.

[Mostyn Bramley-Moore, 2014-05-20 21:30:25]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2014-05-20 21:33:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mostynb@opera.com/290173005/1

[mithro-old, 2014-05-20 23:03:12]

On 2014/05/20 21:33:24, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-status.appspot.com/cq/mostynb@opera.com/290173005/1

I think this change is a LGTM.

However, I don't know what is actually valid in GYP_DEFINEs but are things like
quotes valid? For example, which of these are valid?
  blah=1 host_arch=x86 blah=1
  blah=1 host_arch='x86' blah=1 
  blah=1 host_arch="x86" blah=1
  blah=1 host_arch='a b' blah=1
  blah=1 host_arch="a b" blah=1
  blah=1 host_arch="'a b'" blah=1
  blah=1 host_arch="a \" b" blah=1

To match all the above you want the following (which is horrible);
------------------------------------
import re

tests="""\
  blah=1 host_arch=x86 blah=1
  blah=1 host_arch='x86' blah=1 
  blah=1 host_arch="x86" blah=1
  blah=1 host_arch='a b' blah=1
  blah=1 host_arch='a \\' b' blah=1
  blah=1 host_arch="a b" blah=1
  blah=1 host_arch="'a b'" blah=1
  blah=1 host_arch="a \\" b" blah=1
"""

def extract(line):
  if not line:
    return
  pattern = """
host_arch=(
 ("(?P<a>(([^"]+)|(\\\\"))*[^\\\\])")|  # host_arch="a b" and host_arch="a \" a"
 ('(?P<b>(([^']+)|(\\\\'))*[^\\\\])')|  # host_arch='a b' and host_arch='a \' a'
 (?P<c>\S*)                             # Plain host_arch=ab
)
"""
  m = re.search(pattern, line, re.VERBOSE)
  print line
  print m, m.groups()
  return re.sub("\\\\([\"'])", "\\1", (m.group('a') or m.group('b') or
m.group('c')))


for line in tests.split("\n"):
  print extract(line)
------------------------------------

[Mostyn Bramley-Moore, 2014-05-20 23:36:23]

On 2014/05/20 23:03:12, mithro wrote:
> On 2014/05/20 21:33:24, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >  https://chromium-status.appspot.com/cq/mostynb@opera.com/290173005/1
> 
> I think this change is a LGTM.
> 
> However, I don't know what is actually valid in GYP_DEFINEs but are things
like
> quotes valid? For example, which of these are valid?
>   blah=1 host_arch=x86 blah=1
>   blah=1 host_arch='x86' blah=1 
>   blah=1 host_arch="x86" blah=1
>   blah=1 host_arch='a b' blah=1
>   blah=1 host_arch="a b" blah=1
>   blah=1 host_arch="'a b'" blah=1
>   blah=1 host_arch="a \" b" blah=1

A quick search of my various build configs confirms that at least foo='bar' is
valid, and I suspect several others (if not all) the variations above are also. 
However for host_arch, I think these cases are less likely to occur than leading
or trailing tab/newlines, since the range of values for host_arch do not include
spaces.  

For comparison build/download_nacl_toolchains.py doesn't check for quotes
either, but does use another (also imperfect) method for ignoring
leading/trailing whitespace:
    if 'disable_nacl=1' in os.environ.get('GYP_DEFINES', '')
which would break if given "dont_disable_nacl=1".

> To match all the above you want the following (which is horrible);
[snip]

I would rather defer a "proper" fix for now, and perhaps handle it later as part
of a generic download_and_extract tool as mentioned in your TODO note.

[mithro-old, 2014-05-21 00:32:39]

On 2014/05/20 23:36:23, Mostyn Bramley-Moore wrote:
> On 2014/05/20 23:03:12, mithro wrote:
> > On 2014/05/20 21:33:24, I haz the power (commit-bot) wrote:
> > > CQ is trying da patch. Follow status at
> > >  https://chromium-status.appspot.com/cq/mostynb@opera.com/290173005/1
> > 
> > I think this change is a LGTM.
> > 
> > However, I don't know what is actually valid in GYP_DEFINEs but are things
> like
> > quotes valid? For example, which of these are valid?
> >   blah=1 host_arch=x86 blah=1
> >   blah=1 host_arch='x86' blah=1 
> >   blah=1 host_arch="x86" blah=1
> >   blah=1 host_arch='a b' blah=1
> >   blah=1 host_arch="a b" blah=1
> >   blah=1 host_arch="'a b'" blah=1
> >   blah=1 host_arch="a \" b" blah=1
> 
> A quick search of my various build configs confirms that at least foo='bar' is
> valid, and I suspect several others (if not all) the variations above are
also. 
> However for host_arch, I think these cases are less likely to occur than
leading
> or trailing tab/newlines, since the range of values for host_arch do not
include
> spaces.  
> 
> For comparison build/download_nacl_toolchains.py doesn't check for quotes
> either, but does use another (also imperfect) method for ignoring
> leading/trailing whitespace:
>     if 'disable_nacl=1' in os.environ.get('GYP_DEFINES', '')
> which would break if given "dont_disable_nacl=1".

Yeah :/

> > To match all the above you want the following (which is horrible);
> [snip]
> 
> I would rather defer a "proper" fix for now, and perhaps handle it later as
part
> of a generic download_and_extract tool as mentioned in your TODO note.

Agreed we shouldn't block this CL (hence the LGTM earlier :).

Tim

[commit-bot: I haz the power, 2014-05-21 03:46:22]

Change committed as 271851