Strip trailing backslashes from vcvarsall results

Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes. For more details see the gn
docs:

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/language.md#Strings

BUG=593154
Committed: https://crrev.com/12bbca48cda56b2a10a68f8f69bb90906189c4bf
Cr-Commit-Position: refs/heads/master@{#382752}

[brucedawson, 2016-03-22 23:48:13]

brucedawson@chromium.org changed reviewers:
+ dpranke@chromium.org, scottmg@chromium.org

[brucedawson, 2016-03-22 23:48:13]

This fixes a gn problem with DEPOT_TOOLS_WIN_TOOLCHAIN=0.

I'm mostly adding dpranke@ in case it would be better to fix the gn parser -
this might indicate some inconsistent logic for string parsing.

[Dirk Pranke, 2016-03-22 23:58:15]

dpranke@chromium.org changed reviewers:
+ brettw@chromium.org

[Dirk Pranke, 2016-03-22 23:58:15]

I *think* this is probably the right thing to do, because GN wants to see
backslashes escaped, but cc'ing brettw to confirm.

lgtm otherwise.

[Dirk Pranke, 2016-03-22 23:59:30]

On 2016/03/22 23:58:15, Dirk Pranke wrote:
> I *think* this is probably the right thing to do, because GN wants to see
> backslashes escaped, but cc'ing brettw to confirm.
> 
> lgtm otherwise.

maybe I'm wrong; otherwise I would've expected the directory separators to also
be problematic, and maybe it is just the trailing backslash that fails, in which
case maybe GN should be changed ...

[brucedawson, 2016-03-23 00:03:22]

After thinking about it some more I think that the behavior is consistent with
Python raw strings:

>>> "asdf\asdf\asdf"
'asdf\x07sdf\x07sdf'
>>> r"asdf\asdf\asdf"
'asdf\\asdf\\asdf'
>>> r"asdf\asdf\asdf\"
  File "<stdin>", line 1
    r"asdf\asdf\asdf\"
                     ^
SyntaxError: EOL while scanning string literal


Note that the backslashes in the middle of the string are not escape characters,
but an odd number of backslashes at the end is not legal.

So I'm leaning towards this being the correct fix.

[Dirk Pranke, 2016-03-23 00:13:11]

I double-checked w/ Brett and he reminded me that we wanted to make it "easy"
for devs
to embed win paths, so backslashes do not need to be escaped by default.

However, double-quotes do, so yeah, the trailing backslash needs to be either
removed
or escaped.

cf.

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/languag...

so, still lgtm.

[brucedawson, 2016-03-23 00:16:42]

Description was changed from

==========
Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes which is a safer fix than changing
the gn parser.

BUG=593154
==========

to

==========
Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes. For more details see the gn
docs:

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/languag...

BUG=593154
==========

[brucedawson, 2016-03-23 00:17:41]

> so, still lgtm.

Thanks for the reference. I updated the change description to incorporate that
and to make it clear that this is all WAI.

I predict we will see this problem again - trailing backslashes happen.

[brucedawson, 2016-03-23 00:17:44]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-03-23 00:18:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1827663002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1827663002/1

[commit-bot: I haz the power, 2016-03-23 00:58:16]

Description was changed from

==========
Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes. For more details see the gn
docs:

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/languag...

BUG=593154
==========

to

==========
Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes. For more details see the gn
docs:

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/languag...

BUG=593154
==========

[commit-bot: I haz the power, 2016-03-23 00:58:17]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-23 00:59:37]

Description was changed from

==========
Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes. For more details see the gn
docs:

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/languag...

BUG=593154
==========

to

==========
Strip trailing backslashes from vcvarsall results

When building with DEPOT_TOOLS_WIN_TOOLCHAIN=0 the vs_toolchain.py
script uses environment variables set up by vcvarsall.bat, such as
WINDOWSSDKDIR. The default value of this with VS 2015 is
WindowsSdkDir=C:\Program Files (x86)\Windows Kits\10\ - note the
trailing slash.

When ninja runs "build\vs_toolchain.py get_toolchain_dir" it translates
this to:

sdk_path = "C:\Program Files (x86)\Windows Kits\10\"

The trailing slash in front of the quote confuses the gn parser. This
change strips those trailing slashes. For more details see the gn
docs:

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/languag...

BUG=593154

Committed: https://crrev.com/12bbca48cda56b2a10a68f8f69bb90906189c4bf
Cr-Commit-Position: refs/heads/master@{#382752}
==========

[commit-bot: I haz the power, 2016-03-23 00:59:38]

Patchset 1 (id:??) landed as
https://crrev.com/12bbca48cda56b2a10a68f8f69bb90906189c4bf
Cr-Commit-Position: refs/heads/master@{#382752}

[brettw, 2016-03-23 16:00:12]

The "right way" would be to use build/gn_helpers.py: ToGNString() for output.
This utility is designed for GN interop so everybody can get the parsing and
formatting correct.