Make GenerateSetEnvCmd() more table driven.

Make GenerateSetEnvCmd() more table driven.

No intended behavior change.  This makes it possible to dump this state into
SetEnv.x32.json and SetEnv.x64.json in an easy follow-up.

BUG=495204
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298856

[Nico, 2016-02-18 15:55:26]

thakis@chromium.org changed reviewers:
+ scottmg@chromium.org

[Nico, 2016-02-18 15:55:27]

I ran this locally and checked it produces the same SetEnv.cmd as before (see
the many uploaded revisions for what I got wrong initially… 9_9)

[scottmg, 2016-02-18 18:15:06]

lgtm

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
File win_toolchain/package_from_installed.py (right):

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:206: ['..', '..', 'win_sdk', 'Include',
WIN_VERSION, 'um'],
Could you remove all of the '..', '..' from these lists and put them in
BatDirs() instead?

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:222: ('INCLUDE', include_dirs),
Oh, I guess not because of this one. Hrm. OK.

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:251: def BatDirs(dirs): return
';'.join(['%~dp0' + os.path.join(*d) for d in dirs])
Probably `def bat_dirs(dirs):` instead.

[Nico, 2016-02-18 18:20:33]

thanks!

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
File win_toolchain/package_from_installed.py (right):

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:206: ['..', '..', 'win_sdk', 'Include',
WIN_VERSION, 'um'],
On 2016/02/18 18:15:05, scottmg wrote:
> Could you remove all of the '..', '..' from these lists and put them in
> BatDirs() instead?

But they're part of the relative path – I want them in the json file too

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:251: def BatDirs(dirs): return
';'.join(['%~dp0' + os.path.join(*d) for d in dirs])
On 2016/02/18 18:15:05, scottmg wrote:
> Probably `def bat_dirs(dirs):` instead.

why? style guide says python function names are LikeThis

[scottmg, 2016-02-18 18:43:23]

lgtm

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
File win_toolchain/package_from_installed.py (right):

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:206: ['..', '..', 'win_sdk', 'Include',
WIN_VERSION, 'um'],
On 2016/02/18 18:20:33, Nico wrote:
> On 2016/02/18 18:15:05, scottmg wrote:
> > Could you remove all of the '..', '..' from these lists and put them in
> > BatDirs() instead?
> 
> But they're part of the relative path – I want them in the json file too

It just was a lot of redundancy, so I was thinking making these literal lists
root-relative, but then at output time making them SetEnv.cmd relative. Or
.json-relative.

But then VSINSTALLDIR and INCLUDE would probably need a special case, so it's
probably not worth it.

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:251: def BatDirs(dirs): return
';'.join(['%~dp0' + os.path.join(*d) for d in dirs])
On 2016/02/18 18:20:33, Nico wrote:
> On 2016/02/18 18:15:05, scottmg wrote:
> > Probably `def bat_dirs(dirs):` instead.
> 
> why? style guide says python function names are LikeThis

OK. (I always think of local functions as variables, not functions for some
reason.)

[Nico, 2016-02-18 18:45:24]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2016-02-18 18:45:34]

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

[commit-bot: I haz the power, 2016-02-18 18:51:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-18 18:51:15]

Try jobs failed on following builders:
  depot_tools_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/depot_tools_pre...)

[brucedawson, 2016-02-18 18:54:56]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2016-02-18 18:54:57]

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
File win_toolchain/package_from_installed.py (right):

https://codereview.chromium.org/1708223002/diff/120001/win_toolchain/package_...
win_toolchain/package_from_installed.py:251: def BatDirs(dirs): return
';'.join(['%~dp0' + os.path.join(*d) for d in dirs])
On 2016/02/18 18:43:23, scottmg wrote:
> On 2016/02/18 18:20:33, Nico wrote:
> > On 2016/02/18 18:15:05, scottmg wrote:
> > > Probably `def bat_dirs(dirs):` instead.
> > 
> > why? style guide says python function names are LikeThis
> 
> OK. (I always think of local functions as variables, not functions for some
> reason.)

I guess presubmit doesn't like the one-line function definition.

[Nico, 2016-02-18 18:59:49]

The CQ bit was checked by thakis@chromium.org

[Nico, 2016-02-18 18:59:49]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1708223002/#ps140001
(title: "wot")

[commit-bot: I haz the power, 2016-02-18 18:59:55]

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

[commit-bot: I haz the power, 2016-02-18 19:02:33]

Description was changed from

==========
Make GenerateSetEnvCmd() more table driven.

No intended behavior change.  This makes it possible to dump this state into
SetEnv.x32.json and SetEnv.x64.json in an easy follow-up.

BUG=495204
==========

to

==========
Make GenerateSetEnvCmd() more table driven.

No intended behavior change.  This makes it possible to dump this state into
SetEnv.x32.json and SetEnv.x64.json in an easy follow-up.

BUG=495204

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298856
==========

[commit-bot: I haz the power, 2016-02-18 19:02:33]

Committed patchset #8 (id:140001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298856