scons: dummyPopen object returned due to subprocess error causes unrelated exception in msvc/windows

Current master branch on windows. Lurking since python 3 string/bytes changes.

The dummyPopen object returned when an exception is caught in method _subproc of action.py causes cascading exceptions to be raised in the windows/msvc code path masking the original exception cause.

The issue can be reproduced by specifying an MSVC batch file that does not exist which will raise an exception:
[WinError 2] The system cannot find the file specified.

Sample SConstruct:

env = Environment(MSVC_USE_SCRIPT=r'C:\test_batch-file_does-not_exist.bat')

The causes of the cascading errors are:

• the dummyPopen object does not support the context management protocol (see issue 1 below)
• the dummyPopen object does not support returing an empty bytes string if the streams are not opened in text mode (see issue 2 below)

Given the above are resolved:

• the dummyPopen object returns an empty str/bytes for stderr (see issue 3 below)
• if the exception text is returned via stderr, there may be a missing newline (see issue 4 below)

The windows msvc code path (common.py) contains:

    # Comments removed, debug calls removed, newlines added, and code annotated

    if args:
        popen = SCons.Action._subproc(env,
                                      '"%s" %s & set' % (vcbat, args),
                                      stdin='devnull',
                                      stdout=subprocess.PIPE,
                                      stderr=subprocess.PIPE)
    else:
        popen = SCons.Action._subproc(env,
                                      '"%s" & set' % vcbat,
                                      stdin='devnull',
                                      stdout=subprocess.PIPE,
                                      stderr=subprocess.PIPE)

    with popen.stdout:                  # <- Issue 1: dummyPopen does not support context management protocol
        stdout = popen.stdout.read()
        
    with popen.stderr:                  # <- Issue 1: dummyPopen does not support context management protocol
        stderr = popen.stderr.read()

    if sys.version_info.major == 3 and sys.version_info.minor < 6:
        from ctypes import windll
        OEM = "cp{}".format(windll.kernel32.GetConsoleOutputCP())
    else:
        OEM = "oem"
        
    if stderr:                               # <- Issue 3: stderr is empty str/bytes (no useful error message)
        sys.stderr.write(stderr.decode(OEM)) 
        sys.stderr.write('\n')               # <- [ADDED] Issue 4: stderr may not have trailing newline
                                             
    if popen.wait() != 0:
        raise IOError(stderr.decode(OEM))  # <- Issue 2: str returned instead of bytes
                                           # <- Issue 3: stderr is an empty str/bytes (no useful error message)

Description of cascading errors (line numbers may be off due to added line):

• Issue 1 (context management protocol):

  AttributeError: __enter__:
      <lines removed>
      File "S:\SCons\scons\scripts\..\SCons\Tool\MSCommon\common.py", line 276:
          with popen.stdout:
  

  Adding the context management protocol resolves issue 1.

• Issue 2 (str/bytes, text-mode/binary-mode):

  AttributeError: 'str' object has no attribute 'decode':
      <lines removed>
      File "S:\SCons\scons\scripts\..\SCons\Tool\MSCommon\common.py", line 300:
          raise IOError(stderr.decode(OEM))
  

  The windows/msvc code path expects the streams to return bytes. However, the dummyPopen object returns empty strings.

  The fix for this particular problem is more involved, as some code paths explicitly open the streams in text mode.

  Fixing the str/bytes and text-mode/binary-mode problem resolves issue 2.

• Issue 3 (stderr is empty: no indication of underlying error):

  OSError: :
      <lines removed>
      File "S:\SCons\scons\scripts\..\SCons\Tool\MSCommon\common.py", line 300:
          raise IOError(stderr.decode(OEM))
  

  The exception is correctly raised. However, the dummyPopen object returns an empty str/bytes for stderr which is not particulary useful.

  One potential fix for this problem is to return the exception text via stderr. This becomes more problematic as clients of the stderr stream may use read() or readline().

  Fixing the stderr return text (via read and/or readline) resolves issue 3.

• Issue 4 (concatenated stderr output possible):

  [WinError 2] The system cannot find the file specifiedOSError: [WinError 2] The system cannot find the file specified:
      <lines removed>
      File "S:\SCons\scons\scripts\..\SCons\Tool\MSCommon\common.py", line 300:
          raise IOError(stderr.decode(OEM))
  

  Stderr is correctly populated with the exception text. However, the stderr content is written to the console stderr prior to raising the exception. Unfortunately, the exception text will not contain a trailing newline so the stderr.write() content and the exception content are effectively concatenated in the stderr stream. At worst, an extra newline is written to stderr.

Expected output (given all changes):

[WinError 2] The system cannot find the file specified
OSError: [WinError 2] The system cannot find the file specified:
    <lines removed>
    File "S:\SCons\scons\scripts\..\SCons\Tool\MSCommon\common.py", line 301:
        raise IOError(stderr.decode(OEM))

PR with potential fix(es) can be issued if desired: https://github.com/jcbrill/scons/tree/jbrill-action-dummypopen