feat!: drop support for Python 3.8, MSVC 2017 - #6110
Conversation
|
🤖 AI text below 🤖 Why AppVeyor failedThe job compiled, then failed to link every module: The AppVeyor image has pybind11 undefines This is new because the Visual Studio 2019 image has CMake 4.1, which has no The fixOne flag, so Debug uses the release artifacts of imported targets: AppVeyor is green: Debug links Follow-up, not in this PRThe same mismatch can hit users. An MSVC Debug build against a Python installation that has the optional debug binaries links |
PR 6110 initial findings (codex gpt-5.6-sol)Reviewed at commit 6e22547 Overall assessmentThe implementation is fundamentally sound. I found no supported-version C++ I recommend addressing the first three findings before merging. Findings1.
|
|
I manually updated the description for items 3 and 4. 🤖 AI text below 🤖 Thanks for the review. Items 1, 2, and 5 are addressed in c10f26a. 1.
|
rwgk
left a comment
There was a problem hiding this comment.
PR 6110 second review
Reviewed at:
Overall assessment
The implementation looks sound. I found no supported-version C++ correctness, API, ABI, CMake, or packaging defect in the net changes.
The follow-up commit c10f26a resolves the earlier source-level review findings. Two factual issues remain in the PR description; I recommend correcting both before merging.
Remaining findings
1. The stated reason for requiring MSVC 2019 is incorrect
The suggested changelog entry currently says:
Support for MSVC 2017 has been dropped (CPython 3.9.2+ requires 2019+).
CPython 3.9.2 did not require Visual Studio 2019. Its own Windows build documentation identifies Visual Studio 2017 as officially supported and says that Visual Studio 2015 may also be used for command-line builds:
https://github.com/python/cpython/blob/v3.9.2/PCbuild/readme.txt
Dropping MSVC 2017 can still be a reasonable pybind11 policy and maintenance decision, but it is separate from raising the Python minimum to 3.9.
Suggested changelog wording:
Support for MSVC 2017 was removed. The minimum supported version is now
MSVC 2019.
For the same reason, I suggest changing the opening sentence from "Now require 3.9+ and matching MSVC 2019+" to "Now require Python 3.9+ and MSVC 2019+."
2. The legacy embedding branch was not unconditionally unreachable
The description still says:
Its legacy branch cannot run when the minimum is 3.9, so the macro and the branch are gone.
That is true with the macro's default value, but PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX was deliberately made externally overrideable. A compiler definition could force the legacy path on a supported Python version as an escape hatch for an unforeseen PyConfig problem. The introducing commit explicitly documented that purpose:
I agree with removing this undocumented escape hatch rather than retaining the deprecated CPython initialization path. The PR description should simply record the decision accurately.
Suggested wording:
The default threshold makes the legacy branch unreachable for supported Python versions. This also removes the undocumented override that could force the legacy path on newer Python versions.
Recommendation
Apply the one suggestion (separate review comment), correct the two factual statements in the PR description, sync with current master, and rerun CI. With those housekeeping items addressed, I consider the implementation ready to merge.
The minimum supported version is now Python 3.9. pybind11 v3.0 was the last release that supports Python 3.8. The deprecation note said that support goes away in 3.1. Remove the code paths that this makes dead: - the `_PyObject_Vectorcall` fallback in `cast.h` - the `frame->f_code` and `frame->f_back` fallbacks in `pytypes.h` - the `PyFrame_FastToLocals` path in `get_type_override` - the conditional `Py_VISIT(Py_TYPE(self))` in `tp_traverse` - `PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX`, the pre-PyConfig interpreter init, and the `widen_chars` helpers in `embed.h` Assisted-by: ClaudeCode:claude-opus-5
The AppVeyor job set `PYTHON: 38`, which makes the path `C:\Python38`. The image gives Python 3.9.13 as `C:\Python39`. Assisted-by: ClaudeCode:claude-opus-5
Python 3.9 is the new minimum, so MSVC 2017 is no longer needed. Raise the compile-time floor to _MSC_VER 1920 and remove the workarounds that only applied below it: std::launder, fold expressions, weak_from_this, aligned new/delete, the C4100 warning helper, and the func_handle syntax error. AppVeyor now builds with Visual Studio 2019. Assisted-by: ClaudeCode:claude-opus-5
CMake 4 has no FindPythonLibs, so pybind11 uses FindPython. FindPython reads the registry before PATH and selected `C:\Python314-x64`, but the test packages go into the Python on PATH. Pass `Python_ROOT_DIR` to name the correct one. Also set `CMAKE_ARCH`. It was never set, so the architecture came from the generator. Visual Studio 2017 defaults to Win32, but Visual Studio 2019 defaults to x64, which made this x86 job build 64-bit code. Assisted-by: ClaudeCode:claude-opus-5
The build compiled but failed to link with LNK1104 on a bare `python39.lib`. That name comes from the `#pragma comment(lib, ...)` in pyconfig.h, so the linker needs the directory. CMake 4 has no FindPythonLibs, and FindPython does not add it for this Debug x86 build. Put it on LIB instead. The directory listing is temporary, to confirm the library is present. Assisted-by: ClaudeCode:claude-opus-5
The image ships python39_d.lib next to python39.lib, so FindPython picks the debug import library for a Debug build. pybind11 undefines _DEBUG around Python.h, so pyconfig.h asks for python39.lib instead and the link failed with LNK1104. Name the release library for the debug slot. Setting LIB does not work, because MSBuild replaces it from the toolset, and it would link both import libraries. Assisted-by: ClaudeCode:claude-opus-5
Revert the two attempted fixes. Neither changed the failure: setting LIB does not survive MSBuild, and naming the release library for Python_LIBRARY_DEBUG had no effect. Print the Python cache entries and the link settings of a generated project file instead, to see what the linker really gets. Temporary. Assisted-by: ClaudeCode:claude-opus-5
The generated project file linked C:\Python39\libs\python39_d.lib in the Debug configuration, because the image ships debug binaries next to the release ones. pybind11 undefines _DEBUG around Python.h, so pyconfig.h asks for python39.lib in a #pragma comment(lib), which nothing on the link line satisfies and no library directory holds. Map Debug to the release artifacts. Assisted-by: ClaudeCode:claude-opus-5
Assisted-by: ClaudeCode:claude-opus-5
The distro pytest in the Clang and GCC Docker jobs has no PytestUnraisableExceptionWarning, so an unconditional filterwarnings mark makes pytest fail with an INTERNALERROR after the tests pass. Assisted-by: ClaudeCode:claude-opus-5 Claude-Session: https://claude.ai/code/session_01Aimf6HuSz1vLRwBnbxmCTc
…fail Extend Python_ADDITIONAL_VERSIONS through 3.15, describe the PyConfig behavior of initialize_interpreter, and drop the stale Python 3.8 wording from the PyPy xfail reason. Assisted-by: ClaudeCode:claude-opus-5
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgkio@gmail.com>
fbb835b to
5bb70c4
Compare
|
I just triggered a rerun of the CIBW / iOS wheel macos-15-intel job (it was most likely just some infrastructure flake). |
Now require 3.9+ and matching MSVC 2019+.
I did check
PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEXbefore making the change in the PR, no obvious examples of this undocumented value being overridden.🤖 AI text below 🤖
Description
Python 3.8 is at end of life. The deprecation note in the docs said that support
goes away in 3.1. This PR removes it. The minimum is now Python 3.9.
Configuration and tooling:
requires-python, the classifiers, and the mypy and pylint versions move to 3.9python39-develC:\Python39in place ofC:\Python38#errorindetail/common.hrejects Python earlier than 3.9setup_helpers.pyuses built-in generics in place oftyping.Listandtyping.TupleDead code paths that this removes:
_PyObject_Vectorcallfallback incast.hframe->f_codeandframe->f_backfallbacks inpytypes.hPyFrame_FastToLocalspath inget_type_overridePy_VISIT(Py_TYPE(self))intp_traversePYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX, the pre-PyConfig interpreter init, andthe
widen_charshelpers inembed.hThe default threshold makes the legacy branch unreachable for supported Python versions. This also removes the undocumented override that could force the legacy path on newer Python versions.
The
embed.hmacro had a default of0x03080000. Its legacy branch cannot run whenthe minimum is 3.9, so the macro and the branch are gone. Four interpreter test cases
were behind that macro. They now always run.
I also renamed the
Add program dir to path pre-PyConfigtest case toAdd program dir to path without PyConfig, because no pre-PyConfig path is left.Suggested changelog entry:
📚 Documentation preview 📚: https://pybind11--6110.org.readthedocs.build/