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

Make universal extensions installable #124

Merged
merged 29 commits into from
Dec 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7d17bf7
Rewrite hpy.devel to use a separate build_hpy_ext command.
hodgestar Nov 26, 2020
bc79842
Mark proof-of-concept as not zip-safe.
hodgestar Nov 26, 2020
d6095b0
Go back to overriding build_ext.
hodgestar Nov 26, 2020
c5c4d5e
Move build_ext overriding back into fix_distribution method.
hodgestar Nov 26, 2020
77060eb
Universal modules now loadable in the ordinary fashion from within te…
hodgestar Nov 26, 2020
7f83e39
Put back pkg_resources.
hodgestar Nov 26, 2020
d37a569
Temporarily but test module into sys.modules to support universal mod…
hodgestar Nov 26, 2020
65ca244
Add a test for FatalError -- a Python in a subprocess can now import …
hodgestar Nov 26, 2020
e0589fa
Make test_FatalError syntax python2 compatible.
hodgestar Nov 26, 2020
dea8890
Update proof-of-concept tests to build and import universal modules a…
hodgestar Nov 26, 2020
28215ef
Update proof-of-concept CI tests.
hodgestar Nov 26, 2020
f66017d
Prevent bdist_egg from overwriting the universal stub files.
hodgestar Nov 28, 2020
00cd50b
Ensure the bash shell is used.
hodgestar Nov 28, 2020
6ae8cd7
Remove the explicit zip_safe flag -- this is now determined correctly.
hodgestar Nov 28, 2020
2a2343a
Fix typo in make_module docstring.
hodgestar Nov 28, 2020
46423fc
Fix typo in clean().
hodgestar Nov 28, 2020
168435b
Add object as a base class to help Python 2.
hodgestar Nov 30, 2020
095641d
Skip the remains of the FatalError test when running under pypy app-l…
hodgestar Dec 7, 2020
165cbfa
Filter extension names in a more cross-platform friendly way.
hodgestar Dec 7, 2020
5270ed4
Comment and improve the import mimic'ing in load_module.
hodgestar Dec 7, 2020
935337a
Comment the replacement of the subcommands entry for build_ext.
hodgestar Dec 7, 2020
1b0784c
Neaten up and document the hpy extension name tracking.
hodgestar Dec 7, 2020
ce57906
Add a comment explaining why .extensions needs a property.
hodgestar Dec 7, 2020
ec80d5f
Rename _BASE_DIR to _DEFAULT_BASE_DIR for clarity.
hodgestar Dec 7, 2020
73b4ab9
Import subprocess even later -- it is not importable inside pypy app-…
hodgestar Dec 8, 2020
fe5b6c4
Merge branch 'master' into feature/make-hpy-universal-extensions-inst…
hodgestar Dec 8, 2020
2829a0d
Simplify test module importing.
hodgestar Dec 8, 2020
69bbacc
Merge branch 'master' into feature/make-hpy-universal-extensions-inst…
hodgestar Dec 8, 2020
991de2e
Assert that sys.path[0] wasn't modified by the test module.
hodgestar Dec 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'



Expand Down
347 changes: 215 additions & 132 deletions hpy/devel/__init__.py
Original file line number Diff line number Diff line change
@@ -1,164 +1,247 @@
import os.path
import functools
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.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
# with both Python 2.7 and Python 3.x

_BASE_DIR = Path(__file__).parent

class HPyDevel:
def __init__(self, base_dir=_BASE_DIR):
""" Extra sources for building HPy extensions with hpy.devel. """

_DEFAULT_BASE_DIR = Path(__file__).parent

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')
# 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_include_dirs(self):
""" Extra include directories needed by extensions in both CPython and
Universal modes.
"""
return list(map(str, [
self.include_dir,
]))

def get_extra_sources(self):
return list(map(str, self._extra_sources))
""" Extra sources needed by extensions in both CPython and Universal
modes.
"""
return list(map(str, [
self.src_dir.joinpath('argparse.c'),
]))

def get_ctx_sources(self):
return list(map(str, self._ctx_sources))

def fix_extension(self, ext, hpy_abi):
""" Extra sources needed only in Universal mode.
"""
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))
return list(map(str, self.src_dir.glob('ctx_*.c')))

def fix_distribution(self, dist):
""" Override build_ext to support hpy modules.

def collect_hpy_ext_names(self, dist, hpy_ext_modules):
Used from both setup.py and hpy/test.
"""
This is sub-optimal but it should work in 99% of the cases, and complain
clearly in the others.
dist.hpydevel = self

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.
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

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).
class build_hpy_ext(build_hpy_ext_mixin, base_build_ext, object):
_base_build_ext = base_build_ext

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.::
def dist_has_ext_modules(self):
if self.ext_modules or self.hpy_ext_modules:
return True
return False

setup(ext_modules = [Extension(name='foo.mymod', ...)],
hpy_ext_modules = [Extension(name='bar.mymod', ...)],)
def build_has_ext_modules(self):
return self.distribution.has_ext_modules()

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 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)
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.cmdclass['install'] = install_hpy

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")
Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks obscure and I have no idea of what it does without looking at the source code of distutils. Maybe it's worth a comment?

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):
"""
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
if not hasattr(dist.__class__, 'hpy_abi'):
dist.__class__.hpy_abi = 'cpython'
dist.__class__.global_options += [
('hpy-abi=', None, 'Specify the HPy ABI mode (default: cpython)')
]
# 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)')
]
hpydevel = HPyDevel()
hpydevel.fix_distribution(dist)


_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

hpy_devel = HPyDevel()
hpy_devel.fix_distribution(dist, hpy_ext_modules)
__bootstrap__()
"""


class HPyExtensionName(str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice hack ;)
I remember that I and @arigo considered using something like this when we originally wrote this code, but it looked too obscure even for us. But I agree that nowadays it's a better solution than the old collect_hpy_ext_names.

It is worth a comment though, especially explaining why you can't just stick an attribute is_hpy_extension on the ext objects.

Moreover, you might want to consider to add a function is_hpy_extension which does isinstance(HPyExtensionName), it looks cleaner to me

""" 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.
"""

# 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
def extensions(self):
return self._extensions

@extensions.setter
Copy link
Collaborator

Choose a reason for hiding this comment

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

obscure. Add a comment explaining why it's needed

def extensions(self, value):
pass # ignore any attempts to change the list of extensions directly

def initialize_options(self):
self._base_build_ext.initialize_options(self)
self.hpydevel = self.distribution.hpydevel

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

@remember_hpy_extension
def get_ext_fullname(self, ext_name):
return self._base_build_ext.get_ext_fullname(self, ext_name)

@remember_hpy_extension
def get_ext_fullpath(self, ext_name):
return self._base_build_ext.get_ext_fullpath(self, ext_name)

@remember_hpy_extension
def get_ext_filename(self, ext_name):
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('.')
ext_suffix = '.hpy.so' # XXX Windows?
ext_filename = os.path.join(*ext_path) + ext_suffix
else:
ext_filename = self._base_build_ext.get_ext_filename(
self, ext_name)
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)
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:
with open(stub_file, 'w') as f:
f.write(_HPY_UNIVERSAL_MODULE_STUB_TEMPLATE.format(
ext_file=ext_file, module_name=module_name)
)
Loading