Skip to content

Commit

Permalink
Merge pull request #3297 from mwichmann/win-resourcewarn
Browse files Browse the repository at this point in the history
Fix some more subprocess-unclosed-file warnings
  • Loading branch information
bdbaddog authored Feb 15, 2019
2 parents 3931a68 + 9ce6115 commit 788e65d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 16 deletions.
6 changes: 4 additions & 2 deletions src/engine/SCons/Tool/MSCommon/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,10 @@ def get_output(vcbat, args = None, env = None):
# Use the .stdout and .stderr attributes directly because the
# .communicate() method uses the threading module on Windows
# and won't work under Pythons not built with threading.
stdout = popen.stdout.read()
stderr = popen.stderr.read()
with popen.stdout:
stdout = popen.stdout.read()
with popen.stderr:
stderr = popen.stderr.read()

# Extra debug logic, uncomment if necessary
# debug('get_output():stdout:%s'%stdout)
Expand Down
32 changes: 18 additions & 14 deletions src/engine/SCons/Tool/MSCommon/vc.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class BatchFileExecutionError(VisualCException):
"arm64" : "arm64",
"aarch64" : "arm64",
}

# get path to the cl.exe dir for newer VS versions
# based off a tuple of (host, target) platforms
_HOST_TARGET_TO_CL_DIR_GREATER_THAN_14 = {
Expand Down Expand Up @@ -135,8 +135,8 @@ class BatchFileExecutionError(VisualCException):
_CL_EXE_NAME = 'cl.exe'

def get_msvc_version_numeric(msvc_version):
"""Get the raw version numbers from a MSVC_VERSION string, so it
could be cast to float or other numeric values. For example, '14.0Exp'
"""Get the raw version numbers from a MSVC_VERSION string, so it
could be cast to float or other numeric values. For example, '14.0Exp'
would get converted to '14.0'.
Args:
Expand Down Expand Up @@ -296,7 +296,11 @@ def find_vc_pdir_vswhere(msvc_version):
vswhere_cmd = [vswhere_path, '-products', '*', '-version', msvc_version, '-property', 'installationPath']

if os.path.exists(vswhere_path):
sp = subprocess.Popen(vswhere_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
#TODO PY27 cannot use Popen as context manager
# try putting it back to the old way for now
sp = subprocess.Popen(vswhere_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
vsdir, err = sp.communicate()
if vsdir:
vsdir = vsdir.decode("mbcs").splitlines()
Expand Down Expand Up @@ -415,15 +419,15 @@ def find_batch_file(env,msvc_version,host_arch,target_arch):
__INSTALLED_VCS_RUN = None

def _check_cl_exists_in_vc_dir(env, vc_dir, msvc_version):
"""Find the cl.exe on the filesystem in the vc_dir depending on
TARGET_ARCH, HOST_ARCH and the msvc version. TARGET_ARCH and
HOST_ARCH can be extracted from the passed env, unless its None,
"""Find the cl.exe on the filesystem in the vc_dir depending on
TARGET_ARCH, HOST_ARCH and the msvc version. TARGET_ARCH and
HOST_ARCH can be extracted from the passed env, unless its None,
which then the native platform is assumed the host and target.
Args:
env: Environment
a construction environment, usually if this is passed its
because there is a desired TARGET_ARCH to be used when searching
because there is a desired TARGET_ARCH to be used when searching
for a cl.exe
vc_dir: str
the path to the VC dir in the MSVC installation
Expand All @@ -434,7 +438,7 @@ def _check_cl_exists_in_vc_dir(env, vc_dir, msvc_version):
bool:
"""

# determine if there is a specific target platform we want to build for and
# use that to find a list of valid VCs, default is host platform == target platform
# and same for if no env is specified to extract target platform from
Expand All @@ -460,7 +464,7 @@ def _check_cl_exists_in_vc_dir(env, vc_dir, msvc_version):
try:
with open(default_toolset_file) as f:
vc_specific_version = f.readlines()[0].strip()
except IOError:
except IOError:
debug('_check_cl_exists_in_vc_dir(): failed to read ' + default_toolset_file)
return False
except IndexError:
Expand All @@ -484,14 +488,14 @@ def _check_cl_exists_in_vc_dir(env, vc_dir, msvc_version):
if not host_trgt_dir:
debug('_check_cl_exists_in_vc_dir(): unsupported host/target platform combo')
return False

cl_path = os.path.join(vc_dir, 'bin', host_trgt_dir, _CL_EXE_NAME)
debug('_check_cl_exists_in_vc_dir(): checking for ' + _CL_EXE_NAME + ' at ' + cl_path)

cl_path_exists = os.path.exists(cl_path)
if not cl_path_exists and host_platform == 'amd64':
# older versions of visual studio only had x86 binaries,
# so if the host platform is amd64, we need to check cross
# older versions of visual studio only had x86 binaries,
# so if the host platform is amd64, we need to check cross
# compile options (x86 binary compiles some other target on a 64 bit os)
host_trgt_dir = _HOST_TARGET_TO_CL_DIR.get(('x86', target_platform), None)
if not host_trgt_dir:
Expand All @@ -515,7 +519,7 @@ def _check_cl_exists_in_vc_dir(env, vc_dir, msvc_version):
else:
# version not support return false
debug('_check_cl_exists_in_vc_dir(): unsupported MSVC version: ' + str(ver_num))

return False

def cached_get_installed_vcs(env=None):
Expand Down

0 comments on commit 788e65d

Please sign in to comment.