Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gyp: cherrypick more Python3 changes from node-gyp #28563

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 22 additions & 18 deletions tools/gyp/pylib/gyp/MSVSSettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
MSBuild install directory, e.g. c:\Program Files (x86)\MSBuild
"""

from __future__ import print_function

from gyp import string_types
bnoordhuis marked this conversation as resolved.
Show resolved Hide resolved

import sys
import re

Expand Down Expand Up @@ -106,11 +110,11 @@ class _String(_Type):
"""A setting that's just a string."""

def ValidateMSVS(self, value):
if not isinstance(value, basestring):
if not isinstance(value, string_types):
raise ValueError('expected string; got %r' % value)

def ValidateMSBuild(self, value):
if not isinstance(value, basestring):
if not isinstance(value, string_types):
raise ValueError('expected string; got %r' % value)

def ConvertToMSBuild(self, value):
Expand All @@ -122,11 +126,11 @@ class _StringList(_Type):
"""A settings that's a list of strings."""

def ValidateMSVS(self, value):
if not isinstance(value, basestring) and not isinstance(value, list):
if not isinstance(value, string_types) and not isinstance(value, list):
raise ValueError('expected string list; got %r' % value)

def ValidateMSBuild(self, value):
if not isinstance(value, basestring) and not isinstance(value, list):
if not isinstance(value, string_types) and not isinstance(value, list):
raise ValueError('expected string list; got %r' % value)

def ConvertToMSBuild(self, value):
Expand Down Expand Up @@ -400,7 +404,7 @@ def _ValidateExclusionSetting(setting, settings, error_msg, stderr=sys.stderr):

if unrecognized:
# We don't know this setting. Give a warning.
print >> stderr, error_msg
print(error_msg, file=stderr)


def FixVCMacroSlashes(s):
Expand Down Expand Up @@ -433,7 +437,7 @@ def ConvertVCMacrosToMSBuild(s):
'$(PlatformName)': '$(Platform)',
'$(SafeInputName)': '%(Filename)',
}
for old, new in replace_map.iteritems():
for old, new in replace_map.items():
s = s.replace(old, new)
s = FixVCMacroSlashes(s)
return s
Expand All @@ -453,17 +457,17 @@ def ConvertToMSBuildSettings(msvs_settings, stderr=sys.stderr):
dictionaries of settings and their values.
"""
msbuild_settings = {}
for msvs_tool_name, msvs_tool_settings in msvs_settings.iteritems():
for msvs_tool_name, msvs_tool_settings in msvs_settings.items():
if msvs_tool_name in _msvs_to_msbuild_converters:
msvs_tool = _msvs_to_msbuild_converters[msvs_tool_name]
for msvs_setting, msvs_value in msvs_tool_settings.iteritems():
for msvs_setting, msvs_value in msvs_tool_settings.items():
if msvs_setting in msvs_tool:
# Invoke the translation function.
try:
msvs_tool[msvs_setting](msvs_value, msbuild_settings)
except ValueError, e:
print >> stderr, ('Warning: while converting %s/%s to MSBuild, '
'%s' % (msvs_tool_name, msvs_setting, e))
except ValueError as e:
print('Warning: while converting %s/%s to MSBuild, '
'%s' % (msvs_tool_name, msvs_setting, e), file=stderr)
cclauss marked this conversation as resolved.
Show resolved Hide resolved
else:
_ValidateExclusionSetting(msvs_setting,
msvs_tool,
Expand All @@ -472,8 +476,8 @@ def ConvertToMSBuildSettings(msvs_settings, stderr=sys.stderr):
(msvs_tool_name, msvs_setting)),
stderr)
else:
print >> stderr, ('Warning: unrecognized tool %s while converting to '
'MSBuild.' % msvs_tool_name)
print('Warning: unrecognized tool %s while converting to '
'MSBuild.' % msvs_tool_name, file=stderr)
return msbuild_settings


Expand Down Expand Up @@ -513,13 +517,13 @@ def _ValidateSettings(validators, settings, stderr):
for tool_name in settings:
if tool_name in validators:
tool_validators = validators[tool_name]
for setting, value in settings[tool_name].iteritems():
for setting, value in settings[tool_name].items():
if setting in tool_validators:
try:
tool_validators[setting](value)
except ValueError, e:
print >> stderr, ('Warning: for %s/%s, %s' %
(tool_name, setting, e))
except ValueError as e:
print('Warning: for %s/%s, %s' %
(tool_name, setting, e), file=stderr)
else:
_ValidateExclusionSetting(setting,
tool_validators,
Expand All @@ -528,7 +532,7 @@ def _ValidateSettings(validators, settings, stderr):
stderr)

else:
print >> stderr, ('Warning: unrecognized tool %s' % tool_name)
print('Warning: unrecognized tool %s' % (tool_name), file=stderr)


# MSVS and MBuild names of the tools.
Expand Down
2 changes: 1 addition & 1 deletion tools/gyp/pylib/gyp/MSVSUtil.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def InsertLargePdbShims(target_list, target_dicts, vars):

# Set up the shim to output its PDB to the same location as the final linker
# target.
for config_name, config in shim_dict.get('configurations').iteritems():
for config_name, config in shim_dict.get('configurations').items():
pdb_path = _GetPdbPath(target_dict, config_name, vars)

# A few keys that we don't want to propagate.
Expand Down
14 changes: 10 additions & 4 deletions tools/gyp/pylib/gyp/MSVSVersion.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def _RegistryQuery(key, value=None):
text = None
try:
text = _RegistryQueryBase('Sysnative', key, value)
except OSError, e:
except OSError as e:
if e.errno == errno.ENOENT:
text = _RegistryQueryBase('System32', key, value)
else:
Expand All @@ -207,12 +207,18 @@ def _RegistryGetValueUsingWinReg(key, value):
contents of the registry key's value, or None on failure. Throws
ImportError if _winreg is unavailable.
"""
import _winreg
try:
# Python 2
from _winreg import HKEY_LOCAL_MACHINE, OpenKey, QueryValueEx
except ImportError:
# Python 3
from winreg import HKEY_LOCAL_MACHINE, OpenKey, QueryValueEx

try:
root, subkey = key.split('\\', 1)
assert root == 'HKLM' # Only need HKLM for now.
with _winreg.OpenKey(_winreg.HKEY_LOCAL_MACHINE, subkey) as hkey:
return _winreg.QueryValueEx(hkey, value)[0]
with OpenKey(HKEY_LOCAL_MACHINE, subkey) as hkey:
return QueryValueEx(hkey, value)[0]
except WindowsError:
return None

Expand Down
10 changes: 4 additions & 6 deletions tools/gyp/pylib/gyp/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

from __future__ import with_statement

import collections
import errno
import filecmp
Expand Down Expand Up @@ -363,7 +361,7 @@ def close(self):
same = False
try:
same = filecmp.cmp(self.tmp_path, filename, False)
except OSError, e:
except OSError as e:
if e.errno != errno.ENOENT:
raise

Expand All @@ -382,9 +380,9 @@ def close(self):
#
# No way to get the umask without setting a new one? Set a safe one
# and then set it back to the old value.
umask = os.umask(077)
umask = os.umask(0o77)
os.umask(umask)
os.chmod(self.tmp_path, 0666 & ~umask)
os.chmod(self.tmp_path, 0o666 & ~umask)
if sys.platform == 'win32' and os.path.exists(filename):
# NOTE: on windows (but not cygwin) rename will not replace an
# existing file, so it must be preceded with a remove. Sadly there
Expand Down Expand Up @@ -467,7 +465,7 @@ def CopyTool(flavor, out_path, generator_flags={}):
''.join([source[0], header] + source[1:]))

# Make file executable.
os.chmod(tool_path, 0755)
os.chmod(tool_path, 0o755)


# From Alex Martelli,
Expand Down
3 changes: 2 additions & 1 deletion tools/gyp/pylib/gyp/easy_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import re
import os
import locale
from functools import reduce


def XmlToString(content, encoding='utf-8', pretty=False):
Expand Down Expand Up @@ -80,7 +81,7 @@ def _ConstructContentList(xml_parts, specification, pretty, level=0):
# Optionally in second position is a dictionary of the attributes.
rest = specification[1:]
if rest and isinstance(rest[0], dict):
for at, val in sorted(rest[0].iteritems()):
for at, val in sorted(rest[0].items()):
xml_parts.append(' %s="%s"' % (at, _XmlEscape(val, attr=True)))
rest = rest[1:]
if rest:
Expand Down
16 changes: 9 additions & 7 deletions tools/gyp/pylib/gyp/generator/cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
CMakeLists.txt file.
"""

from __future__ import print_function

import multiprocessing
import os
import signal
Expand Down Expand Up @@ -644,8 +646,8 @@ def WriteTarget(namer, qualified_target, target_dicts, build_dir, config_to_use,

cmake_target_type = cmake_target_type_from_gyp_target_type.get(target_type)
if cmake_target_type is None:
print ('Target %s has unknown target type %s, skipping.' %
( target_name, target_type ) )
print('Target %s has unknown target type %s, skipping.' %
( target_name, target_type))
return

SetVariable(output, 'TARGET', target_name)
Expand Down Expand Up @@ -868,8 +870,8 @@ def WriteTarget(namer, qualified_target, target_dicts, build_dir, config_to_use,
default_product_ext = generator_default_variables['SHARED_LIB_SUFFIX']

elif target_type != 'executable':
print ('ERROR: What output file should be generated?',
'type', target_type, 'target', target_name)
print('ERROR: What output file should be generated?',
'type', target_type, 'target', target_name)

product_prefix = spec.get('product_prefix', default_product_prefix)
product_name = spec.get('product_name', default_product_name)
Expand Down Expand Up @@ -1207,11 +1209,11 @@ def PerformBuild(data, configurations, params):
output_dir,
config_name))
arguments = ['cmake', '-G', 'Ninja']
print 'Generating [%s]: %s' % (config_name, arguments)
print('Generating [%s]: %s' % (config_name, arguments))
subprocess.check_call(arguments, cwd=build_dir)

arguments = ['ninja', '-C', build_dir]
print 'Building [%s]: %s' % (config_name, arguments)
print('Building [%s]: %s' % (config_name, arguments))
subprocess.check_call(arguments)


Expand Down Expand Up @@ -1239,7 +1241,7 @@ def GenerateOutput(target_list, target_dicts, data, params):
arglists.append((target_list, target_dicts, data,
params, config_name))
pool.map(CallGenerateOutputForConfig, arglists)
except KeyboardInterrupt, e:
except KeyboardInterrupt as e:
pool.terminate()
raise e
else:
Expand Down
24 changes: 13 additions & 11 deletions tools/gyp/pylib/gyp/generator/make.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
# toplevel Makefile. It may make sense to generate some .mk files on
# the side to keep the files readable.

from __future__ import print_function

import os
import re
import sys
Expand Down Expand Up @@ -650,7 +652,7 @@ def _ValidateSourcesForOSX(spec, all_sources):
basenames.setdefault(basename, []).append(source)

error = ''
for basename, files in basenames.iteritems():
for basename, files in basenames.items():
if len(files) > 1:
error += ' %s: %s\n' % (basename, ' '.join(files))

Expand Down Expand Up @@ -1202,16 +1204,16 @@ def WriteSources(self, configs, deps, sources,
cflags_c = config.get('cflags_c')
cflags_cc = config.get('cflags_cc')

self.WriteLn("# Flags passed to all source files.");
self.WriteLn("# Flags passed to all source files.")
self.WriteList(cflags, 'CFLAGS_%s' % configname)
self.WriteLn("# Flags passed to only C files.");
self.WriteLn("# Flags passed to only C files.")
self.WriteList(cflags_c, 'CFLAGS_C_%s' % configname)
self.WriteLn("# Flags passed to only C++ files.");
self.WriteLn("# Flags passed to only C++ files.")
self.WriteList(cflags_cc, 'CFLAGS_CC_%s' % configname)
if self.flavor == 'mac':
self.WriteLn("# Flags passed to only ObjC files.");
self.WriteLn("# Flags passed to only ObjC files.")
self.WriteList(cflags_objc, 'CFLAGS_OBJC_%s' % configname)
self.WriteLn("# Flags passed to only ObjC++ files.");
self.WriteLn("# Flags passed to only ObjC++ files.")
self.WriteList(cflags_objcc, 'CFLAGS_OBJCC_%s' % configname)
includes = config.get('include_dirs')
if includes:
Expand Down Expand Up @@ -1359,8 +1361,8 @@ def ComputeOutputBasename(self, spec):
elif self.type == 'none':
target = '%s.stamp' % target
elif self.type != 'executable':
print ("ERROR: What output file should be generated?",
"type", self.type, "target", target)
print("ERROR: What output file should be generated?",
"type", self.type, "target", target)

target_prefix = spec.get('product_prefix', target_prefix)
target = spec.get('product_name', target)
Expand Down Expand Up @@ -1524,7 +1526,7 @@ def WriteTarget(self, spec, configs, deps, link_deps, bundle_deps,
# Postbuilds expect to be run in the gyp file's directory, so insert an
# implicit postbuild to cd to there.
postbuilds.insert(0, gyp.common.EncodePOSIXShellList(['cd', self.path]))
for i in xrange(len(postbuilds)):
for i in range(len(postbuilds)):
if not postbuilds[i].startswith('$'):
postbuilds[i] = EscapeShellArgument(postbuilds[i])
self.WriteLn('%s: builddir := $(abs_builddir)' % QuoteSpaces(self.output))
Expand Down Expand Up @@ -1616,7 +1618,7 @@ def WriteTarget(self, spec, configs, deps, link_deps, bundle_deps,
self.WriteDoCmd([self.output_binary], deps, 'touch', part_of_all,
postbuilds=postbuilds)
else:
print "WARNING: no output for", self.type, target
print("WARNING: no output for", self.type, self.target)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why has target become self.target here?

Copy link
Contributor Author

@cclauss cclauss Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From upstream nodejs/node-gyp#1334 (comment) last year. target was an undefined name in this context because it was neither defined nor imported. However, self.target is used above and a few lines below. These don't forget self issues are fairly common.

In a compiled language this would be easily caught as compile-time error. However in a dynamic language like Python, we either find them with a linter or they when they halt our scripts at runtime.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assumed as much but wanted to confirm it was intentional :)


# Add an alias for each target (if there are any outputs).
# Installable target aliases are created below.
Expand Down Expand Up @@ -1968,7 +1970,7 @@ def PerformBuild(data, configurations, params):
if options.toplevel_dir and options.toplevel_dir != '.':
arguments += '-C', options.toplevel_dir
arguments.append('BUILDTYPE=' + config)
print 'Building [%s]: %s' % (config, arguments)
print('Building [%s]: %s' % (config, arguments))
subprocess.check_call(arguments)


Expand Down