From 7d17bf78f202eab5a0d6b1ead1fa40d56a497267 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 03:41:30 +0200 Subject: [PATCH 01/27] Rewrite hpy.devel to use a separate build_hpy_ext command. --- hpy/devel/__init__.py | 275 +++++++++++++++++++++--------------------- setup.py | 3 + 2 files changed, 143 insertions(+), 135 deletions(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index ba2118745..a6d8b9a76 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -1,164 +1,169 @@ import os.path from pathlib import Path -from setuptools import Extension -from distutils.errors import DistutilsError, DistutilsSetupError +from distutils import log +from distutils.command.build import build +from distutils.command.install_lib import install_lib +from distutils.errors import DistutilsError +from setuptools.command.build_ext import build_ext # NOTE: this file is also imported by PyPy tests, so it must be compatible # with both Python 2.7 and Python 3.x -_BASE_DIR = Path(__file__).parent class HPyDevel: + """ Extra sources for building HPy extensions with hpy.devel. """ + + _BASE_DIR = Path(__file__).parent + def __init__(self, base_dir=_BASE_DIR): self.base_dir = Path(base_dir) self.include_dir = self.base_dir.joinpath('include') self.src_dir = self.base_dir.joinpath('src', 'runtime') - # extra_sources are needed both in CPython and Universal mode - self._extra_sources = [ - self.src_dir.joinpath('argparse.c'), - ] - # ctx_sources are needed only in Universal mode - self._ctx_sources = list(self.src_dir.glob('ctx_*.c')) - def get_extra_sources(self): - return list(map(str, self._extra_sources)) - - def get_ctx_sources(self): - return list(map(str, self._ctx_sources)) - - def fix_extension(self, ext, hpy_abi): - """ - Modify an existing setuptools.Extension to generate an HPy module. - """ - if hasattr(ext, 'hpy_abi'): - return - ext.hpy_abi = hpy_abi - ext.include_dirs.append(str(self.include_dir)) - ext.sources += self.get_extra_sources() - if hpy_abi == 'cpython': - ext.sources += self.get_ctx_sources() - if hpy_abi == 'universal': - ext.define_macros.append(('HPY_UNIVERSAL_ABI', None)) - - def collect_hpy_ext_names(self, dist, hpy_ext_modules): + def get_extra_include_dirs(self): + """ Extra include directories needed by extensions in both CPython and + Universal modes. """ - This is sub-optimal but it should work in 99% of the cases, and complain - clearly in the others. - - In order to implement build_hpy_ext, we need to know whether an - Extension was put inside hpy_ext_modules or ext_modules, and we need - to know it ONLY by looking at its name (because that's all we get when - distutils calls build_hpy_ext.get_ext_filename). So here we collect - and return all hpy_ext_names. - - However, there is a problem: if the module is inside a package, - distutils' build_ext.get_ext_fullpath calls get_ext_filename with ONLY - the last part of the dotted name (see distutils/commands/build_ext.py). - - This means that there is a risk of conflicts if we have two ext - modules with the same name in two different packages, of which one is - HPy and the other is legacy; e.g.:: + return list(map(str, [ + self.include_dir, + ])) - setup(ext_modules = [Extension(name='foo.mymod', ...)], - hpy_ext_modules = [Extension(name='bar.mymod', ...)],) - - In that case, we cannot know whether ``mymod`` is an HPy ext module or - not. If we detect such a problem, we exit early, and the only solution - is to rename one of them :( + def get_extra_sources(self): + """ Extra sources needed by extensions in both CPython and Universal + modes. """ - def collect_ext_names(exts): - if exts is None: - return set() - names = set() - for ext in exts: - names.add(ext.name) # full name, e.g. 'foo.bar.baz' - names.add(ext.name.split('.')[-1]) # modname, e.g. 'baz' - return names - - hpy_ext_names = collect_ext_names(hpy_ext_modules) - ext_names = collect_ext_names(dist.ext_modules) - conflicts = hpy_ext_names.intersection(ext_names) - if conflicts: - lines = ['\n'] - lines.append('Name conflict between ext_modules and hpy_ext_modules:') - for name in conflicts: - lines.append(' - %s' % name) - lines.append('You can not have modules ending with the same name in both') - lines.append('ext_modules and hpy_ext_modules: this is a limitation of ') - lines.append('hpy.devel, please rename one of them.') - raise DistutilsSetupError('\n'.join(lines)) - return hpy_ext_names - - def fix_distribution(self, dist, hpy_ext_modules): - from setuptools.command.build_ext import build_ext - from setuptools.command.install import install - - def is_hpy_extension(ext_name): - return ext_name in is_hpy_extension._ext_names - is_hpy_extension._ext_names = self.collect_hpy_ext_names(dist, hpy_ext_modules) - - # add the hpy_extension modules to the normal ext_modules - if dist.ext_modules is None: - dist.ext_modules = [] - dist.ext_modules += hpy_ext_modules - - hpy_devel = self - base_build_ext = dist.cmdclass.get('build_ext', build_ext) - base_install = dist.cmdclass.get('install', install) - - class build_hpy_ext(base_build_ext): - """ - Custom distutils command which properly recognizes and handle hpy - extensions: - - - modify 'include_dirs', 'sources' and 'define_macros' depending on - the selected hpy_abi - - - modify the filename extension if we are targeting the universal - ABI. - """ - - def build_extension(self, ext): - if is_hpy_extension(ext.name): - # add the required include_dirs, sources and macros - hpy_devel.fix_extension(ext, hpy_abi=self.distribution.hpy_abi) - return base_build_ext.build_extension(self, ext) - - def get_ext_filename(self, ext_name): - # this is needed to give the .hpy.so extension to universal extensions - if is_hpy_extension(ext_name) and self.distribution.hpy_abi == 'universal': - ext_path = ext_name.split('.') - ext_suffix = '.hpy.so' # XXX Windows? - return os.path.join(*ext_path) + ext_suffix - return base_build_ext.get_ext_filename(self, ext_name) - - class install_hpy(base_install): - - def run(self): - if self.distribution.hpy_abi == 'universal': - raise DistutilsError( - 'setup.py install is not supported for HPy universal modules.\n' - ' At the moment, the only supported method is: \n' - ' setup.py --hpy-abi-universal build_ext --inplace') - return base_install.run(self) - - dist.cmdclass['build_ext'] = build_hpy_ext - dist.cmdclass['install'] = install_hpy + return list(map(str, [ + self.src_dir.joinpath('argparse.c'), + ])) + def get_ctx_sources(self): + """ Extra sources needed only in Universal mode. + """ + return list(map(str, self.src_dir.glob('ctx_*.c'))) def handle_hpy_ext_modules(dist, attr, hpy_ext_modules): - """ - setuptools entry point, see setup.py + """ Distuils hpy_ext_module setup(...) argument and --hpy-abi option. + + See hpy's setup.py where this function is registered as an entry + point. """ assert attr == 'hpy_ext_modules' - # Add a global option --hpy-abi to setup.py + base_build = dist.cmdclass.get("build", build) + base_install_lib = dist.cmdclass.get("install_lib", install_lib) + orig_has_ext_modules = base_build.has_ext_modules + orig_get_outputs = base_install_lib.get_outputs + orig_get_inputs = base_install_lib.get_inputs + + def has_hpy_ext_modules(self): + return bool(self.distribution.hpy_ext_modules) + + def has_any_ext_modules(self): + return has_hpy_ext_modules(self) or orig_has_ext_modules(self) + + def get_outputs(self): + outputs = orig_get_outputs(self) + hpy_ext_outputs = self._mutate_outputs( + self.distribution.has_hpy_ext_modules(), + 'build_hpy_ext', 'build_lib', + self.install_dir) + outputs.extend(hpy_ext_outputs) + return outputs + + def get_inputs(self): + inputs = orig_get_inputs(self) + if self.distribution.has_hpy_ext_modules(): + build_hpy_ext = self.get_finalized_command('build_hpy_ext') + inputs.extend(build_hpy_ext.get_outputs()) + return inputs + if not hasattr(dist.__class__, 'hpy_abi'): + # add a global option --hpy-abi to setup.py dist.__class__.hpy_abi = 'cpython' dist.__class__.global_options += [ ('hpy-abi=', None, 'Specify the HPy ABI mode (default: cpython)') ] + # add build_hpy_ext subcommand + base_build.sub_commands.append(("build_hpy_ext", has_hpy_ext_modules)) + base_build.has_ext_modules = has_any_ext_modules + base_install_lib.get_outputs = get_outputs + base_install_lib.get_inputs = get_inputs + + +class build_hpy_ext(build_ext): + """ Distutils command for building HPy extensions. + + See hpy's setup.py where this class is registered as an entry point. + """ - hpy_devel = HPyDevel() - hpy_devel.fix_distribution(dist, hpy_ext_modules) + description = "build HPy C extensions" + + @property + def extensions(self): + return self.distribution.hpy_ext_modules + + @extensions.setter + def extensions(self, value): + pass # ignore any attempts to change the list of extensions + + def initialize_options(self): + super(self.__class__, self).initialize_options() + self.hpydevel = HPyDevel() + + def get_outputs(self): + outputs = super(self.__class__, self).get_outputs() + print(outputs) + return outputs + + def build_extension(self, ext): + if not hasattr(ext, "hpy_abi"): + ext.hpy_abi = self.distribution.hpy_abi + ext.include_dirs += self.hpydevel.get_extra_include_dirs() + ext.sources += self.hpydevel.get_extra_sources() + if ext.hpy_abi == 'cpython': + ext.sources += self.hpydevel.get_ctx_sources() + if ext.hpy_abi == 'universal': + ext.define_macros.append(('HPY_UNIVERSAL_ABI', None)) + ext._needs_stub = True + return super(self.__class__, self).build_extension(ext) + + def get_ext_filename(self, ext_name): + # this is needed to give the .hpy.so extension to universal extensions + if self.distribution.hpy_abi == 'universal': + ext_path = ext_name.split('.') + ext_suffix = '.hpy.so' # XXX Windows? + ext_filename = os.path.join(*ext_path) + ext_suffix + log.info("Building extension: %r" % ext_filename) + return ext_filename + return super(self.__class__, self).get_ext_filename(ext_name) + + def write_stub(self, output_dir, ext, compile=False): + log.info("writing stub loader for %s to %s", ext._full_name, + output_dir) + stub_file = (os.path.join(output_dir, *ext._full_name.split('.')) + + '.py') + if compile and os.path.exists(stub_file): + raise DistutilsError(stub_file + " already exists! Please delete.") + ext_file = os.path.basename(ext._file_name) + module_name = ext_file.split(".")[0] + if not self.dry_run: + f = open(stub_file, 'w') + f.write(""" +class Spec: + def __init__(self, name, origin): + self.name = name + self.origin = origin + + +def __bootstrap__(): + global __loader__, __file__ + import sys, pkg_resources + from hpy.universal import load_from_spec + __file__ = pkg_resources.resource_filename(__name__, {ext_file!r}) + m = load_from_spec(Spec({module_name!r}, __file__)) + sys.modules[__name__] = m + +__bootstrap__() +""".format(ext_file=ext_file, module_name=module_name)) + f.close() diff --git a/setup.py b/setup.py index 37fbe2797..e4d39fceb 100644 --- a/setup.py +++ b/setup.py @@ -85,6 +85,9 @@ def get_scm_config(): "distutils.setup_keywords": [ "hpy_ext_modules = hpy.devel:handle_hpy_ext_modules", ], + "distutils.commands": [ + "build_hpy_ext = hpy.devel:build_hpy_ext", + ] }, use_scm_version = get_scm_config, setup_requires=['setuptools_scm'], From bc79842a00725fafc9bd7dbd44186dd59b25494f Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 03:41:53 +0200 Subject: [PATCH 02/27] Mark proof-of-concept as not zip-safe. --- proof-of-concept/setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/proof-of-concept/setup.py b/proof-of-concept/setup.py index bbdd940eb..10c033a5b 100644 --- a/proof-of-concept/setup.py +++ b/proof-of-concept/setup.py @@ -7,4 +7,5 @@ Extension('pofpackage.foo', sources=['pofpackage/foo.c']), ], setup_requires=['hpy.devel'], + zip_safe=False, ) From d6095b06c9304e491a4a39264c7a123c4ed27024 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 16:41:36 +0200 Subject: [PATCH 03/27] Go back to overriding build_ext. --- hpy/devel/__init__.py | 200 +++++++++++++++++++++++------------------- setup.py | 3 - 2 files changed, 110 insertions(+), 93 deletions(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index a6d8b9a76..906902578 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -2,7 +2,6 @@ from pathlib import Path from distutils import log from distutils.command.build import build -from distutils.command.install_lib import install_lib from distutils.errors import DistutilsError from setuptools.command.build_ext import build_ext @@ -51,96 +50,132 @@ def handle_hpy_ext_modules(dist, attr, hpy_ext_modules): assert attr == 'hpy_ext_modules' base_build = dist.cmdclass.get("build", build) - base_install_lib = dist.cmdclass.get("install_lib", install_lib) - orig_has_ext_modules = base_build.has_ext_modules - orig_get_outputs = base_install_lib.get_outputs - orig_get_inputs = base_install_lib.get_inputs - - def has_hpy_ext_modules(self): - return bool(self.distribution.hpy_ext_modules) - - def has_any_ext_modules(self): - return has_hpy_ext_modules(self) or orig_has_ext_modules(self) - - def get_outputs(self): - outputs = orig_get_outputs(self) - hpy_ext_outputs = self._mutate_outputs( - self.distribution.has_hpy_ext_modules(), - 'build_hpy_ext', 'build_lib', - self.install_dir) - outputs.extend(hpy_ext_outputs) - return outputs - - def get_inputs(self): - inputs = orig_get_inputs(self) - if self.distribution.has_hpy_ext_modules(): - build_hpy_ext = self.get_finalized_command('build_hpy_ext') - inputs.extend(build_hpy_ext.get_outputs()) - return inputs - - if not hasattr(dist.__class__, 'hpy_abi'): - # add a global option --hpy-abi to setup.py - dist.__class__.hpy_abi = 'cpython' - dist.__class__.global_options += [ - ('hpy-abi=', None, 'Specify the HPy ABI mode (default: cpython)') - ] - # add build_hpy_ext subcommand - base_build.sub_commands.append(("build_hpy_ext", has_hpy_ext_modules)) - base_build.has_ext_modules = has_any_ext_modules - base_install_lib.get_outputs = get_outputs - base_install_lib.get_inputs = get_inputs - - -class build_hpy_ext(build_ext): - """ Distutils command for building HPy extensions. - - See hpy's setup.py where this class is registered as an entry point. + base_build_ext = dist.cmdclass.get("build_ext", build_ext) + + class build_hpy_ext(build_hpy_ext_mixin, base_build_ext): + _base_build_ext = base_build_ext + + def dist_has_ext_modules(self): + if self.ext_modules or self.hpy_ext_modules: + return True + return False + + def build_has_ext_modules(self): + return self.distribution.has_ext_modules() + + # add a global option --hpy-abi to setup.py + dist.__class__.hpy_abi = 'cpython' + dist.__class__.global_options += [ + ('hpy-abi=', None, 'Specify the HPy ABI mode (default: cpython)') + ] + # replace build_ext subcommand + dist.cmdclass['build_ext'] = build_hpy_ext + dist.__class__.has_ext_modules = dist_has_ext_modules + base_build.has_ext_modules = build_has_ext_modules + idx = [sub[0] for sub in base_build.sub_commands].index("build_ext") + base_build.sub_commands[idx] = ("build_ext", build_has_ext_modules) + + +_HPY_UNIVERSAL_MODULE_STUB_TEMPLATE = """ +class Spec: + def __init__(self, name, origin): + self.name = name + self.origin = origin + + +def __bootstrap__(): + import sys, pkg_resources + from hpy.universal import load_from_spec + ext_filepath = pkg_resources.resource_filename(__name__, {ext_file!r}) + m = load_from_spec(Spec({module_name!r}, ext_filepath)) + m.__file__ = ext_filepath + sys.modules[__name__] = m + +__bootstrap__() +""" + + +class HPyExtensionName(str): + def split(self, *args, **kw): + result = str.split(self, *args, **kw) + return [self.__class__(s) for s in result] + + +class build_hpy_ext_mixin: + """ A mixin class for setuptools build_ext to add support for buidling + HPy extensions. """ - description = "build HPy C extensions" + _extensions = None @property def extensions(self): - return self.distribution.hpy_ext_modules + return self._extensions @extensions.setter def extensions(self, value): - pass # ignore any attempts to change the list of extensions + pass # ignore any attempts to change the list of extensions directly def initialize_options(self): - super(self.__class__, self).initialize_options() + self._base_build_ext.initialize_options(self) self.hpydevel = HPyDevel() - def get_outputs(self): - outputs = super(self.__class__, self).get_outputs() - print(outputs) - return outputs - - def build_extension(self, ext): - if not hasattr(ext, "hpy_abi"): - ext.hpy_abi = self.distribution.hpy_abi - ext.include_dirs += self.hpydevel.get_extra_include_dirs() - ext.sources += self.hpydevel.get_extra_sources() - if ext.hpy_abi == 'cpython': - ext.sources += self.hpydevel.get_ctx_sources() - if ext.hpy_abi == 'universal': - ext.define_macros.append(('HPY_UNIVERSAL_ABI', None)) - ext._needs_stub = True - return super(self.__class__, self).build_extension(ext) + def _finalize_hpy_ext(self, ext): + if hasattr(ext, "hpy_abi"): + return + ext.name = HPyExtensionName(ext.name) + ext.hpy_abi = self.distribution.hpy_abi + ext.include_dirs += self.hpydevel.get_extra_include_dirs() + ext.sources += self.hpydevel.get_extra_sources() + if ext.hpy_abi == 'cpython': + ext.sources += self.hpydevel.get_ctx_sources() + ext._hpy_needs_stub = False + if ext.hpy_abi == 'universal': + ext.define_macros.append(('HPY_UNIVERSAL_ABI', None)) + ext._hpy_needs_stub = True + + def finalize_options(self): + self._extensions = self.distribution.ext_modules or [] + hpy_ext_modules = self.distribution.hpy_ext_modules or [] + for ext in hpy_ext_modules: + self._finalize_hpy_ext(ext) + self._extensions.extend(hpy_ext_modules) + self._base_build_ext.finalize_options(self) + for ext in hpy_ext_modules: + ext._needs_stub = ext._hpy_needs_stub + + def get_ext_fullname(self, ext_name): + fullname = self._base_build_ext.get_ext_fullname(self, ext_name) + if isinstance(ext_name, HPyExtensionName): + fullname = HPyExtensionName(fullname) + return fullname + + def get_ext_fullpath(self, ext_name): + fullpath = self._base_build_ext.get_ext_fullpath(self, ext_name) + if isinstance(ext_name, HPyExtensionName): + fullpath = HPyExtensionName(fullpath) + return fullpath def get_ext_filename(self, ext_name): - # this is needed to give the .hpy.so extension to universal extensions + if not isinstance(ext_name, HPyExtensionName): + return self._base_build_ext.get_ext_filename(self, ext_name) if self.distribution.hpy_abi == 'universal': ext_path = ext_name.split('.') ext_suffix = '.hpy.so' # XXX Windows? ext_filename = os.path.join(*ext_path) + ext_suffix - log.info("Building extension: %r" % ext_filename) - return ext_filename - return super(self.__class__, self).get_ext_filename(ext_name) + else: + ext_filename = self._base_build_ext.get_ext_filename( + self, ext_name) + return HPyExtensionName(ext_filename) def write_stub(self, output_dir, ext, compile=False): - log.info("writing stub loader for %s to %s", ext._full_name, - output_dir) + if (not hasattr(ext, "hpy_abi") or + self.distribution.hpy_abi != 'universal'): + return self._base_build_ext.write_stub( + self, output_dir, ext, compile=compile) + log.info( + "writing hpy universal stub loader for %s to %s", + ext._full_name, output_dir) stub_file = (os.path.join(output_dir, *ext._full_name.split('.')) + '.py') if compile and os.path.exists(stub_file): @@ -148,22 +183,7 @@ def write_stub(self, output_dir, ext, compile=False): ext_file = os.path.basename(ext._file_name) module_name = ext_file.split(".")[0] if not self.dry_run: - f = open(stub_file, 'w') - f.write(""" -class Spec: - def __init__(self, name, origin): - self.name = name - self.origin = origin - - -def __bootstrap__(): - global __loader__, __file__ - import sys, pkg_resources - from hpy.universal import load_from_spec - __file__ = pkg_resources.resource_filename(__name__, {ext_file!r}) - m = load_from_spec(Spec({module_name!r}, __file__)) - sys.modules[__name__] = m - -__bootstrap__() -""".format(ext_file=ext_file, module_name=module_name)) - f.close() + with open(stub_file, 'w') as f: + f.write(_HPY_UNIVERSAL_MODULE_STUB_TEMPLATE.format( + ext_file=ext_file, module_name=module_name) + ) diff --git a/setup.py b/setup.py index e4d39fceb..37fbe2797 100644 --- a/setup.py +++ b/setup.py @@ -85,9 +85,6 @@ def get_scm_config(): "distutils.setup_keywords": [ "hpy_ext_modules = hpy.devel:handle_hpy_ext_modules", ], - "distutils.commands": [ - "build_hpy_ext = hpy.devel:build_hpy_ext", - ] }, use_scm_version = get_scm_config, setup_requires=['setuptools_scm'], From c5c4d5ee349996d42adf17c34aeb422cc710cccb Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 17:14:21 +0200 Subject: [PATCH 04/27] Move build_ext overriding back into fix_distribution method. --- hpy/devel/__init__.py | 58 ++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index 906902578..5e4aefbb4 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -40,6 +40,34 @@ def get_ctx_sources(self): """ return list(map(str, self.src_dir.glob('ctx_*.c'))) + def fix_distribution(self, dist): + """ Override build_ext to support hpy modules. + + Used from both setup.py and hpy/test. + """ + dist.hpydevel = self + + base_build = dist.cmdclass.get("build", build) + base_build_ext = dist.cmdclass.get("build_ext", build_ext) + + class build_hpy_ext(build_hpy_ext_mixin, base_build_ext): + _base_build_ext = base_build_ext + + def dist_has_ext_modules(self): + if self.ext_modules or self.hpy_ext_modules: + return True + return False + + def build_has_ext_modules(self): + return self.distribution.has_ext_modules() + + # replace build_ext subcommand + dist.cmdclass['build_ext'] = build_hpy_ext + dist.__class__.has_ext_modules = dist_has_ext_modules + base_build.has_ext_modules = build_has_ext_modules + idx = [sub[0] for sub in base_build.sub_commands].index("build_ext") + base_build.sub_commands[idx] = ("build_ext", build_has_ext_modules) + def handle_hpy_ext_modules(dist, attr, hpy_ext_modules): """ Distuils hpy_ext_module setup(...) argument and --hpy-abi option. @@ -49,31 +77,13 @@ def handle_hpy_ext_modules(dist, attr, hpy_ext_modules): """ assert attr == 'hpy_ext_modules' - base_build = dist.cmdclass.get("build", build) - base_build_ext = dist.cmdclass.get("build_ext", build_ext) - - class build_hpy_ext(build_hpy_ext_mixin, base_build_ext): - _base_build_ext = base_build_ext - - def dist_has_ext_modules(self): - if self.ext_modules or self.hpy_ext_modules: - return True - return False - - def build_has_ext_modules(self): - return self.distribution.has_ext_modules() - # add a global option --hpy-abi to setup.py dist.__class__.hpy_abi = 'cpython' dist.__class__.global_options += [ ('hpy-abi=', None, 'Specify the HPy ABI mode (default: cpython)') ] - # replace build_ext subcommand - dist.cmdclass['build_ext'] = build_hpy_ext - dist.__class__.has_ext_modules = dist_has_ext_modules - base_build.has_ext_modules = build_has_ext_modules - idx = [sub[0] for sub in base_build.sub_commands].index("build_ext") - base_build.sub_commands[idx] = ("build_ext", build_has_ext_modules) + hpydevel = HPyDevel() + hpydevel.fix_distribution(dist) _HPY_UNIVERSAL_MODULE_STUB_TEMPLATE = """ @@ -84,9 +94,9 @@ def __init__(self, name, origin): def __bootstrap__(): - import sys, pkg_resources + import os, sys from hpy.universal import load_from_spec - ext_filepath = pkg_resources.resource_filename(__name__, {ext_file!r}) + ext_filepath = os.path.join(os.path.dirname(__file__), {ext_file!r}) m = load_from_spec(Spec({module_name!r}, ext_filepath)) m.__file__ = ext_filepath sys.modules[__name__] = m @@ -118,7 +128,7 @@ def extensions(self, value): def initialize_options(self): self._base_build_ext.initialize_options(self) - self.hpydevel = HPyDevel() + self.hpydevel = self.distribution.hpydevel def _finalize_hpy_ext(self, ext): if hasattr(ext, "hpy_abi"): @@ -176,6 +186,8 @@ def write_stub(self, output_dir, ext, compile=False): log.info( "writing hpy universal stub loader for %s to %s", ext._full_name, output_dir) + # ignore output_dir which points to completely the wrong place + output_dir = self.build_lib stub_file = (os.path.join(output_dir, *ext._full_name.split('.')) + '.py') if compile and os.path.exists(stub_file): From 77060eb3f7098c985787dc369d142ead40f4ce50 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 17:15:02 +0200 Subject: [PATCH 05/27] Universal modules now loadable in the ordinary fashion from within tests. --- test/support.py | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/test/support.py b/test/support.py index b14e00ea0..53b7f3cff 100644 --- a/test/support.py +++ b/test/support.py @@ -198,30 +198,17 @@ def compile_module(self, ExtensionTemplate, main_src, name, extra_sources): def make_module(self, ExtensionTemplate, main_src, name, extra_sources): """ - Compile&load a modulo into memory. This is NOT a proper import: e.g. the module - is not put into sys.modules + Compile & load a modulo into memory. This is NOT a proper import: e.g. + the module is not put into sys.modules """ - so_filename = self.compile_module(ExtensionTemplate, main_src, name, - extra_sources) - if self.hpy_abi == 'universal': - return self.load_universal_module(name, so_filename) - else: - return self.load_cython_module(name, so_filename) - - def load_universal_module(self, name, so_filename): - assert self.hpy_abi == 'universal' - import hpy.universal - spec = Spec(name, so_filename) - return hpy.universal.load_from_spec(spec) - - def load_cython_module(self, name, so_filename): - assert self.hpy_abi == 'cpython' - # we've got a normal CPython module compiled with the CPython API/ABI, - # let's load it normally. It is important to do the imports only here, - # because this file will be imported also by PyPy tests which runs on - # Python2 + mod_filename = self.compile_module( + ExtensionTemplate, main_src, name, extra_sources) + return self.load_module(name, mod_filename) + + def load_module(self, name, so_filename): + # It is important to do the imports only here, because this file will + # be imported also by PyPy tests which runs on Python2 import importlib.util - from importlib.machinery import ExtensionFileLoader spec = importlib.util.spec_from_file_location(name, so_filename) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) @@ -263,6 +250,7 @@ def c_compile(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose=0, debug=None): os.environ[key] = value return outputfilename + def _build(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose=0, debug=None): # XXX compact but horrible :-( from distutils.core import Distribution @@ -280,15 +268,23 @@ def _build(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose=0, debug=None): # # this is the equivalent of passing --hpy-abi from setup.py's command line dist.hpy_abi = hpy_abi - hpy_devel.fix_distribution(dist, hpy_ext_modules=[ext]) + dist.hpy_ext_modules = [ext] + hpy_devel.fix_distribution(dist) # old_level = distutils.log.set_threshold(0) or 0 try: distutils.log.set_verbosity(compiler_verbose) dist.run_command('build_ext') cmd_obj = dist.get_command_obj('build_ext') - [soname] = cmd_obj.get_outputs() + if hpy_abi == "cpython": + [mod_filename] = [ + x for x in cmd_obj.get_outputs() if x.endswith(".so") + ] + else: + [mod_filename] = [ + x for x in cmd_obj.get_outputs() if x.endswith(".py") + ] finally: distutils.log.set_threshold(old_level) # - return soname + return mod_filename From 7f83e39d1e5b192ba395d554d31f723f545720c0 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 17:27:01 +0200 Subject: [PATCH 06/27] Put back pkg_resources. --- hpy/devel/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index 5e4aefbb4..887dc9eb2 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -94,9 +94,9 @@ def __init__(self, name, origin): def __bootstrap__(): - import os, sys + import sys, pkg_resources from hpy.universal import load_from_spec - ext_filepath = os.path.join(os.path.dirname(__file__), {ext_file!r}) + ext_filepath = pkg_resources.resource_filename(__name__, {ext_file!r}) m = load_from_spec(Spec({module_name!r}, ext_filepath)) m.__file__ = ext_filepath sys.modules[__name__] = m From d37a569c57133e50606f58be9461027522fed5cf Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 17:27:35 +0200 Subject: [PATCH 07/27] Temporarily but test module into sys.modules to support universal module loading. --- test/support.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/support.py b/test/support.py index 53b7f3cff..94f464b07 100644 --- a/test/support.py +++ b/test/support.py @@ -209,10 +209,12 @@ def load_module(self, name, so_filename): # It is important to do the imports only here, because this file will # be imported also by PyPy tests which runs on Python2 import importlib.util + import sys spec = importlib.util.spec_from_file_location(name, so_filename) module = importlib.util.module_from_spec(spec) + sys.modules[name] = module spec.loader.exec_module(module) - return module + return sys.modules.pop(name) @pytest.mark.usefixtures('initargs') @@ -254,7 +256,8 @@ def c_compile(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose=0, debug=None): def _build(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose=0, debug=None): # XXX compact but horrible :-( from distutils.core import Distribution - import distutils.errors, distutils.log + import distutils.errors + import distutils.log # dist = Distribution() dist.parse_config_files() From 65ca2447cff2b7c3cb2ebd0cf012582acdf6e306 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 17:28:21 +0200 Subject: [PATCH 08/27] Add a test for FatalError -- a Python in a subprocess can now import universal test modules too. --- test/test_hpyerr.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/test_hpyerr.py b/test/test_hpyerr.py index 529d299e8..a61ecefef 100644 --- a/test/test_hpyerr.py +++ b/test/test_hpyerr.py @@ -20,6 +20,9 @@ def test_NoMemory(self): mod.f() def test_FatalError(self): + import os + import subprocess + import sys mod = self.make_module(""" HPyDef_METH(f, "f", f_impl, HPyFunc_NOARGS) static HPy f_impl(HPyContext ctx, HPy self) @@ -33,8 +36,14 @@ def test_FatalError(self): @EXPORT(f) @INIT """) - # Calling mod.f() gives a fatal error, ending in abort(). - # How to check that? For now we just check that the above compiles + result = subprocess.run([ + sys.executable, "-c", f"import {mod.__name__} as mod; mod.f()" + ], env={ + "PYTHONPATH": os.path.dirname(mod.__file__), + }, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + assert result.returncode == -6 + assert result.stdout == b"" + assert result.stderr.startswith(b"Fatal Python error: boom!\n") def test_HPyErr_Occurred(self): import pytest From e0589fa396f0673488824ae35656836d26a16dd0 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 17:52:30 +0200 Subject: [PATCH 09/27] Make test_FatalError syntax python2 compatible. --- test/test_hpyerr.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_hpyerr.py b/test/test_hpyerr.py index a61ecefef..90b29a7ab 100644 --- a/test/test_hpyerr.py +++ b/test/test_hpyerr.py @@ -37,7 +37,8 @@ def test_FatalError(self): @INIT """) result = subprocess.run([ - sys.executable, "-c", f"import {mod.__name__} as mod; mod.f()" + sys.executable, + "-c", "import {} as mod; mod.f()".format(mod.__name__) ], env={ "PYTHONPATH": os.path.dirname(mod.__file__), }, stdout=subprocess.PIPE, stderr=subprocess.PIPE) From dea8890522eb4ad4a53a4934e7bc8de97837f817 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 18:31:55 +0200 Subject: [PATCH 10/27] Update proof-of-concept tests to build and import universal modules as normal. --- proof-of-concept/test_pof.py | 28 ++------------- proof-of-concept/test_pof.sh | 67 +++++++++++++++--------------------- 2 files changed, 30 insertions(+), 65 deletions(-) diff --git a/proof-of-concept/test_pof.py b/proof-of-concept/test_pof.py index 31734f231..e0f0c53af 100644 --- a/proof-of-concept/test_pof.py +++ b/proof-of-concept/test_pof.py @@ -1,30 +1,6 @@ -import os +import pof +import pofpackage.foo -def import_universal_pof(): - # so far, we need to manually load&import universal modules. Eventually, - # this should be supported directly by hpy, but it's still unclear how. - import pathlib - import hpy.universal - root = pathlib.Path(__file__).parent - class PofSpec: - name = 'pof' - origin = str(root.joinpath('pof.hpy.so')) - class FooSpec: - name = 'pofpackage.foo' - origin = str(root.joinpath('pofpackage', 'foo.hpy.so')) - - import pofpackage - pof = hpy.universal.load_from_spec(PofSpec) - pofpackage.foo = hpy.universal.load_from_spec(FooSpec) - return pof, pofpackage - - -if os.getenv('HPY_UNIVERSAL') == '1': - pof, pofpackage = import_universal_pof() -else: - # import pof modules as normal - import pof - import pofpackage.foo def test_do_nothing(): assert pof.do_nothing() is None diff --git a/proof-of-concept/test_pof.sh b/proof-of-concept/test_pof.sh index 19ff76d12..6e1f0dd04 100755 --- a/proof-of-concept/test_pof.sh +++ b/proof-of-concept/test_pof.sh @@ -15,18 +15,20 @@ _install_hpy() { _test_pof() { echo "==== testing pof ====" # this assumes that pof is already installed, e.g. after calling - # install_pof_wheel + # wheel or setup_py_install python3 -m pip install pytest pytest-azurepipelines cd proof-of-concept python3 -m pytest } _build_wheel() { + HPY_ABI="$1" + VENV="venv/wheel_builder_$HPY_ABI" # we use this venv just to build the wheel, and then we install the wheel # in the currently active virtualenv - echo "Create venv: wheel_builder" - python3 -m venv venv/wheel_builder - PY_BUILDER="`pwd`/venv/wheel_builder/bin/python3" + echo "Create venv: $VENV" + python3 -m venv "$VENV" + PY_BUILDER="`pwd`/$VENV/bin/python3" echo echo "Installing hpy and requirements" _install_hpy ${PY_BUILDER} @@ -34,7 +36,7 @@ _build_wheel() { ${PY_BUILDER} -m pip install -r requirements.txt echo echo "Building wheel" - ${PY_BUILDER} setup.py bdist_wheel + ${PY_BUILDER} setup.py --hpy-abi="$HPY_ABI" bdist_wheel popd } @@ -50,10 +52,12 @@ _myrm() { clean() { echo "=== cleaning up old stuff ===" - _myrm ${ROOT}/venv/wheel_builder - _myrm ${ROOT}/venv/wheel_runner - _myrm ${ROOT}/venv/cpython_abi - _myrm ${ROOT}/venv/universal_abi + _myrm ${ROOT}/venv/wheel_builder_cpython + _myrm ${ROOT}/venv/wheel_builder_universal + _myrm ${ROOT}/venv/wheel_runner_cpython + _myrm ${ROOT}/venv/wheel_runner_universal + _myrm ${ROOT}/venv/setup_py_install_cpython + _myrm ${ROOT}/venv/setup_pu_install_universal _myrm ${ROOT}/build _myrm ${ROOT}/proof-of-concept/build _myrm ${ROOT}/proof-of-concept/dist @@ -62,57 +66,42 @@ clean() { wheel() { # build a wheel, install and test + HPY_ABI="$1" + VENV="venv/wheel_runner_$HPY_ABI" clean echo "=== testing setup.py bdist_wheel ===" - _build_wheel + _build_wheel "$HPY_ABI" WHEEL=`ls proof-of-concept/dist/*.whl` echo "Wheel created: ${WHEEL}" echo - echo "Create venv: wheel_runner" - python3 -m venv venv/wheel_runner - source venv/wheel_runner/bin/activate + echo "Create venv: $VENV" + python3 -m venv "$VENV" + source "$VENV/bin/activate" echo "Installing wheel" python3 -m pip install $WHEEL echo _test_pof } -setup_py_cpython() { - # install using setup.py, CPython ABI +setup_py_install() { + # install proof-of-concept using setup.py install and run tests + HPY_ABI="$1" + VENV="venv/setup_py_install_$HPY_ABI" clean - echo "=== testing setup.py --hpy-abi=cpython install ===" - echo "Create venv: cpython_abi" - python3 -m venv venv/cpython_abi - source venv/cpython_abi/bin/activate + echo "=== testing setup.py --hpy-abi=$HPY_ABI install ===" + echo "Create venv: $VENV" + python3 -m venv "$VENV" + source "$VENV/bin/activate" _install_hpy python echo echo "Running setup.py" pushd proof-of-concept - python3 setup.py --hpy-abi=cpython install + python3 setup.py --hpy-abi="$HPY_ABI" install popd echo _test_pof } -build_ext_inplace_universal() { - # install using setup.py build_ext -i, Universal ABI - clean - echo "=== testing setup.py --hpy-abi=universal build_ext -i ===" - echo "Create venv: universal_abi" - python3 -m venv venv/universal_abi - source venv/universal_abi/bin/activate - _install_hpy python - echo - echo "Running setup.py" - pushd proof-of-concept - python3 setup.py --hpy-abi=universal build_ext --inplace - popd - echo - export HPY_UNIVERSAL=1 - _test_pof -} - - # ======== main code ======= # call the function mentioned as the first arg From 28215efe59543144585a1a4df502d60b9875cb90 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Thu, 26 Nov 2020 18:32:12 +0200 Subject: [PATCH 11/27] Update proof-of-concept CI tests. --- azure-pipelines.yml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f3437e002..8085bac82 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -58,12 +58,14 @@ jobs: - template: azure-templates/python.yml parameters: pythonVersion: $(python.version) - - script: proof-of-concept/test_pof.sh wheel - displayName: 'Test bdist_wheel' - - script: proof-of-concept/test_pof.sh setup_py_cpython + - script: proof-of-concept/test_pof.sh wheel cpython + displayName: 'Test setup.py --hpy-abi=cpython bdist_wheel' + - script: proof-of-concept/test_pof.sh wheel universal + displayName: 'Test setup.py --hpy-abi=universal bdist_wheel' + - script: proof-of-concept/test_pof.sh setup_py_install cpython displayName: 'Test setup.py --hpy-abi=cpython install' - - script: proof-of-concept/test_pof.sh build_ext_inplace_universal - displayName: 'Test setup.py --hpy-abi=universal build_ext --inplace' + - script: proof-of-concept/test_pof.sh setup_py_install universal + displayName: 'Test setup.py --hpy-abi=universal install' From f66017df759c66195b392a5bf2ffde27b21a9dc3 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Sat, 28 Nov 2020 10:54:50 +0200 Subject: [PATCH 12/27] Prevent bdist_egg from overwriting the universal stub files. --- hpy/devel/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index 887dc9eb2..04dd7e1e9 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -3,6 +3,7 @@ from distutils import log from distutils.command.build import build from distutils.errors import DistutilsError +from setuptools.command import bdist_egg as bdist_egg_mod from setuptools.command.build_ext import build_ext # NOTE: this file is also imported by PyPy tests, so it must be compatible @@ -49,6 +50,7 @@ def fix_distribution(self, dist): base_build = dist.cmdclass.get("build", build) base_build_ext = dist.cmdclass.get("build_ext", build_ext) + orig_bdist_egg_write_stub = bdist_egg_mod.write_stub class build_hpy_ext(build_hpy_ext_mixin, base_build_ext): _base_build_ext = base_build_ext @@ -61,12 +63,19 @@ def dist_has_ext_modules(self): def build_has_ext_modules(self): return self.distribution.has_ext_modules() + def bdist_egg_write_stub(resource, pyfile): + if resource.endswith(".hpy.so"): + log.info("stub file already created for %s", resource) + return + orig_bdist_egg_write_stub(resource, pyfile) + # replace build_ext subcommand dist.cmdclass['build_ext'] = build_hpy_ext dist.__class__.has_ext_modules = dist_has_ext_modules base_build.has_ext_modules = build_has_ext_modules idx = [sub[0] for sub in base_build.sub_commands].index("build_ext") base_build.sub_commands[idx] = ("build_ext", build_has_ext_modules) + bdist_egg_mod.write_stub = bdist_egg_write_stub def handle_hpy_ext_modules(dist, attr, hpy_ext_modules): From 00cd50b63f87f4248dc1be2a357b2d906a374c21 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Sat, 28 Nov 2020 10:55:17 +0200 Subject: [PATCH 13/27] Ensure the bash shell is used. --- proof-of-concept/test_pof.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/proof-of-concept/test_pof.sh b/proof-of-concept/test_pof.sh index 6e1f0dd04..94aafd2aa 100755 --- a/proof-of-concept/test_pof.sh +++ b/proof-of-concept/test_pof.sh @@ -1,3 +1,4 @@ +#!/bin/bash set -e ROOT=`pwd` # we expect this script to be run from the repo root From 6ae8cd705c3aa368a4e973fb68cb403dae2229f7 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Sat, 28 Nov 2020 10:55:53 +0200 Subject: [PATCH 14/27] Remove the explicit zip_safe flag -- this is now determined correctly. --- proof-of-concept/setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/proof-of-concept/setup.py b/proof-of-concept/setup.py index 10c033a5b..bbdd940eb 100644 --- a/proof-of-concept/setup.py +++ b/proof-of-concept/setup.py @@ -7,5 +7,4 @@ Extension('pofpackage.foo', sources=['pofpackage/foo.c']), ], setup_requires=['hpy.devel'], - zip_safe=False, ) From 2a2343ab601b5113159cf5c80fd95ad517d5a0df Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Sat, 28 Nov 2020 11:02:31 +0200 Subject: [PATCH 15/27] Fix typo in make_module docstring. --- test/support.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support.py b/test/support.py index 94f464b07..d12392d7d 100644 --- a/test/support.py +++ b/test/support.py @@ -198,7 +198,7 @@ def compile_module(self, ExtensionTemplate, main_src, name, extra_sources): def make_module(self, ExtensionTemplate, main_src, name, extra_sources): """ - Compile & load a modulo into memory. This is NOT a proper import: e.g. + Compile & load a module. This is NOT a proper import: e.g. the module is not put into sys.modules """ mod_filename = self.compile_module( From 46423fc192758e289f821cd2a43606c9ea6215da Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Sat, 28 Nov 2020 11:15:41 +0200 Subject: [PATCH 16/27] Fix typo in clean(). --- proof-of-concept/test_pof.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proof-of-concept/test_pof.sh b/proof-of-concept/test_pof.sh index 94aafd2aa..40cae87c8 100755 --- a/proof-of-concept/test_pof.sh +++ b/proof-of-concept/test_pof.sh @@ -58,7 +58,7 @@ clean() { _myrm ${ROOT}/venv/wheel_runner_cpython _myrm ${ROOT}/venv/wheel_runner_universal _myrm ${ROOT}/venv/setup_py_install_cpython - _myrm ${ROOT}/venv/setup_pu_install_universal + _myrm ${ROOT}/venv/setup_py_install_universal _myrm ${ROOT}/build _myrm ${ROOT}/proof-of-concept/build _myrm ${ROOT}/proof-of-concept/dist From 168435be3787ab6214665faaefb02f593f2de748 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 30 Nov 2020 17:23:05 +0200 Subject: [PATCH 17/27] Add object as a base class to help Python 2. --- hpy/devel/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index 04dd7e1e9..7e5ad893a 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -52,7 +52,7 @@ def fix_distribution(self, dist): base_build_ext = dist.cmdclass.get("build_ext", build_ext) orig_bdist_egg_write_stub = bdist_egg_mod.write_stub - class build_hpy_ext(build_hpy_ext_mixin, base_build_ext): + class build_hpy_ext(build_hpy_ext_mixin, base_build_ext, object): _base_build_ext = base_build_ext def dist_has_ext_modules(self): From 095641d2002286610eb6c64e801a255a8ed40181 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 15:42:10 +0200 Subject: [PATCH 18/27] Skip the remains of the FatalError test when running under pypy app-level tests. --- test/test_hpyerr.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_hpyerr.py b/test/test_hpyerr.py index 90b29a7ab..5e2337b15 100644 --- a/test/test_hpyerr.py +++ b/test/test_hpyerr.py @@ -36,6 +36,10 @@ def test_FatalError(self): @EXPORT(f) @INIT """) + if not hasattr(sys, "executable"): + # if sys.executable is not available (e.g. inside pypy app-level) + # tests, then skip the rest of this test + return result = subprocess.run([ sys.executable, "-c", "import {} as mod; mod.f()".format(mod.__name__) From 165cbfacda71fd457d7de415fcc02691c68bca60 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 16:00:46 +0200 Subject: [PATCH 19/27] Filter extension names in a more cross-platform friendly way. --- test/support.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/support.py b/test/support.py index d12392d7d..58c2b0213 100644 --- a/test/support.py +++ b/test/support.py @@ -268,26 +268,23 @@ def _build(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose=0, debug=None): options['force'] = ('ffiplatform', True) options['build_lib'] = ('ffiplatform', tmpdir) options['build_temp'] = ('ffiplatform', tmpdir) - # + # this is the equivalent of passing --hpy-abi from setup.py's command line dist.hpy_abi = hpy_abi dist.hpy_ext_modules = [ext] hpy_devel.fix_distribution(dist) - # + old_level = distutils.log.set_threshold(0) or 0 try: distutils.log.set_verbosity(compiler_verbose) dist.run_command('build_ext') cmd_obj = dist.get_command_obj('build_ext') + outputs = cmd_obj.get_outputs() if hpy_abi == "cpython": - [mod_filename] = [ - x for x in cmd_obj.get_outputs() if x.endswith(".so") - ] + [mod_filename] = [x for x in outputs if not x.endswith(".py")] else: - [mod_filename] = [ - x for x in cmd_obj.get_outputs() if x.endswith(".py") - ] + [mod_filename] = [x for x in outputs if x.endswith(".py")] finally: distutils.log.set_threshold(old_level) - # + return mod_filename From 5270ed4053b32de658c1b4fc5692a0798951324f Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 16:43:12 +0200 Subject: [PATCH 20/27] Comment and improve the import mimic'ing in load_module. --- test/support.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/support.py b/test/support.py index 58c2b0213..436a2b9b6 100644 --- a/test/support.py +++ b/test/support.py @@ -210,11 +210,25 @@ def load_module(self, name, so_filename): # be imported also by PyPy tests which runs on Python2 import importlib.util import sys + if name in sys.modules: + raise ValueError( + "Test module {!r} already present in sys.modules".format(name)) spec = importlib.util.spec_from_file_location(name, so_filename) module = importlib.util.module_from_spec(spec) + # the ordinary Python import machinery adds the module to sys.modules + # before executing the module, and pkg_resources (which is used in the + # universal module stub .py file) relies on this, so we do the same + # here. sys.modules[name] = module - spec.loader.exec_module(module) - return sys.modules.pop(name) + try: + spec.loader.exec_module(module) + finally: + # the universal module stub .py file places the newly loaded HPy + # extension module in sys.modules so we retrieve it from there + # (for cpython modules, the module was already placed in + # sys.modules earlier in this function) + module = sys.modules.pop(name) + return module @pytest.mark.usefixtures('initargs') From 935337a28a2531b985712cb659a4b9b573608b4c Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 16:53:23 +0200 Subject: [PATCH 21/27] Comment the replacement of the subcommands entry for build_ext. --- hpy/devel/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index 7e5ad893a..3e8d14edd 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -73,6 +73,9 @@ def bdist_egg_write_stub(resource, pyfile): dist.cmdclass['build_ext'] = build_hpy_ext dist.__class__.has_ext_modules = dist_has_ext_modules base_build.has_ext_modules = build_has_ext_modules + # setuptools / distutils store subcommands in .subcommands which + # is a list of tuples of (extension_name, extension_needs_to_run_func). + # The two lines below replace .subcommand entry for build_ext. idx = [sub[0] for sub in base_build.sub_commands].index("build_ext") base_build.sub_commands[idx] = ("build_ext", build_has_ext_modules) bdist_egg_mod.write_stub = bdist_egg_write_stub From 1b0784ccc7aa043087316f9df952c6c07e776ce8 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 17:33:50 +0200 Subject: [PATCH 22/27] Neaten up and document the hpy extension name tracking. --- hpy/devel/__init__.py | 51 +++++++++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index 3e8d14edd..d8029bdfd 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -1,4 +1,5 @@ import os.path +import functools from pathlib import Path from distutils import log from distutils.command.build import build @@ -118,11 +119,40 @@ def __bootstrap__(): class HPyExtensionName(str): + """ Wrapper around str to allow HPy extension modules to be identified. + + The following build_ext command methods are passed only the *name* + of the extension and not the full extension object. The + build_hpy_ext_mixin class needs to detect when HPy are extensions + passed to these methods and override the default behaviour. + + This str sub-class allows HPy extensions to be detected, while + still allowing the extension name to be used as an ordinary string. + """ + def split(self, *args, **kw): result = str.split(self, *args, **kw) return [self.__class__(s) for s in result] +def is_hpy_extension(ext_name): + """ Return True if the extension name is for an HPy extension. """ + return isinstance(ext_name, HPyExtensionName) + + +def remember_hpy_extension(f): + """ Decorator for remembering whether an extension name belongs to an + HPy extension. + """ + @functools.wraps(f) + def wrapper(self, ext_name): + result = f(self, ext_name) + if is_hpy_extension(ext_name): + result = HPyExtensionName(result) + return result + return wrapper + + class build_hpy_ext_mixin: """ A mixin class for setuptools build_ext to add support for buidling HPy extensions. @@ -166,20 +196,17 @@ def finalize_options(self): for ext in hpy_ext_modules: ext._needs_stub = ext._hpy_needs_stub + @remember_hpy_extension def get_ext_fullname(self, ext_name): - fullname = self._base_build_ext.get_ext_fullname(self, ext_name) - if isinstance(ext_name, HPyExtensionName): - fullname = HPyExtensionName(fullname) - return fullname + return self._base_build_ext.get_ext_fullname(self, ext_name) + @remember_hpy_extension def get_ext_fullpath(self, ext_name): - fullpath = self._base_build_ext.get_ext_fullpath(self, ext_name) - if isinstance(ext_name, HPyExtensionName): - fullpath = HPyExtensionName(fullpath) - return fullpath + return self._base_build_ext.get_ext_fullpath(self, ext_name) + @remember_hpy_extension def get_ext_filename(self, ext_name): - if not isinstance(ext_name, HPyExtensionName): + if not is_hpy_extension(ext_name): return self._base_build_ext.get_ext_filename(self, ext_name) if self.distribution.hpy_abi == 'universal': ext_path = ext_name.split('.') @@ -188,18 +215,18 @@ def get_ext_filename(self, ext_name): else: ext_filename = self._base_build_ext.get_ext_filename( self, ext_name) - return HPyExtensionName(ext_filename) + return ext_filename def write_stub(self, output_dir, ext, compile=False): if (not hasattr(ext, "hpy_abi") or self.distribution.hpy_abi != 'universal'): return self._base_build_ext.write_stub( self, output_dir, ext, compile=compile) + # ignore output_dir which points to completely the wrong place + output_dir = self.build_lib log.info( "writing hpy universal stub loader for %s to %s", ext._full_name, output_dir) - # ignore output_dir which points to completely the wrong place - output_dir = self.build_lib stub_file = (os.path.join(output_dir, *ext._full_name.split('.')) + '.py') if compile and os.path.exists(stub_file): From ce57906accce83cd17e2f3e299cd88fad41c9d1a Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 17:40:42 +0200 Subject: [PATCH 23/27] Add a comment explaining why .extensions needs a property. --- hpy/devel/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index d8029bdfd..d567e6cc1 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -158,6 +158,13 @@ class build_hpy_ext_mixin: HPy extensions. """ + # Ideally we would have simply added the HPy extensions to .extensions + # at the end of .initialize_options() but the setuptools build_ext + # .finalize_options both iterate over and needless overwrite the + # .extensions attribute, so we hide the full extension list in + # ._extensions and expose it as a settable property that ignores attempts + # to overwrite it: + _extensions = None @property From ec80d5ff19a40a6c6559ba6862143241fa14d798 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Mon, 7 Dec 2020 17:41:50 +0200 Subject: [PATCH 24/27] Rename _BASE_DIR to _DEFAULT_BASE_DIR for clarity. --- hpy/devel/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hpy/devel/__init__.py b/hpy/devel/__init__.py index d567e6cc1..21af0b9ef 100644 --- a/hpy/devel/__init__.py +++ b/hpy/devel/__init__.py @@ -14,9 +14,9 @@ class HPyDevel: """ Extra sources for building HPy extensions with hpy.devel. """ - _BASE_DIR = Path(__file__).parent + _DEFAULT_BASE_DIR = Path(__file__).parent - def __init__(self, base_dir=_BASE_DIR): + def __init__(self, base_dir=_DEFAULT_BASE_DIR): self.base_dir = Path(base_dir) self.include_dir = self.base_dir.joinpath('include') self.src_dir = self.base_dir.joinpath('src', 'runtime') From 73b4ab9fc7ccbe3ac4ae17044dcccd77238c81b5 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Tue, 8 Dec 2020 15:27:36 +0200 Subject: [PATCH 25/27] Import subprocess even later -- it is not importable inside pypy app-level tests. --- test/test_hpyerr.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_hpyerr.py b/test/test_hpyerr.py index 5e2337b15..c7b1d2471 100644 --- a/test/test_hpyerr.py +++ b/test/test_hpyerr.py @@ -21,7 +21,6 @@ def test_NoMemory(self): def test_FatalError(self): import os - import subprocess import sys mod = self.make_module(""" HPyDef_METH(f, "f", f_impl, HPyFunc_NOARGS) @@ -40,6 +39,8 @@ def test_FatalError(self): # if sys.executable is not available (e.g. inside pypy app-level) # tests, then skip the rest of this test return + # subprocess is not importable in pypy app-level tests + import subprocess result = subprocess.run([ sys.executable, "-c", "import {} as mod; mod.f()".format(mod.__name__) From 2829a0de82246224e9752b78ed6ff776d8924e94 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Tue, 8 Dec 2020 16:18:49 +0200 Subject: [PATCH 26/27] Simplify test module importing. --- test/support.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/test/support.py b/test/support.py index 436a2b9b6..8bb7d5f82 100644 --- a/test/support.py +++ b/test/support.py @@ -205,29 +205,23 @@ def make_module(self, ExtensionTemplate, main_src, name, extra_sources): ExtensionTemplate, main_src, name, extra_sources) return self.load_module(name, mod_filename) - def load_module(self, name, so_filename): + def load_module(self, name, mod_filename): # It is important to do the imports only here, because this file will # be imported also by PyPy tests which runs on Python2 - import importlib.util + import importlib import sys + import os if name in sys.modules: raise ValueError( "Test module {!r} already present in sys.modules".format(name)) - spec = importlib.util.spec_from_file_location(name, so_filename) - module = importlib.util.module_from_spec(spec) - # the ordinary Python import machinery adds the module to sys.modules - # before executing the module, and pkg_resources (which is used in the - # universal module stub .py file) relies on this, so we do the same - # here. - sys.modules[name] = module + importlib.invalidate_caches() try: - spec.loader.exec_module(module) + sys.path.insert(0, os.path.dirname(mod_filename)) + module = importlib.import_module(name) finally: - # the universal module stub .py file places the newly loaded HPy - # extension module in sys.modules so we retrieve it from there - # (for cpython modules, the module was already placed in - # sys.modules earlier in this function) - module = sys.modules.pop(name) + del sys.path[0] + if name in sys.modules: + del sys.modules[name] return module From 991de2e49d1e8a64a665903ad8166e596a3c2b53 Mon Sep 17 00:00:00 2001 From: Simon Cross Date: Wed, 9 Dec 2020 17:08:26 +0200 Subject: [PATCH 27/27] Assert that sys.path[0] wasn't modified by the test module. --- test/support.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/support.py b/test/support.py index 8bb7d5f82..417320a54 100644 --- a/test/support.py +++ b/test/support.py @@ -215,10 +215,14 @@ def load_module(self, name, mod_filename): raise ValueError( "Test module {!r} already present in sys.modules".format(name)) importlib.invalidate_caches() + mod_dir = os.path.dirname(mod_filename) try: - sys.path.insert(0, os.path.dirname(mod_filename)) + sys.path.insert(0, mod_dir) module = importlib.import_module(name) finally: + # assert that the module import didn't change the sys.path entry + # that was added above, then remove the entry. + assert sys.path[0] == mod_dir del sys.path[0] if name in sys.modules: del sys.modules[name]