stefanseefeld / faber

Faber
https://stefanseefeld.github.io/faber
Boost Software License 1.0
14 stars 2 forks source link

Support PyPy #21

Closed mattip closed 3 years ago

mattip commented 4 years ago

Over at boost/python there is a PR to add PyPy to CI. It doesn't work: the first problem is that PyPy returns None for distutils.sysconfig.get_config_var('LIBRARY'), so by line 98 self.lib has not been created.

Is the project open for PRs to support PyPy?

stefanseefeld commented 3 years ago

Faber is definitely open for PRs to expand its tool support. The python tool in faber expects CPython (as I only have experience with that), but I'd be happy to generalize it if that's possible. The python.lib attribute defines the name of the library that would typically be linked to (e.g. libpython3.8.so). I'm not sure what the equivalent with PyPy would be, how that can be detected (if distutils.sysconfig.get_config_var('LIBRARY') doesn't work), and how it's used.

jcpunk commented 3 years ago

I believe libpypy3-c.so is the alternative library name.

On my system:

$ pypy3
Python 3.6.9 (78d4c48fa091, Apr 30 2020, 07:55:31)
[PyPy 7.3.1 with GCC 10.0.1 20200328 (Red Hat 10.0.1-0.11)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>> import sysconfig
>>>> from pprint import pprint
>>>> pprint(sysconfig.get_config_vars()['LIBDIR'])
'/usr/lib64/pypy3-7.3/bin'

and

$ ls -l /usr/lib64/pypy3-7.3/bin/libpypy3-c.so
-rwxr-xr-x. 1 root root 60952552 Apr 30 03:54 /usr/lib64/pypy3-7.3/bin/libpypy3-c.so
stefanseefeld commented 3 years ago

OK, but how can I programmatically query the library name ? I surely can't hard-code it, as it may vary, say, if another PyPy version is used.

jcpunk commented 3 years ago

This seems to work for me

print(os.path.join(sysconfig.get_config_vars()['LIBDIR'], 'libpypy' + sys.version[0] + '-c' + sysconfig.get_config_vars()['SHLIB_SUFFIX']))

That is still a bit hard coded, but less so....

My best hint from pypy itself is : https://foss.heptapod.net/pypy/pypy/-/blob/branch/py3.7/lib-python/3/subprocess.py#L1835

mattip commented 3 years ago

In any case. I would suggest you use sysconfig rather than distutils.sysconfig as the later is probably going to move to setuptools if PEP 632 is accepted. I see that the value is missing on CPython3.8 from windows, what does faber do there?

PyPy can add 'LIBRARY' to the myriad of undocumented variables available from sysconfig.get_config_var, but that will only be available from the next release. In the mean time, the prefix libpypy3-c is a safe bet, it is in no danger of changing any time soon.

edit: it is libpypy-c for python2.7, does faber support that?

stefanseefeld commented 3 years ago

Thanks for the additional info, @mattip . Faber itself requires Python 3.6+ to run, but can certainly be instructed to support arbitrary Python versions as "tools" (on par with compiler toolchains). At present it supports Python 2.7 as well as Python 3.x, so I'm inclined to add PyPy and PyPy3 into the mix. As a starting point I'm just trying to "fix" the existing python tool as discussed above. But if PyPy differs enough from CPython in terms of functionality, it might be better to split the two into distinct tools.

mattip commented 3 years ago

Thanks for the effort. PyPy should not differ too much in terms of functionality, barring bugs. We do try hard to pass all the tests in the CPython stdlib. Some things, like sysconfig and distutils are not really well tested, thus the proposal in PEP 632. Hopefully we will get better with time.

stefanseefeld commented 3 years ago

very simple question: how would I port an existing CPython extension module (either using the C API directly or Boost.Python) to compile against PyPy ? As a starting point, I don't see a Python.h header in PyPy. Any pointers ? (OK, never mind that last question, I saw some headers being installed with pypy3, but the Python.h only included in pypy3-dev.)

mattip commented 3 years ago

travis supports PyPy, although the versions are a bit old. We recently released 7.3.2, I see the version there is 7.1.1 which was released in April 2019.

stefanseefeld commented 3 years ago

I have a draft of a PR that adds some support for PyPy to Faber: https://github.com/stefanseefeld/faber/pull/23 Unfortunately it doesn't quite work yet. Consider this simple python example.

When I build this with faber --logs=actions --logs=commands --logs==output test, I get

gcc.makedep greet.d
makedep(<scan greet.d-7f4f005ea5e0>, <source greet.c-7f4f00657c70>, cppflags=['-I/usr/include/python3.8'])
gcc.compile greet.o
gcc -I/usr/include/python3.8 -fPIC -c -o gcc-9/x86_64/shared/greet.o ./greet.c
gcc.link greet
gcc -m64 -shared -o gcc-9/x86_64/shared/greet.so gcc-9/x86_64/shared/greet.o 
python.run test
LD_LIBRARY_PATH= PYTHONPATH=gcc-9/x86_64/shared python3 ./greet_test.py

, so all looks fine. If I then configure pypy3 instead, I get

gcc.makedep greet.d
makedep(<scan greet.d-7faf9d5618e0>, <source greet.c-7faf9d5cfc10>, cppflags=['-I/usr/lib/pypy3/include'])
gcc.compile greet.o
gcc -I/usr/lib/pypy3/include -fPIC -c -o gcc-9/x86_64/shared/greet.o ./greet.c
gcc.link greet
gcc -m64 -L/usr/lib/pypy3/libs -shared -o gcc-9/x86_64/shared/greet.so gcc-9/x86_64/shared/greet.o 
python.run test
LD_LIBRARY_PATH= PYTHONPATH=gcc-9/x86_64/shared pypy3 ./greet_test.py
Traceback (most recent call last):
  File "./greet_test.py", line 9, in <module>
    import greet
ModuleNotFoundError: No module named 'greet'

which shows that the build process itself appears to be working fine, but the compiled greet module (gcc-9/x86_64/shared/greet.so) can not be imported. Any idea what might be missing ?

mattip commented 3 years ago

The so is misnamed for PyPy. I am not sure why it works on CPython, but for PyPy it must be named greet.pypy36-pp73-x86_64-linux-gnu.so (the suffix is the result of sysconfig.get_config_var('EXT_SUFFIX')). By definition the problem is with PyPy. There must be some corner case in the importer that we miss.

mattip commented 3 years ago

yup, incompatibility. I think this is by design to prevent people from shooting themselves in the foot and importing cpython modules into pypy. We would have to implement the solution in this comment to support loading CPython c-extension modules into Pyy. Here is the root cause:

 $ pypy3 -c"import importlib.machinery as m; print(m.EXTENSION_SUFFIXES)"
['.pypy36-pp73-x86_64-linux-gnu.so']

$ python3 -c"import importlib.machinery as m; print(m.EXTENSION_SUFFIXES)"
['.cpython-37m-x86_64-linux-gnu.so', '.abi3.so', '.so']
stefanseefeld commented 3 years ago

Thanks @mattip ! Indeed, I had entirely overlooked this entire point, and it merely worked by chance / accident with CPython. I have augmented the PR to adjust the extension suffix for both CPython and PyPy, and now everything appears to be working fine. I'll do a bit more testing (and augment the CI coverage) before checking this in. Once I have tagged a new release, we can shift focus onto Boost.Python.

stefanseefeld commented 3 years ago

Fixed via https://github.com/stefanseefeld/faber/commit/e5f4ff9ea45bf76126c1ccb455f1d79ea356ba2f

nulano commented 3 years ago

travis supports PyPy, although the versions are a bit old. We recently released 7.3.2, I see the version there is 7.1.1 which was released in April 2019.

Travis also has 7.3.1, but it has to be specified explicitly, e.g. pypy3.6-7.3.1. The full list is at https://docs.travis-ci.com/user/languages/python/#python-versions

There is a request to add 7.3.2 here: https://travis-ci.community/t/add-pypy-7-3-2-support/10007, linking to: https://github.com/pyenv/pyenv/issues/1699

stefanseefeld commented 3 years ago

Thanks. This may be relevant information to https://github.com/boostorg/python/pull/291 much more than to this project (Faber).

nulano commented 3 years ago

Thanks. This may be relevant information to boostorg/python#291 much more than to this project (Faber).

Looks like https://github.com/boostorg/python/pull/291 already uses 7.3.2: https://travis-ci.org/github/boostorg/python/jobs/736108678#L1274, probably because it is not installed from Travis but using apt.