Error about missing SYSTEMROOT environment variable with MSVC2013 Express

[GoogleCodeExporter]

When I run gyp, I get an error about an invalid SYSTEMROOT environment 
variable.  Here's the command-line output:

    C:\...\atom\atom-shell>..\..\gyp-read-only\gyp.bat -f ninja --depth . atom.gyp -Icommon.gypi -Ivendor/brightray/brightray.gypi -Dlinux_clang=0 -Dtarget_arch=ia32 -Dlibrary=static_library
    Traceback (most recent call last):
      File "C:\...\gyp-read-only\gyp_main.py", line 18, in <module>
        sys.exit(gyp.script_main())
      File "C:\...\gyp-read-only\pylib\gyp\__init__.py", line 532, in script_main
        return main(sys.argv[1:])
      File "C:\...\gyp-read-only\pylib\gyp\__init__.py", line 525, in main
        return gyp_main(args)
      File "C:\...\gyp-read-only\pylib\gyp\__init__.py", line 510, in gyp_main
        generator.GenerateOutput(flat_list, targets, data, params)
      File "C:\...\gyp-read-only\pylib\gyp\generator\ninja.py", line 2376, in GenerateOutput
        pool.map(CallGenerateOutputForConfig, arglists)
      File "C:\Python27\lib\multiprocessing\pool.py", line 251, in map
        return self.map_async(func, iterable, chunksize).get()
      File "C:\Python27\lib\multiprocessing\pool.py", line 558, in get
        raise self._value
    Exception: Environment variable "SYSTEMROOT" required to be set to valid path

The error is misleading; the problem isn't really SYSTEMROOT.

The problem only happens when I run gyp in a VC command prompt, which defines 
WindowsSDKDir.  _DetectVisualStudioVersions decides that I have a single Visual 
Studio install:

   name: 2013e
   path: C:\msvs12\VC\..
   sdk_based: True

This is correct.  My machine has Visual Studio 2013 Express installed, and no 
other copies of Visual Studio or any Windows SDKs.

The root problem is that MSVSVersion.SetupScript returns a path to a 
non-existent file:

      def SetupScript(self, target_arch):
        """Returns a command (with arguments) to be used to set up the
        environment."""
        # Check if we are running in the SDK command line environment and use
        # the setup script from the SDK if so. |target_arch| should be either
        # 'x86' or 'x64'.
        assert target_arch in ('x86', 'x64')
        sdk_dir = os.environ.get('WindowsSDKDir')
        if self.sdk_based and sdk_dir:
          return [os.path.normpath(os.path.join(sdk_dir, 'Bin/SetEnv.Cmd')),
                  '/' + target_arch]
        ...

Because I have the VC vars set, WindowsSDKDir is set.  Its value is:

    C:\Program Files\Windows Kits\8.1\

MSVSVersion.SetupScript therefore returns:

    ['C:\\Program Files\\Windows Kits\\8.1\\Bin\\SetEnv.Cmd', '/x86']

My Windows Kits directory has 8.0 and 8.1 subdirectories.  There is no 
SetEnv.Cmd (or anything similar) in any of them.  (Looking just at 8.1, there 
is a Bin directory, and it has arm, x86, and x64 subdirectories.)

The lack of error handling makes things worse.  In GenerateEnvironmentFiles, we 
have this code:

    # Extract environment variables for subprocesses.
    args = vs.SetupScript(arch)
    args.extend(('&&', 'set'))
    popen = subprocess.Popen(
        args, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
    variables, _ = popen.communicate()
    env = _ExtractImportantEnvironment(variables)

We're running the command:

    ['C:\\Program Files\\Windows Kits\\8.1\\Bin\\SetEnv.Cmd', '/x86', '&&', 'set']

The command finishes with return code 1 (indicating error, even in Windows, 
AFAIK).  We combine stdout and stderr, so we don't print cmd's failure output:

    '"C:\Program Files\Windows Kits\8.1\Bin\SetEnv.Cmd"' is not recognized as an internal or external command,
    operable program or batch file.

gyp doesn't check the error code, so it calls _ExtractImportantEnvironment, 
which decides that SYSTEMROOT isn't set.  This makes sense, because SET didn't 
run.

I'd suggest replacing these lines with subprocess.check_output, which is 
simpler, but that function tries to raise a subprocess.CalledProcessError, and 
something goes horribly wrong with the threading/processes, and instead of a 
meaningful error message, we get a complaint that CalledProcessError is 
constructed with the wrong number of arguments.  (I tried raising a 
locally-defined exception class, and I got a pickling error.)  check_output 
also requires Python 2.7; not sure what gyp's requirements are.

Instead, I'd suggest this patch.  It works for me (once I unset WindowsSDKDir), 
but perhaps some other VC installs print noise to stderr, so maybe we want to 
suppress stderr instead.  stderr=subprocess.STDOUT is always wrong, though, 
unless we're expecting _ExtractImportantEnvironment to examine the setup script 
output.  To suppress stderr, I think we'd use stderr=subprocess.PIPE.  (We 
already discard popen.communicate()'s error output.)

Index: pylib/gyp/msvs_emulation.py
===================================================================
--- pylib/gyp/msvs_emulation.py (revision 2012)
+++ pylib/gyp/msvs_emulation.py (working copy)
@@ -1002,8 +1002,10 @@
     args = vs.SetupScript(arch)
     args.extend(('&&', 'set'))
     popen = subprocess.Popen(
-        args, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+        args, shell=True, stdout=subprocess.PIPE)
     variables, _ = popen.communicate()
+    if popen.returncode != 0:
+      raise Exception('Error invoking setup script: ' + repr(args))
     env = _ExtractImportantEnvironment(variables)

     # Inject system includes from gyp files into INCLUDE.

FWIW, my machine is a 32-bit Windows 7 Ultimate SP1 VM.

I hit this problem while building atom-shell, which uses gyp.  I reproduced it 
using the latest SVN source for gyp.

Original issue reported on code.google.com by ryan.pri...@gmail.com on 4 Dec 2014 at 12:28