yuhangwang / gyp

Automatically exported from code.google.com/p/gyp
0 stars 0 forks source link

<@(VAR)> expansion drops \ in front of " #438

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Imagine that you have a variable 'external_cflags' with the following string 
content:

  -O2 -ggdb -Dversion=\"bar\"

If you define a variable in .gyp like:
    'cflags_c': [
      '<(external_cflags)',
    ],
the generator will output the right command, with the \".

But, if you convert this variable to a list to operate with it (for example, 
have a conditional to remove the '-ggdb' flag) the \" is converted to a ":
    'cflags_c': [
      '<@(external_cflags)',
    ],
which will output this as
... -Dversion="bar" ...
instead of 
... -Dversion=\"bar\" ...

This is being done on input.py:966 ( 
https://code.google.com/p/gyp/source/browse/trunk/pylib/gyp/input.py#966 ) on 
the if block "if expand_to_list". In the string to list case, it does:

  output = shlex.split(str(replacement))

where replacement is the original value of external_cflags. shlex.split() in 
posix mode (the default) will drop the \ in front of the ". The output is:
>>> shlex.split(r' -O2 -ggdb -Dversion=\"bar\"')
['-O2', '-ggdb', '-Dversion="bar"']

A proposed fix is to call it as:
  output = shlex.split(str(replacement), posix=False)

being the output:
>>> shlex.split(r' -O2 -ggdb -Dversion=\"bar\"', posix=False)
['-O2', '-ggdb', '-Dversion=\\"bar\\"']

I think this is a bug because the operation of split and join should give you 
back the original result (module some spaces). We have this exact use case in 
ChromeOS platform2.gyp where we want to be able to filter flags passed via 
-Dexternal_cflags=...

Original issue reported on code.google.com by de...@chromium.org on 25 Jun 2014 at 7:50

GoogleCodeExporter commented 9 years ago
Here is a patch to fix this bug, which also adds a new test for it.

Notice that an existing test changed its output from ['samplepathfoo.cpp'] to 
['sample\\path\\foo.cpp'], which I think is the right output.

I also fixed the test/variables/commands/update_golden script to call gyp 
without python, since it's no longer a python script.

I faked the line numbers on this patch so the diff is smaller and clearer. You 
should re-run update_golden after this patch to update the lines numbers, or 
change that script to remove the line numbers.

All the tests pass with this change. Thanks :)

Original comment by de...@chromium.org on 26 Jun 2014 at 5:41

Attachments:

GoogleCodeExporter commented 9 years ago
ping?

Original comment by de...@chromium.org on 14 Jul 2014 at 9:50