tttapa / py-build-cmake

Modern, PEP 517 compliant build backend for creating Python packages with extensions built using CMake.
https://pypi.org/project/py-build-cmake
MIT License
38 stars 6 forks source link

Minimum Python version #5

Closed cimes-isi closed 1 year ago

cimes-isi commented 2 years ago

Hello again,

My software runs on an older system that has Python 3.7, but py-build-cmake declares a minimum version of 3.8, so installation fails. Is Python>=3.8 a hard constraint, or is it straightforward to support older versions?

Thanks.

tttapa commented 2 years ago

The code is written for >=3.8. I just installed Python 3.7 and tried to run it, and it failed on the walrus operator, which was introduced in 3.8. There are probably other incompatibilities as well. I'll give it a try and see how far I get by just splitting all walrus operators into separate statements.

tttapa commented 2 years ago

I created a py3.7 branch with a version that should work with older versions of Python.

Python 3.7 had different ABIs, and it looks like distlib.wheel doesn't detect the correct ABI on Windows:

https://github.com/tttapa/py-build-cmake/runs/6498522175?check_suite_focus=true

The ABI in the Wheel tag should be cp37m, not cp37. I'm not sure what's going on there, and I don't have much time to look into it right now. But they do have code to do some ABI detection: https://github.com/pypa/distlib/blob/052787b7abcecc5498c1443f4f0ee2867d6a3db0/distlib/wheel.py#L59

It should be noted that the last bugfix release of Python 3.7 was almost two years ago, and in about a year, it will no longer receive security fixes either. Other projects like NumPy, SciPy, etc. already dropped support for Python 3.7 last year.

cimes-isi commented 2 years ago

I really appreciate you making the effort to look at this! I realize 3.7 is a little older and supporting old software can be annoying, but unfortunately, 3.7 is still out there (on Debian buster in my use case), and in fact so is 3.6 (e.g., on Ubuntu 18.04 LTS systems). Those other projects you mention may have dropped support, but they still have older releases available that support earlier Python versions.

I've been testing your py3.7 branch this morning. I ran into a different issue (see #6), but after forcing a newer cmake version than the system has (3.13.4), I got past it. The branch appears to work correctly on Linux with Python 3.7.3 and macOS with Python 3.8.9. I don't have a Windows environment to try.

cimes-isi commented 2 years ago

I've been trying to look into this a bit, though my knowledge of Python packaging is limited. I see a related issue at https://github.com/pypa/packaging/issues/181 which references _cpython_abi() in pypa/packaging, similar to what you linked to above. Compare this with the more recent implementation. Perhaps this is a bug in distlib?

The GH Action build is using a slightly outdated pip, so we might consider upgrading it first, though I don't have reason to believe it will resolve the issue.

Cheers.

tttapa commented 2 years ago

Thanks for looking into this!

I've opened an issue with distlib: https://github.com/pypa/distlib/issues/172

I've now used packaging to determine the ABI, which seems to work as expected: 68e751c6a9b3afaf3090e1df7fd550c63905c883

I'm not entirely sure about the robustness, perhaps I shouldn't rely on the order of the tags returned by packaging.tags.sys_tags(), the documentation just says

The order of the sequence corresponds to priority order for the interpreter, from most to least important.

The other relevant functions like _cpython_abis are private.

The GH Action build is using a slightly outdated pip, so we might consider upgrading it first

Pip already gets updated before actually running the tests: https://github.com/tttapa/py-build-cmake/blob/cc72fc76e7186f52c8319b4dba14f73aad1557ec/noxfile.py#L7

The warning originates from the installation of Nox in the global environment, the actual Nox test environment uses the latest version of pip.

cimes-isi commented 2 years ago

I've opened an issue with distlib: pypa/distlib#172

I'm glad we could identify the source - thank you for reporting it upstream. I've subscribed to the issue, hopefully we'll get some feedback soon.

I've now used packaging to determine the ABI, which seems to work as expected: 68e751c

I'm not entirely sure about the robustness, perhaps I shouldn't rely on the order of the tags returned by packaging.tags.sys_tags(), the documentation just says

The order of the sequence corresponds to priority order for the interpreter, from most to least important.

That's promising. How can we resolve the robustness concern (and, if needed, decide what a better approach should be)? It looks like there's an IRC channel: #pypa on Libera.Chat, where somebody might be able to answer authoritatively.

cimes-isi commented 1 year ago

I see that distlib 0.3.5 is now released which is supposed to fix the ABI issue in Windows. If we update the distlib version dependency, would that then complete Python 3.7 support? Thanks!

tttapa commented 1 year ago

Release 0.0.11.post1 adds support for Python 3.7 (and 3.6 seems to work as well).

cimes-isi commented 1 year ago

Thanks! I see you made this a post release, which appears to be made out of a branch. Will you merge this support into main before your next semantically-versioned release?

tttapa commented 1 year ago

Indeed. The post release doesn't change the behavior of the package, it only makes it compatible with Python 3.6 and 3.7.

I mostly just wanted to have a PyPI version of the package with 3.6 support because RHEL 8 is still on Python 3.6. Unless I make drastic changes or serious bug fixes to py-build-cmake, I don't think it's worth keeping the py3.6 branch up-to-date, though.

For 3.7, I'll definitely keep it updated until its EOL next year, and probably longer for Buster. The differences between main and py3.7 are minor, so that shouldn't be an issue, I might as well merge them when I have a bit more time on my hands.

cimes-isi commented 1 year ago

Great, thank you! I think it makes sense to merge support for both Python 3.6 and (definitely) 3.7 into main and be done with the branches. At least for the foreseeable future, it doesn't seem like py-build-cmake itself would need to depend on language features in newer Python versions. I expect at some point you may have to update py-build-cmake's dependencies to minimum package versions that drop support older Python version(s), at which point you might be forced to also drop support. But as long as it's easy to support older Python versions, you might as well do so!

tttapa commented 1 year ago

Python 3.7 support has been merged into main (#9).