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

drop python<=3.7 support #769

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

drop python<=3.7 support #769

wants to merge 4 commits into from

Conversation

kloczek
Copy link

@kloczek kloczek commented Jun 14, 2024

According to https://endoflife.date/python python 3.7 has been EOSed 27 Jun 2023.
Filter all code over pyupgrade --py38-plus.

@kloczek
Copy link
Author

kloczek commented Jun 14, 2024

Cannot find were is defined python 3.7 CI to remove it 🤔

@LecrisUT
Copy link
Collaborator

LecrisUT commented Jun 14, 2024

python-version: ["3.7", "pypy-3.9", "3.12"]

You still have a few sys.version_info < (3, 8) stragglers.

Also change the pyproject.toml itself

@LecrisUT
Copy link
Collaborator

Not sure what policy is for this. Last year when python 3.7 was EOL we still kept compatibility since academic software are notoriously slow to adopt. Should we adopt dropping 3.(n-1) when 3.n reached EOL (a bit later this year)

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 78.57143% with 6 lines in your changes missing coverage. Please review.

Project coverage is 81.91%. Comparing base (3a16f45) to head (2794b44).

Files Patch % Lines
src/scikit_build_core/_logging.py 42.85% 4 Missing ⚠️
src/scikit_build_core/builder/get_requires.py 50.00% 1 Missing ⚠️
src/scikit_build_core/hatch/plugin.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #769      +/-   ##
==========================================
- Coverage   81.99%   81.91%   -0.09%     
==========================================
  Files          68       68              
  Lines        3933     3903      -30     
==========================================
- Hits         3225     3197      -28     
+ Misses        708      706       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@henryiii
Copy link
Collaborator

I'd probably like to wait till 1.0 (which is probably ~3 months away); dropping a Python version means we have LTS support on the last 3.7 one. Maybe sooner, if pybind11 drops 3.7 (currently will be dropping 3.6 first!). Hatchling plugins require 3.8+, we can fully use uv if we use 3.8+, and I'll be looking over the changes here and probably drooling a little...

One issue is that bumping here causes cmake and ninja, which don't require any version of Python really, to now either use an old version on 3.7 or also have to drop 3.7. Once we hit 1.0, that's a bit less of an issue, so that's one reason I'd like to wait a bit longer. But we'll see, I'll bring it up at our next community or developer meeting (I may miss the one next week, so that's next month).

opts: Any = {}
else:
opts: Any = {"stacklevel": 2}
opts: Any = {"stacklevel": 2}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could actually just inline this now.

@LecrisUT
Copy link
Collaborator

I'd probably like to wait till 1.0 (which is probably ~3 months away)

🤩

One issue is that bumping here causes cmake and ninja

Oh, ninja got ported as well? Awesome 🎉

which don't require any version of Python really, to now either use an old version on 3.7 or also have to drop 3.7.

Shouldn't it still be possible to use the last available scikit-build-core version that supported 3.7? I guess that's ehat you want 1.0 for. This should only affect PyPI distributions since cmake and ninja would be packaged by the system, spack, conda, etc.

In Fedora we finally added dummy {cmake,ninja}.dist-info packages to better satisfy pip operations. Other distros should follow suit. Would like to also add the other python files in there, but it is tricky to plan.

@kloczek
Copy link
Author

kloczek commented Jun 15, 2024

I'd probably like to wait till 1.0 (which is probably ~3 months away);

Around that time will be EOSed 3.8.

dropping a Python version means we have LTS support on the last 3.7 one.

LTS means freezing API/ABI.
If any critical issue will be found with last supporting 3.7 python version always on top of the version tag is possible to add branch and add fix issue -> release new minor version.
Using this procedure it is possible to support any LTS distro and have on master up-to-date version.
Many modules currently on master support only 3.9, 3.10 or higher and still can provide support for older versions.

@LecrisUT
Copy link
Collaborator

If any critical issue will be found with last supporting 3.7 python version always on top of the version tag is possible to add branch and add fix issue -> release new minor version.

Probably that would require releasing 1.0 with 3.7 support and immediately after that 1.1 dropping 3.7 in order to have 1.1.x to distribute patches.

Using this procedure it is possible to support any LTS distro and have on master up-to-date version.

Generally LTS distros would be responsible to backport CVE and bug fixes. If upstream is able to help (at least with merging patches and releasing bug fixes) it is a great help.

Testing for both branches upstream would be hard, but maybe we can add some runners based on EPEL if dependencies allow

@kloczek
Copy link
Author

kloczek commented Jun 15, 2024

OK I've added more manual cleanups to remove python<=3.7 support.
I have impression that still much more can be added but what I've committed is absolute minimum.

It would be good to check what changes pyupgrade --py39-plus generates and so on to check is the code ready to be fully filterable by pypgrade for next versions, and if not adjust some if statements which are using sys.version_info to have smooth drop legacy code.

@kloczek
Copy link
Author

kloczek commented Jun 15, 2024

Generally LTS distros would be responsible to backport CVE and bug fixes. If upstream is able to help (at least with merging patches and releasing bug fixes) it is a great help.

That is true. On top of that as critical issue could be treated some functional bug (something is not working as expected).
To be honest currently catching such bugs automatically is a bit hard because it can be done only on forming distribution layer. Simple none of distros are not obligatory executing test suites during packaging process. For example in Fedora only ~1/3 of all python modules are executing included test suites.

[tkloczko@pers-jacek SPECS.fedora]$ ls -1 python-*.spec | wc -l; grep -l ^%pytest python-*.spec | wc -l
3309
1149

In distro which I'm using

[tkloczko@pers-jacek SPECS]$ ls -1 python-*.spec | wc -l; grep -l ^%pytest python-*.spec | wc -l
1259
1241

and in the past I was able to catch really huge number of such functional issues not only in packaged modules but in other modules involved in testing procedure of just packaged module.

@LecrisUT
Copy link
Collaborator

For example in Fedora only ~1/3 of all python modules are executing included test suites.

It's a bit complicated because: tests mightbe run from tox, unit-test (files directly), outside with tmt. I do agree that any package that does not include tests should be reported, and if you have a way to show them with something like sourcegraph.com, it would be greatly appreciated. For RHEL and CentOS Stream though, the package list is significantly reduced so it would be better to check against Centos Stream on the LTS testability.

and in the past I was able to catch really huge number of such functional issues not only in packaged modules but in other modules involved in testing procedure of just packaged module.

Indeed and that's why packagers can be so useful for the overall ecosystem. We do need to be flexible with upstream's workflow though. Luckily scikit-build-core is quite accommodating on this.

PS: can you mark the PR as draft to avoid accidental merges. I am a bit paranoid of my fat fingers.

@kloczek
Copy link
Author

kloczek commented Jun 15, 2024

It's a bit complicated because: tests mightbe run from tox, unit-test (files directly), outside with tmt

Trust me .. it is not 😋
unittest based test suite as long as it is correctly written can be 100% handled by pytest.
On use tox someone assumes that module needs to be installed.
Typically on packaging .whl is installed in some </install/preffix> form which are collected all files to form binary package.
Testing stage with rpm and other PM tools are executed after that installation.
All what you need is provide $PYTHONPATH with paths pointing to that </install/preffix>

[tkloczko@pers-jacek SPECS]$ rpm -E %pytest
\

ASMFLAGS="-m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -mtls-dialect=gnu2 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdata-sections -ffunction-sections -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -flto=auto -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Werror=format-security";
CFLAGS="-m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -mtls-dialect=gnu2 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdata-sections -ffunction-sections -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -flto=auto -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Werror=format-security";
CXXFLAGS="-m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -mtls-dialect=gnu2 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdata-sections -ffunction-sections -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -flto=auto -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Werror=format-security";
FFLAGS="-m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -mtls-dialect=gnu2 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdata-sections -ffunction-sections -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -flto=auto -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Werror=format-security -I/usr/lib64/gfortran/modules";
FCFLAGS="-m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -O2 -g -grecord-gcc-switches -pipe -mtls-dialect=gnu2 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fdata-sections -ffunction-sections -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -flto=auto -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -Wall -Werror=format-security -I/usr/lib64/gfortran/modules";
LDFLAGS="-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -Wl,--gc-sections -Wl,--as-needed -Wl,--build-id=sha1 -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -Wl,-z,pack-relative-relocs -flto=auto -fuse-linker-plugin";
RUSTFLAGS="-C codegen-units=1 -C debuginfo=2 -C opt-level=2 -C link-arg=-Wl,--as-needed -C link-arg=-Wl,--build-id=sha1 -C link-arg=-Wl,-z,now -C link-arg=-specs=/usr/lib/rpm/redhat/redhat-hardened-ld -C link-arg=-Wl,-z,pack-relative-relocs -C link-arg=-Wl,-z,relro -C link-arg=-flto=auto --cap-lints=warn" ;
VALAFLAGS="-g" ;
CC="/usr/bin/gcc"; CXX="/usr/bin/g++"; FC="/usr/bin/gfortran";
AR="/usr/bin/gcc-ar"; NM="/usr/bin/gcc-nm"; RANLIB="/usr/bin/gcc-ranlib";
export ASMFLAGS CFLAGS CXXFLAGS FFLAGS FCFLAGS LDFLAGS VALAFLAGS CC CXX FC AR NM RANLIB RUSTFLAGS VALAFLAGS;
 \
        PATH=/home/tkloczko/rpmbuild/BUILDROOT/%{NAME}-%{VERSION}-%{RELEASE}.x86_64/usr/bin:$PATH \
        LD_LIBRARY_PATH=/home/tkloczko/rpmbuild/BUILDROOT/%{NAME}-%{VERSION}-%{RELEASE}.x86_64/usr/lib64 \
        PYTHONDONTWRITEBYTECODE=1 \
        PDM_BUILD_SCM_VERSION=%{version} \
        PBR_VERSION=%{version} \
        SETUPTOOLS_SCM_PRETEND_VERSION=%{version} \
        PYTHONPATH=${PYTHONPATH:-/home/tkloczko/rpmbuild/BUILDROOT/%{NAME}-%{VERSION}-%{RELEASE}.x86_64/usr/lib64/python3.10/site-packages:/home/tkloczko/rpmbuild/BUILDROOT/%{NAME}-%{VERSION}-%{RELEASE}.x86_64/usr/lib/python3.10/site-packages} \
         \
        /usr/bin/pytest -ra -m "not network"

As you see in this case such injection of the $PYTHONPATH into pytest script env variables is provided 😋

@kloczek kloczek marked this pull request as draft June 15, 2024 10:43
@kloczek
Copy link
Author

kloczek commented Jun 15, 2024

PS: can you mark the PR as draft to avoid accidental merges. I am a bit paranoid of my fat fingers.

Done 👍

@LecrisUT
Copy link
Collaborator

It's a bit complicated because: tests mightbe run from tox, unit-test (files directly), outside with tmt

Trust me .. it is not 😋

I was referring to the grep -l ^%pytest python-*.spec. That does not pick up on other methods of running %check. There are ~700 packages that don't have %check, and although you could still have tests with tmt, but I didn't check if any of them do.

unittest based test suite as long as it is correctly written can be 100% handled by pytest.

I've had hit-and-miss experience there. Most recent was with packaging yq

@kloczek
Copy link
Author

kloczek commented Jun 15, 2024

[tkloczko@pers-jacek SPECS.fedora]$ grep %check python-*.spec | wc -l
2771

As you see ~20% of all python modules in Fedora has no at all any test suite execution
Tox is used:

[tkloczko@pers-jacek SPECS.fedora]$ grep %tox python-*.spec | wc -l
247

In all those cases it can be used pytest.

Testing that module is importable only

[tkloczko@pers-jacek SPECS.fedora]$ grep %pyproject_check_import python-*.spec | wc -l
490

For everythingeelse (1/3) are used even setup.py test (which is already deprecated

[tkloczko@pers-jacek SPECS.fedora]$ grep "setup.py test" python-*.spec | wc -l
261

zope test runner and nox

[tkloczko@pers-jacek SPECS.fedora]$ grep "zope-testrunner " python-*.spec | wc -l
8
[tkloczko@pers-jacek SPECS.fedora]$ grep "nox " python-*.spec | wc -l
13

nox it is just another variation of the tox (can be replaced by pytest). Zope test runner as well can be handled by pytest (sometimes it is only necessary to rename test suite files to tests_*.py)
Everything else in Fedora uses custom testing methodologies which more and more frequently does not work at all and needs to be disabled (limited number of people using those custom frameworks). In many cases even those old files can be used with pytest.

In other words pytest is now de facto standard and only other testing framework which is under development is standard unittest (zope test runner from quite long time only received unittests cleanups and some critical fixes).
Most of the modules which have no at least partial or proper pytest support are usually no longer maintained and sooner or later will be replaced by other modules.

According to https://endoflife.date/python python 3.7 has been EOSed
27 Jun 2023.
Filter all code over `pyupgrade --py38-plus`.

Signed-off-by: Tomasz Kłoczko <[email protected]>
Signed-off-by: Tomasz Kłoczko <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
henryiii added a commit that referenced this pull request Aug 6, 2024
This adds an `if.scikit-build-version`, and makes sure it can be used to
gate features not yet implemented. This should help with #769 by
providing a way to support old scikit-build-core's on Python 3.7 even
after we drop support.

Also fixes auto minimum-version to respect markers.

---------

Signed-off-by: Henry Schreiner <[email protected]>
@henryiii
Copy link
Collaborator

FWIW, 3.7 is the third most popular Python version downloading scikit-build-core. 3.11 dominates by a ton, though - factor of 10. Something must be up with that, you don't see the same domination on pip or other packages. I expect some large user that often builds from source is using only 3.11.

Screenshot 2024-09-19 at 12 55 05 AM

@kloczek
Copy link
Author

kloczek commented Sep 19, 2024

Looks like in pyproject.toml still is listed 3.7.

requires-python = ">=3.7"

So that line should be updated to requires-python = ">=3.8".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants