Fix to vs_toolchain.FindVCToolsRoot

build/vs_toolchain.py

Index: build/vs_toolchain.py
diff --git a/build/vs_toolchain.py b/build/vs_toolchain.py
index 61a3559859f7d38e0d5d21ea2c27eaaef3eab1f7..bb8f96c4e52cc3232e0ab5eaa2580e8cfba3a82d 100755
--- a/build/vs_toolchain.py
+++ b/build/vs_toolchain.py
@@ -229,6 +229,7 @@ def FindVCToolsRoot():
   This function should only be called when using VS2017.
   """
   assert GetVisualStudioVersion() == '2017'
+  SetEnvironmentAndGetRuntimeDllDirs()

[brucedawson, 2017/05/22 21:47:14]

Placing this call here means that the environment is set when you call
_CopyPGORuntime if-and-only-if you are building with VS 2017. It seems like it
would be more obvious if it was called directly and unconditionally from
_CopyPGORuntime. Thoughts?

[Sébastien Marchand, 2017/05/22 22:02:59]

The environment is already always set when you call _CopyPGORuntime because this
only get called by CopyDlls, which calls SetEnvironmentAndGetRuntimeDllDirs.

The only issue that I could see with this function is that it leak some
environment variable, I probably could save the environment at the beginning of
the function and restore it before returning, but I'm not sure if it's really
worth it? 

This function (FindVCToolsRoot) gets called by the merge_pgc_files script, so
another option would be to call SetEnvironment... from this script instead of
doing it here?

[brucedawson, 2017/05/22 23:48:45]

Ah - I understand.

I can see a case for calling SetEnvironment from the other script, unless it
just feels like that is postponing confusion.

I'm not sure I'd worry about environmental pollution.

[Sébastien Marchand, 2017/05/23 00:39:45]

I'll keep this call here then, relying on GYP_MSVS_OVERRIDE path being set seems
like an implementation detail that I don't want the callers to have to know.

   assert ('GYP_MSVS_OVERRIDE_PATH' in os.environ)
   vc_tools_msvc_root = os.path.join(os.environ['GYP_MSVS_OVERRIDE_PATH'],
       'VC', 'Tools', 'MSVC')