stefanseefeld / faber

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

Add support for PyPy to 'python' tool. #23

Closed stefanseefeld closed 3 years ago

codecov-io commented 3 years ago

Codecov Report

Merging #23 into develop will decrease coverage by 0.06%. The diff coverage is 63.15%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #23      +/-   ##
===========================================
- Coverage    72.91%   72.84%   -0.07%     
===========================================
  Files           71       71              
  Lines         4065     4081      +16     
===========================================
+ Hits          2964     2973       +9     
- Misses        1101     1108       +7     
Impacted Files Coverage Δ
src/faber/tools/python.py 76.40% <61.11%> (-4.16%) :arrow_down:
src/faber/artefacts/python.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ff4f894...8b83e6b. Read the comment docs.

mattip commented 3 years ago

You may want to think about changing

    def check_python(self, cmd):
        return subprocess.check_output([self.command, '-c', cmd]).decode().strip()

    def check_sysconfig(self, cmd):
        r = self.check_python('import distutils.sysconfig as c; print(c.{})'.format(cmd))
        return r if r != 'None' else ''
stefanseefeld commented 3 years ago

Thanks for the suggestions. As to the difference between sysconfig and distutils.sysconfig, there are unfortunately a few crucial additions in the latter that I need. E.g. `https://github.com/python/cpython/blob/master/Lib/distutils/sysconfig.py#L87-L124)

(Python's packaging and distribution support is a big mess !)

The other two suggestions are fine. I'm a fan of f-strings, too, and as I just recently decided to no longer support Python 2.7, will happily clean up the code incrementally.

mattip commented 3 years ago

E.g. `https://github.com/python/cpython/blob/master/Lib/distutils/sysconfig.py#L87-L124)

sysconfig.get_config_var('INCLUDEPY') ?

stefanseefeld commented 3 years ago

Merged manually.

stefanseefeld commented 3 years ago

I just noticed that the sysconfig.get_config_var("EXT_SUFFIX") trick works on Python3, but not Python2 (where it returns None. @mattip any idea what I may have to substitute it with there ?

mattip commented 3 years ago

On CPython2 you can do

import imp  # python2 only, will emit deprecation warning on python3
[x[0] for x in imp.get_suffixes() if x[2] == imp.C_EXTENSION]

which returns

['.x86_64-linux-gnu.so', '.so', 'module.so']

for me, so take ret[0]. I think PyPy2 does export sysconfig.get_config_var("EXT_SUFFIX"), so you might want to do some if None kind of check.