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

Scratch PR for testing pybind11 v2.10.0 with Python 3.11rc2 #4271

Closed
wants to merge 2 commits into from

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Oct 21, 2022

Scratch PR for testing pybind11 v2.10.0 + cherry picks with Python 3.11rc2, to answer the question:

What is the minimum we'd need to do for 3.11?

Related discussion: #4218 (comment)

…ybind#4119)

* Add debug fprintf to test_interpreter.cpp

* Update `sys.path` from `PYTHONPATH` in Python >= 3.11 branch of `initialize_interpreter()`

* Use `config.isolated = 0; config.use_environment = 1;`

As suggsted by @vstinner here: pybind#4119 (comment)

* Add `TEST_CASE("PYTHONPATH is used to update sys.path")`

* Fix clang-tidy error.

* Use `_putenv_s()` under Windows.

* Fix clang-tidy error: argument name ... in comment does not match parameter name

* Remove slash from PYTHONPATH addition, to work around Windows slash-vs-backslash issue.

* Use `py::str(...)` instead of `.attr("__str__")` as suggested by @Skylion007

Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com>

Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com>
@rwgk rwgk added the python dev Working on development versions of Python label Oct 21, 2022
@rwgk
Copy link
Collaborator Author

rwgk commented Oct 21, 2022

Ugh, the CI isn't doing what I wanted, it's still using all of current master.

@rwgk rwgk changed the base branch from master to stable October 21, 2022 20:17
@rwgk rwgk closed this Oct 21, 2022
@rwgk rwgk reopened this Oct 21, 2022
@rwgk
Copy link
Collaborator Author

rwgk commented Oct 21, 2022

v2.10.0 + PR #4119 passes the Upstream testing (using Python 3.11rc2).

PR #4119 is my own PR, I created it because I had an issue with my scons-based build & test environment that nobody else is using AFAIK. How many people in other situations are affected by the "embedding & PYTHONPATH" issue?

I'll try again with pure v2.10.0.

@henryiii
Copy link
Collaborator

There is already an v2.10 branch, you should start with that. There are several other important bugfixes that we have been promising users. I'd don't see what the point of this PR is.


Version 2.10.1 (Oct 2?, 2022)

Changes:

  • Allow pybind11::capsule constructor to take null destructor
    pointers. #4221
  • embed.h was changed so that PYTHONPATH is used also with Python
    3.11 (established behavior).
    #4119

Bug fixes:

  • Fix MSVC 2019 v.1924 & C++14 mode error for overload_cast.
    #4188
  • Make augmented assignment operators non-const for the object-api.
    Behavior was previously broken for augmented assignment operators.
    #4065
  • Add proper error checking to C++ bindings for Python list append and
    insert. #4208
  • Work-around for Nvidia's CUDA nvcc compiler in versions 11.4.0 -
    11.8.0. #4220
  • A workaround for PyPy was added in the py::error_already_set
    implementation, related to PR
    #1895 released with
    v2.10.0. #4079
  • Fixed compiler errors when C++23 std::forward_like is available.
    #4136
  • Properly raise exceptions in contains methods (like when an object in
    unhashable). #4209
  • Further improve another error in exception handling.
    #4232
  • get_local_internals() was made compatible with
    finalize_interpreter(), fixing potential freezes during interpreter
    finalization. #4192

Performance and style:

  • Reserve space in set and STL map casters if possible. This will
    prevent unnecessary rehashing / resizing by knowing the number of keys
    ahead of time for Python to C++ casting. This improvement will greatly
    speed up the casting of large unordered maps and sets.
    #4194
  • GIL RAII scopes are non-copyable to avoid potential bugs.
    #4183
  • Explicitly default all relevant ctors for pytypes in the
    PYBIND11_OBJECT macros and enforce the clang-tidy checks
    modernize-use-equals-default in macros as well.
    #4017
  • Optimize iterator advancement in C++ bindings.
    #4237
  • Use the modern PyObject_GenericGetDict and PyObject_GenericSetDict
    for handling dynamic attribute dictionaries.
    #4106
  • Document that users should use PYBIND11_NAMESPACE instead of using
    pybind11 when opening namespaces. Using namespace declarations and
    namespace qualification remain the same as pybind11. This is done to
    ensure consistent symbol visibility.
    #4098
  • Mark detail::forward_like as constexpr.
    #4147
  • Optimize unpacking_collector when processing arg_v arguments.
    #4219

Build system improvements:

  • Include a pkg-config file when installing pybind11, such as in the
    Python package. #4077
  • Avoid stripping debug symbols when CMAKE_BUILD_TYPE is set to
    DEBUG instead of Debug.
    #4078
  • Followup to #3948,
    fixing vcpkg again.
    #4123

@rwgk
Copy link
Collaborator Author

rwgk commented Oct 21, 2022

There is already an v2.10 branch, you should start with that. There are several other important bugfixes that we have been promising users. I'd don't see what the point of this PR is.

I was just looking through the changelog while waiting for the current CI run to finish. The Upstream job just finished, passing with pure v2.10.0.

The only change obviously specific to Python 3.11 is #4119.

All other changes appear to be general bug fixes or small enhancements.

Is #4119 enough justification to rush out 2.10.1 by Monday? Am I missing something?

I still believe the 2.10.1 release only distract us and many others from making a clean move, resolving the doubts about ABI compatibility, and removing the gil_scoped_acquire trap.

I don't want to stop you from making the release, but based on what I know at the moment, I'd definitely want to remove my name from README.rst on the v2.10 branch. Also, there needs to be a very prominent warning pointing out the ABI doubts, and the gil_scoped_acquire situation.

@henryiii
Copy link
Collaborator

gil_scoped_acquire hasn't changed. I've known it doesn't support nesting for several years. It is not buggy; it simply does not support nested access. There's no pressure on us to change it before a patch release. Supporting nested access is a feature. If someone expects something to support something, and it doesn't, that's not a bug. It might be unfortunate, could be seen as a trap, but it's not a bug.

We don't have any proof about the ABI compatibility. We have a couple of hard-to-reproduce reports from users. Many pybind11 users don't use the ABI compatibly feature at all. And 3.10.1 isn't any worse than 3.10.0 (that's the point of patch release, only small improvements).

@henryiii
Copy link
Collaborator

There is already an v2.10 branch

I've just realized I've been pushing the 2.10 branch to my fork. So it's not quite as "official" if there's something you don't want in it. But to me all those things look like good fixes to have - I'm particularly fond of several of the packaging related ones, since I've been telling vcpkg and a few others that there would be a fix for them soon.

@virtuald
Copy link
Contributor

FWIW, I've been using smart_holder 6df8693 for about a month or so on 3.11rcX, haven't seen anything unusual.

:shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python dev Working on development versions of Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants