Make GenerateSetEnvCmd() more table driven.

win_toolchain/package_from_installed.py

Index: win_toolchain/package_from_installed.py
diff --git a/win_toolchain/package_from_installed.py b/win_toolchain/package_from_installed.py
index e1bea2afc5183fcd9b4654cebe8983d675b13518..707229204b2737cd37a97ff21d5aab6a2a917c4b 100644
--- a/win_toolchain/package_from_installed.py
+++ b/win_toolchain/package_from_installed.py
@@ -201,45 +201,70 @@ def GenerateSetEnvCmd(target_dir):
 
   This is normally generated by a full install of the SDK, but we
   do it here manually since we do not do a full install."""
-  with open(os.path.join(
-        target_dir, r'win_sdk\bin\SetEnv.cmd'), 'w') as f:
+  # All these paths are relative to the directory containing SetEnv.cmd.
+  include_dirs = [
+    ['..', '..', 'win_sdk', 'Include', WIN_VERSION, 'um'],

[scottmg, 2016/02/18 18:15:05]

Could you remove all of the '..', '..' from these lists and put them in
BatDirs() instead?

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

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

[scottmg, 2016/02/18 18:43:22]

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.

+    ['..', '..', 'win_sdk', 'Include', WIN_VERSION, 'shared'],
+    ['..', '..', 'win_sdk', 'Include', WIN_VERSION, 'winrt'],
+  ]
+  if VS_VERSION == '2015':
+    include_dirs.append(['..', '..', 'win_sdk', 'Include', WIN_VERSION, 'ucrt'])
+  include_dirs.extend([
+    ['..', '..', 'VC', 'include'],
+    ['..', '..', 'VC', 'atlmfc', 'include'],
+  ])
+  # Common to x86 and x64
+  env = [
+    # Yuck: These two have a trailing \ character. No good way to represent this
+    # in an OS-independent way.
+    ('VSINSTALLDIR', [['..', '..\\']]),
+    ('VCINSTALLDIR', [['..', '..', 'VC\\']]),
+    ('INCLUDE', include_dirs),

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

Oh, I guess not because of this one. Hrm. OK.

+  ]
+  # x86. Always use amd64_x86 cross, not x86 on x86.
+  env_x86 = [
+    ('PATH', [
+      ['..', '..', 'win_sdk', 'bin', 'x86'],
+      ['..', '..', 'VC', 'bin', 'amd64_x86'],
+      ['..', '..', 'VC', 'bin', 'amd64'],  # Needed for mspdb1x0.dll.
+    ]),
+    ('LIB', [
+      ['..', '..', 'VC', 'lib'],
+      ['..', '..', 'win_sdk', 'Lib', WIN_VERSION, 'um', 'x86'],
+      ['..', '..', 'win_sdk', 'Lib', WIN_VERSION, 'ucrt', 'x86'],  # VS 2015
+      ['..', '..', 'VC', 'atlmfc', 'lib'],
+    ]),
+  ]
+  # x64.
+  env_x64 = [
+    ('PATH', [
+      ['..', '..', 'win_sdk', 'bin', 'x64'],
+      ['..', '..', 'VC', 'bin', 'amd64'],
+    ]),
+    ('LIB', [
+      ['..', '..', 'VC', 'lib', 'amd64'],
+      ['..', '..', 'win_sdk', 'Lib', WIN_VERSION, 'um', 'x64'],
+      ['..', '..', 'win_sdk', 'Lib', WIN_VERSION, 'ucrt', 'x64'],  # VS 2015
+      ['..', '..', 'VC', 'atlmfc', 'lib', 'amd64'],
+    ]),
+  ]
+  def BatDirs(dirs): return ';'.join(['%~dp0' + os.path.join(*d) for d in dirs])

[scottmg, 2016/02/18 18:15:05]

Probably `def bat_dirs(dirs):` instead.

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

why? style guide says python function names are LikeThis

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

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

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

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

+  with open(os.path.join(target_dir, r'win_sdk\bin\SetEnv.cmd'), 'w') as f:
     f.write('@echo off\n'
-            ':: Generated by win_toolchain\\package_from_installed.py.\n'
-            # Common to x86 and x64
-            'set VSINSTALLDIR=%~dp0..\\..\\\n'
-            'set VCINSTALLDIR=%~dp0..\\..\\VC\\\n'
-            'set INCLUDE=%~dp0..\\..\\win_sdk\\Include\\WINVERSION\\um;'
-            '%~dp0..\\..\\win_sdk\\Include\\WINVERSION\\shared;'
-            '%~dp0..\\..\\win_sdk\\Include\\WINVERSION\\winrt;'.replace(
-                'WINVERSION', WIN_VERSION))
-    if VS_VERSION == '2015':
-      f.write('%~dp0..\\..\\win_sdk\\Include\\WINVERSION\\ucrt;'.replace(
-                  'WINVERSION', WIN_VERSION))
-    f.write('%~dp0..\\..\\VC\\include;'
-            '%~dp0..\\..\\VC\\atlmfc\\include\n'
-            'if "%1"=="/x64" goto x64\n')
-
-    # x86. Always use amd64_x86 cross, not x86 on x86.
-    f.write('set PATH=%~dp0..\\..\\win_sdk\\bin\\x86;'
-              '%~dp0..\\..\\VC\\bin\\amd64_x86;'
-              '%~dp0..\\..\\VC\\bin\\amd64;'  # Needed for mspdb1x0.dll.
-              '%PATH%\n')
-    f.write('set LIB=%~dp0..\\..\\VC\\lib;'
-               '%~dp0..\\..\\win_sdk\\Lib\\WINVERSION\\um\\x86;'
-               '%~dp0..\\..\\win_sdk\\Lib\\WINVERSION\\ucrt\\x86;' # VS 2015
-               '%~dp0..\\..\\VC\\atlmfc\\lib\n'
-            'goto :EOF\n'.replace('WINVERSION', WIN_VERSION))
-
-    # x64.
-    f.write(':x64\n'
-            'set PATH=%~dp0..\\..\\win_sdk\\bin\\x64;'
-                '%~dp0..\\..\\VC\\bin\\amd64;'
-                '%PATH%\n')
-    f.write('set LIB=%~dp0..\\..\\VC\\lib\\amd64;'
-               '%~dp0..\\..\\win_sdk\\Lib\\WINVERSION\\um\\x64;'
-               '%~dp0..\\..\\win_sdk\\Lib\\WINVERSION\\ucrt\\x64;' # VS 2015
-               '%~dp0..\\..\\VC\\atlmfc\\lib\\amd64\n'
-               .replace('WINVERSION', WIN_VERSION))
+            ':: Generated by win_toolchain\\package_from_installed.py.\n')
+    for var, dirs in env:
+      f.write('set %s=%s\n' % (var, BatDirs(dirs)))
+    f.write('if "%1"=="/x64" goto x64\n')
+
+    for var, dirs in env_x86:
+      f.write('set %s=%s%s\n' % (
+          var, BatDirs(dirs), ';%PATH%' if var == 'PATH' else ''))
+    f.write('goto :EOF\n')
+
+    f.write(':x64\n')
+    for var, dirs in env_x64:
+      f.write('set %s=%s%s\n' % (
+          var, BatDirs(dirs), ';%PATH%' if var == 'PATH' else ''))
 
 
 def AddEnvSetup(files):