Make GenerateSetEnvCmd() more table driven.

win_toolchain/package_from_installed.py

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """
 From a system-installed copy of the toolchain, packages all the required bits
 into a .zip file.
 
 It assumes default install locations for tools, in particular:
 - C:\Program Files (x86)\Microsoft Visual Studio 12.0\...
@@ skipping 183 matching lines @@
                                        'arm64\\' not in f.lower() and
                                        not f.lower().endswith('.msi')]
 
 
 def GenerateSetEnvCmd(target_dir):
   """Generate a batch file that gyp expects to exist to set up the compiler
   environment.
 
   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')
+            ':: 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')
 
-    # 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))
+    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')
 
-    # 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))
+    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):
   """We need to generate this file in the same way that the "from pieces"
   script does, so pull that in here."""
   tempdir = tempfile.mkdtemp()
   os.makedirs(os.path.join(tempdir, 'win_sdk', 'bin'))
   GenerateSetEnvCmd(tempdir)
   files.append((os.path.join(tempdir, 'win_sdk', 'bin', 'SetEnv.cmd'),
                 'win_sdk\\bin\\SetEnv.cmd'))
@@ skipping 91 matching lines @@
   sys.stdout.write('\rWrote to %s.%s\n' % (output, ' '*50))
   sys.stdout.flush()
 
   RenameToSha1(output)
 
   return 0
 
 
 if __name__ == '__main__':
   sys.exit(main())