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

DEV: git merge smart_holder after PR #5257 #30141

Closed
wants to merge 18 commits into from

Conversation

rwgk
Copy link
Contributor

@rwgk rwgk commented Aug 3, 2024

Description

TL;DR: This PR rebuilds the pybind11clif main branch on top of the latest smart_holder branch.

The final state of this helper PR (closed) is identical to #30143 (merged).

This very complex merge is best understood by reviewing the individual commits here.

The complexity arises from the very large changes under pybind/pybind11#5257: Bake smart_holder functionality into class_ and type_caster_base

Mixed in are several small changes merged from the pybind11 master branch, that accumulated in the meantime.

Suggested changelog entry:

Ralf W. Grosse-Kunstleve and others added 11 commits July 30, 2024 09:18
* First pass updating misc files, informed by https://github.com/pybind/pybind11/pull/5177/commits

* Remove jobs using silkeh/clang and gcc docker containers that come with Python 3.7

* Add silkeh/clang:17-bookworm

* Add job using GCC 7

* Revert "Add job using GCC 7"

This reverts commit 518515a761ac37dc2cf5d0980da82d0de39edc28.

* Try running in ubuntu-18.04 container under ubuntu-latest (to get GCC 7)

* Fix `-` vs `:` mixup.

* This reverts commit b1c4304475b8ad129c12330c7ed7eb85d15ba14a.

Revert "Try running in ubuntu:18.04 container under ubuntu-latest (to get GCC 7)"

This reverts commit b203a294bb444fc6ae57a0100fa91dc91b8d3264.

* `git grep 0x03080000` cleanup.

* `git grep -I -E '3\.7'` cleanup.

Removes two changes made under pybind/pybind11#3702

* Revert "`git grep -I -E '3\.7'` cleanup."

This reverts commit bb5b9d187bffbfb61e2977d7eee46b766fa1cce9.

* Remove comments that are evidently incorrect:

```
...
-- The CXX compiler identification is Clang 15.0.7
...
- Found Python: /usr/bin/python3.9 (found suitable version "3.9.2", minimum required is "3.7") found components: Interpreter Development.Module Development.Embed
...
/__w/pybind11/pybind11/include/pybind11/gil.h:150:13: error: 'auto key' can be declared as 'auto *key' [readability-qualified-auto,-warnings-as-errors]
            auto key = internals.tstate;
            ^~~~~
            auto *
/__w/pybind11/pybind11/include/pybind11/gil.h:174:13: error: 'auto key' can be declared as 'auto *key' [readability-qualified-auto,-warnings-as-errors]
            auto key = detail::get_internals().tstate;
            ^~~~~
            auto *
```

* .github/workflows/configure.yml: Change from Python 3.7 to 3.8

* Misc cleanup pass

* Miscellaneous changes based on manual review of the `git grep` matches below:

```
git_grep_37_38.sh |& sort | uniq -c
```

With git_grep_37_38.sh:

```
set -x
git grep 0x0307
git grep 0x0308
git grep PY_MINOR_VERSION
git grep PYPY_VERSION
git grep -I -E '3\.7'
git grep -I -E '3\.8'
git grep -I -E '\(3, 7'
git grep -I -E '\(3, 8'
git grep -I -E '3[^A-Za-z0-9.]+7'
git grep -I -E '3[^A-Za-z0-9.]+8'
```

Output:

```
      1 .appveyor.yml:    $env:CMAKE_INCLUDE_PATH = "eigen-3.3.7;$env:CMAKE_INCLUDE_PATH"
      1 .appveyor.yml:    7z x eigen-3.3.7.zip -y > $null
      1 .appveyor.yml:    Start-FileDownload 'https://gitlab.com/libeigen/eigen/-/archive/3.3.7/eigen-3.3.7.zip'
      1 CMakeLists.txt:    # Bug in macOS CMake < 3.7 is unable to download catch
      1 CMakeLists.txt:  elseif(WINDOWS AND CMAKE_VERSION VERSION_LESS 3.8)
      1 CMakeLists.txt:  if(OSX AND CMAKE_VERSION VERSION_LESS 3.7)
      1 CMakeLists.txt:    message(WARNING "CMAKE 3.7+ needed on macOS to download catch, and newer HIGHLY recommended")
      1 CMakeLists.txt:    message(WARNING "CMAKE 3.8+ tested on Windows, previous versions untested")
      1 CMakeLists.txt:    # Only tested with 3.8+ in CI.
      1 docs/advanced/functions.rst:Python 3.8 introduced a new positional-only argument syntax, using ``/`` in the
      1 docs/changelog.rst:* Adapt pybind11 to a C API convention change in Python 3.8. `google#1950
      1 docs/changelog.rst:* Allow thread termination to be avoided during shutdown for CPython 3.7+ via
      1 docs/changelog.rst:  considered as conversion, consistent with Python 3.8+.
      1 docs/changelog.rst:  CPython 3.8 and 3.9 debug builds.
      1 docs/changelog.rst:* Enum now has an ``__index__`` method on Python <3.8 too.
      1 docs/changelog.rst:  on Python 3.8. `google#1780 <https://github.com/pybind/pybind11/pull/1780>`_.
      1 docs/changelog.rst:* PyPy 3.10 support was added, PyPy 3.7 support was dropped.
      2 docs/changelog.rst:* Support PyPy 7.3.7 and the PyPy3.8 beta. Test python-3.11 on PRs with the
      1 docs/changelog.rst:* Use ``macos-13`` (Intel) for CI jobs for now (will drop Python 3.7 soon).
      1 docs/changelog.rst:* Use new Python 3.7 Thread Specific Storage (TSS) implementation if available.
      1 docs/compiling.rst:    cmake -DPYBIND11_PYTHON_VERSION=3.8 ..
      1 docs/compiling.rst:    find_package(Python 3.8 COMPONENTS Interpreter Development REQUIRED)
      1 docs/limitations.rst:- PyPy3 7.3.1 and 7.3.2 have issues with several tests on 32-bit Windows.
      1 docs/requirements.txt:idna==3.7 \
      1 + git grep 0x0307
      1 + git grep 0x0308
      1 + git grep -I -E '\(3, 7'
      1 + git grep -I -E '3\.7'
      1 + git grep -I -E '\(3, 8'
      1 + git grep -I -E '3\.8'
      1 + git grep -I -E '3[^A-Za-z0-9.]+7'
      1 + git grep -I -E '3[^A-Za-z0-9.]+8'
      1 + git grep PY_MINOR_VERSION
      1 + git grep PYPY_VERSION
      2 .github/workflows/ci.yml:        - '3.8'
      1 .github/workflows/ci.yml:        - 3.8
      1 .github/workflows/ci.yml:    - name: Add Python 3.8
      1 .github/workflows/ci.yml:        - 'pypy-3.8'
      2 .github/workflows/ci.yml:            python: '3.8'
      1 .github/workflows/ci.yml:          - python: '3.8'
      1 .github/workflows/ci.yml:          - python: 3.8
      1 .github/workflows/ci.yml:            python: 'pypy-3.8'
      1 .github/workflows/configure.yml:          cmake: "3.8"
      1 .github/workflows/configure.yml:    name: 🐍 3.8 • CMake ${{ matrix.cmake }} • ${{ matrix.runs-on }}
      1 .github/workflows/configure.yml:    - name: Setup Python 3.8
      1 .github/workflows/configure.yml:        python-version: 3.8
      1 .github/workflows/pip.yml:    name: 🐍 3.8 • 📦 & 📦 tests • ubuntu-latest
      1 .github/workflows/pip.yml:    name: 🐍 3.8 • 📦 tests • windows-latest
      2 .github/workflows/pip.yml:    - name: Setup 🐍 3.8
      2 .github/workflows/pip.yml:        python-version: 3.8
      2 include/pybind11/cast.h:#if !defined(PYPY_VERSION)
      2 include/pybind11/cast.h:#if defined(PYPY_VERSION)
      2 include/pybind11/cast.h:            // PyPy: 7.3.7's 3.8 does not implement PyLong_*'s __index__ calls.
      5 include/pybind11/detail/class.h:#if !defined(PYPY_VERSION)
      1 include/pybind11/detail/class.h:#if defined(PYPY_VERSION)
      1 include/pybind11/detail/class.h:    // This was not needed before Python 3.8 (Python issue 35810)
      1 include/pybind11/detail/common.h:    && !defined(PYPY_VERSION) && !defined(PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF)
      2 include/pybind11/detail/common.h:#    error "PYTHON < 3.8 IS UNSUPPORTED. pybind11 v2.13 was the last to support Python 3.7."
      1 include/pybind11/detail/common.h:#if defined(PYPY_VERSION) && !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
      1 include/pybind11/detail/common.h:#if PY_VERSION_HEX < 0x03080000
      1 include/pybind11/detail/common.h:            = PYBIND11_TOSTRING(PY_MAJOR_VERSION) "." PYBIND11_TOSTRING(PY_MINOR_VERSION);        \
      1 include/pybind11/detail/internals.h:        // called. PYBIND11_TLS_FREE is PyThread_tss_free on python 3.7+. On older python, it does
      1 include/pybind11/detail/internals.h:#if PYBIND11_INTERNALS_VERSION <= 4 || defined(PYPY_VERSION)
      1 include/pybind11/detail/internals.h:// The old Python Thread Local Storage (TLS) API is deprecated in Python 3.7 in favor of the new
      1 include/pybind11/detail/type_caster_base.h:#if defined(PYPY_VERSION)
      1 include/pybind11/embed.h:#    define PYBIND11_PYCONFIG_SUPPORT_PY_VERSION_HEX (0x03080000)
      1 include/pybind11/embed.h:#if defined(PYPY_VERSION)
      1 include/pybind11/eval.h:    // globals if not yet present.  Python 3.8 made PyRun_String behave
      2 include/pybind11/eval.h:#if defined(PYPY_VERSION)
      2 include/pybind11/eval.h:    // was missing from PyPy3.8 7.3.7.
      2 include/pybind11/gil.h:    /// allowed during shutdown. Check _Py_IsFinalizing() on Python 3.7+, and
      1 include/pybind11/pybind11.h:#if !defined(PYPY_VERSION)
      4 include/pybind11/pybind11.h:#if !defined(PYPY_VERSION) && PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION == 9
      1 include/pybind11/pytypes.h:#endif //! defined(PYPY_VERSION)
      2 include/pybind11/pytypes.h:#if !defined(PYPY_VERSION)
      1 include/pybind11/pytypes.h:#    if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030a00
      1 include/pybind11/pytypes.h:#ifdef PYPY_VERSION
      1 include/pybind11/stl/filesystem.h:#    if !defined(PYPY_VERSION)
      2 pybind11/__init__.py:if sys.version_info < (3, 8):
      2 pybind11/__init__.py:    msg = "pybind11 does not support Python < 3.8. v2.13 was the last release supporting Python 3.7."
      1 pyproject.toml:master.py-version = "3.8"
      1 pyproject.toml:python_version = "3.8"
      1 README.rst:lines of code and depend on Python (3.8+, or PyPy) and the C++
      2 README.rst:- Python 3.8+, and PyPy3 7.3 are supported with an implementation-agnostic
      1 setup.cfg:    Programming Language :: Python :: 3.8
      1 setup.cfg:python_requires = >=3.8
      1 setup.py:# TODO: use literals & overload (typing extensions or Python 3.8)
      1 tests/CMakeLists.txt:if(NOT CMAKE_VERSION VERSION_LESS 3.8)
      2 tests/constructor_stats.h:#if defined(PYPY_VERSION)
      1 tests/env.py:    doesn't work on CPython 3.8.0 with pytest==3.3.2 on Ubuntu 18.04 (google#2922).
      1 tests/requirements.txt:build~=1.0; python_version>="3.8"
      1 tests/requirements.txt:numpy~=1.21.5; platform_python_implementation!="PyPy" and python_version>="3.8" and python_version<"3.10"
      1 tests/requirements.txt:numpy~=1.23.0; python_version=="3.8" and platform_python_implementation=="PyPy"
      1 tests/test_buffers.py:    env.PYPY, reason="PyPy 7.3.7 doesn't clear this anymore", strict=False
      1 tests/test_builtin_casters.py:    # Before Python 3.8, `PyLong_AsLong` does not pick up on `obj.__index__`,
      2 tests/test_builtin_casters.py:    if (3, 8) <= sys.version_info < (3, 10) and env.CPYTHON:
      4 tests/test_builtin_casters.py:    # TODO: PyPy 3.8 does not behave like CPython 3.8 here yet (7.3.7)
      1 tests/test_callbacks.py:    assert m.test_callback3(z.double) == "func(43) = 86"
      2 tests/test_call_policies.cpp:#if !defined(PYPY_VERSION)
      1 tests/test_chrono.py:    diff = m.test_chrono_float_diff(43.789012, 1.123456)
      1 tests/test_constants_and_functions.py:    assert m.f3(86) == 89
      1 tests/test_eigen_matrix.py:    a_copy3[8, 1] = 11
      1 tests/test_eigen_matrix.py:    assert np.all(cornersc == np.array([[1.0, 3], [7, 9]]))
      1 tests/test_eigen_matrix.py:    assert np.all(cornersr == np.array([[1.0, 3], [7, 9]]))
      1 tests/test_eigen_matrix.py:        mymat = chol(np.array([[1.0, 2, 4], [2, 13, 23], [4, 23, 77]]))
      1 tests/test_exceptions.py:    if hasattr(pytest, unraisable):  # Python >= 3.8 and pytest >= 6
      2 tests/test_exceptions.py:@pytest.mark.xfail(env.PYPY, reason="Failure on PyPy 3.8 (7.3.7)", strict=False)
      1 tests/test_factory_constructors.py:    assert [i.alive() for i in cstats] == [13, 7]
      1 tests/test_kwargs_and_defaults.cpp:#ifdef PYPY_VERSION
      1 tests/test_local_bindings.py:    assert i1.get3() == 8
      1 tests/test_methods_and_attributes.cpp:#if !defined(PYPY_VERSION)
      1 tests/test_numpy_array.py:    a = np.arange(3 * 7 * 2) + 1
      1 tests/test_numpy_array.py:    assert str(excinfo.value) == "cannot reshape array of size 42 into shape (3,7,1)"
      2 tests/test_numpy_array.py:    assert x.shape == (3, 7, 2)
      2 tests/test_numpy_array.py:        m.reshape_tuple(a, (3, 7, 1))
      2 tests/test_numpy_array.py:    x = m.reshape_tuple(a, (3, 7, 2))
      1 tests/test_numpy_vectorize.py:    assert np.isclose(m.vectorized_func3(np.array(3 + 7j)), [6 + 14j])
      1 tests/test_pickling.cpp:#if !defined(PYPY_VERSION)
      1 tests/test_pytypes.cpp:#if (defined(__APPLE__) && defined(__clang__)) || defined(PYPY_VERSION)
      1 tests/test_smart_ptr.cpp:    m.def("make_myobject3_1", []() { return new MyObject3(8); });
      1 tests/test_smart_ptr.py:    assert cstats.values() == ["MyObject3[9]", "MyObject3[8]", "MyObject3[9]"]
      1 tests/test_stl_binders.py:    assert v_int2 == m.VectorInt([0, 99, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 88])
      1 tests/test_stl_binders.py:    assert v_int2 == m.VectorInt([0, 99, 2, 3, 4, 5, 6, 7, 0, 1, 2, 3, 88, 4])
      1 tests/test_type_caster_pyobject_ptr.cpp:#if !defined(PYPY_VERSION) // It is not worth the trouble doing something special for PyPy.
      1 tools/FindPythonLibsNew.cmake:  set(PythonLibsNew_FIND_VERSION "3.8")
      1 tools/JoinPaths.cmake:# https://docs.python.org/3.7/library/os.path.html#os.path.join
      1 tools/pybind11NewTools.cmake:    Python 3.8 REQUIRED COMPONENTS ${_pybind11_interp_component} ${_pybind11_dev_component}
      1 tools/pybind11NewTools.cmake:# Python debug libraries expose slightly different objects before 3.8
      1 tools/pybind11Tools.cmake:    "3.12;3.11;3.10;3.9;3.8"
      1 tools/pybind11Tools.cmake:    if(NOT DEFINED PYPY_VERSION)
      1 tools/pybind11Tools.cmake:    message(STATUS "PYPY ${PYPY_VERSION} (Py ${PYTHON_VERSION})")
      1 tools/pybind11Tools.cmake:# Python debug libraries expose slightly different objects before 3.8
      1 tools/pybind11Tools.cmake:      set(PYPY_VERSION
```

* Change `[tool.ruff]` `target-version` to `"py38"`, as suggested by @Skylion007
…e_caster_base` (google#5257)

* Put bakein branch @ 18b72c0ffa6ff2747ed6c4b869a80adfb8e762c9 on top of smart_holder branch:

Commands used:

```
git checkout bakein
git diff smart_holder > ~/zd
git checkout smart_holder
git checkout -b bakein_sh
patch -p 1 < ~/zd
git checkout smart_holder \
MANIFEST.in \
README.rst \
README_smart_holder.rst \
docs/advanced/smart_ptrs.rst \
ubench/holder_comparison.cpp \
ubench/holder_comparison.py \
ubench/holder_comparison_extract_sheet_data.py \
ubench/number_bucket.h \
ubench/python/number_bucket.clif
git add -A
```

* Add back README_smart_holder.rst in tests/extra_python_package/test_files.py

* Restore smart_holder_poc.h as-is on smart_holder branch (i.e. undo `PYBIND11_SMART_HOLDER_PADDING`, which was meant for stress-testing only).

* Insert `std::move()` as suggested by @laramiel

* `property_cpp_function_sh_*` named specializations, as suggested by @laramiel (pybind/pybind11#5257 (comment))

* Call `property_cpp_function_classic` member functions, rather than inlining the implementations.

* Use `PYBIND11_HAVE_INTERNALS_WITH_SMART_HOLDER_SUPPORT` in holder_comparison.cpp (holder_comparison.py is NOT changed accordingly in this commit, i.e. can still only be run if the smart_holder functionality is available).

* Systematically rename `loaded_as` to `load_as` (`shared_ptr`, `unique_ptr`) as suggested by @laramiel

* Make change as suggested by @laramiel. This makes it much more obvious that the latest implementation of `smart_holder_from_unique_ptr()` accepts all existing `return_value_policy` enum values except `copy`.

* Resolve `BAKEIN_WIP: Rewrite comment.` for `property_cpp_function_*` specializations.

* Resolve `BAKEIN_WIP: Add comment to explain: This is meant for stress-testing only.`

* Resolve all remaining BAKEIN_WIP (in pybind11/cast.h).

Leave only two pairs of SMART_HOLDER_BAKEIN_FOLLOW_ON comments: refactoring of copyable_holder_caster, move_only_holder_caster. This is best left until after the smart_holder branch is merged into the master branch.

* Remove obsolete `using holder_type = smart_holder;` in `load_helper`

* Add SMART_HOLDER_BAKEIN_FOLLOW_ON comment for `internals::default_holder`

* README_smart_holder.rst update (line count reduced from 356 to 123).
…art_holder_from_shared_ptr()`, to achieve successful building. (Testing aborts with a segfault in test_class_release_gil_before_calling_cpp_dtor.py)
…m smart_holder_type_casters.h to type_caster_base.h

Resolve all `PYBIND11K_MERGE_SH_AFTER_PR5257_WIP` in test_return_value_policy_override.py (in other words, restore the original complete tests).
…_casters.h to type_caster_base.h

Restore the original, complete test_class_sh_void_ptr_capsule.py
copybara-service bot pushed a commit to pybind/pybind11_abseil that referenced this pull request Aug 4, 2024
Ralf W. Grosse-Kunstleve and others added 7 commits August 4, 2024 09:16
Bumps the actions group with 2 updates: [pypa/cibuildwheel](https://github.com/pypa/cibuildwheel) and [actions/attest-build-provenance](https://github.com/actions/attest-build-provenance).


Updates `pypa/cibuildwheel` from 2.19 to 2.20
- [Release notes](https://github.com/pypa/cibuildwheel/releases)
- [Changelog](https://github.com/pypa/cibuildwheel/blob/main/docs/changelog.md)
- [Commits](pypa/cibuildwheel@v2.19...v2.20)

Updates `actions/attest-build-provenance` from 1.3.3 to 1.4.0
- [Release notes](https://github.com/actions/attest-build-provenance/releases)
- [Changelog](https://github.com/actions/attest-build-provenance/blob/main/RELEASE.md)
- [Commits](actions/attest-build-provenance@5e9cb68...210c191)

---
updated-dependencies:
- dependency-name: pypa/cibuildwheel
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: actions
- dependency-name: actions/attest-build-provenance
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
updates:
- [github.com/astral-sh/ruff-pre-commit: v0.5.0 → v0.5.6](astral-sh/ruff-pre-commit@v0.5.0...v0.5.6)
- [github.com/pre-commit/mirrors-mypy: v1.10.1 → v1.11.1](pre-commit/mirrors-mypy@v1.10.1...v1.11.1)
- [github.com/PyCQA/pylint: v3.2.4 → v3.2.6](pylint-dev/pylint@v3.2.4...v3.2.6)
- [github.com/python-jsonschema/check-jsonschema: 0.28.6 → 0.29.1](python-jsonschema/check-jsonschema@0.28.6...0.29.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@rwgk rwgk changed the title WIP: git merge smart_holder after PR #5257 DEV: git merge smart_holder after PR #5257 Aug 9, 2024
@rwgk
Copy link
Contributor Author

rwgk commented Aug 9, 2024

The final state of this PR was manually squashed and merged under #30143.

@rwgk rwgk closed this Aug 9, 2024
@rwgk rwgk deleted the pybind11k_merge_sh_after_pr5257 branch August 9, 2024 17:50
This pull request was closed.
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.

2 participants