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

[smart_holder] Fix terrible merge accident: PYBIND11_INTERNALS_SH_DEF missing in PYBIND11_INTERNALS_ID and PYBIND11_MODULE_LOCAL_ID #5159

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Jun 10, 2024

Description

This PR fixes two oversights:

  1. A terrible merge accident introduced with [smart_holder] git merge master #5085: PYBIND11_INTERNALS_SH_DEF was accidentally removed from PYBIND11_INTERNALS_ID and PYBIND11_MODULE_LOCAL_ID. — How did this happen? First PR chore(deps): update pre-commit hooks #5084 on master introduced whitespace changes:

Those were incorrectly applied in smart_holder PR #5085:

  1. Simple change that was overlooked before: missing -DCMAKE_CXX_FLAGS="-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT" (manylinux)

Validation of both fixes:

ci.yml manylinux:

C++ Info: 13.2.1 20231014 C++17 __pybind11_internals_v5_gcc_libstdcpp_cxxabi1018__ PYBIND11_SIMPLE_GIL_MANAGEMENT=False PYBIND11_NUMPY_1_ONLY=False

ci_sh_def.yml manylinux:

C++ Info: 13.2.1 20231014 C++17 __pybind11_internals_v5_gcc_libstdcpp_cxxabi1018_sh_def__ PYBIND11_SIMPLE_GIL_MANAGEMENT=False PYBIND11_NUMPY_1_ONLY=False

Suggested changelog entry:

…_DEFAULT"` for new `manylinux` job (Manylinux on 🐍 3.13t • GIL).
@rwgk rwgk changed the title [smart_holder] Fix oversight: missing -DCMAKE_CXX_FLAGS="-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT" (manylinux) [smart_holder] Fix terrible merge accident: PYBIND11_INTERNALS_SH_DEF missing in PYBIND11_INTERNALS_ID and PYBIND11_MODULE_LOCAL_ID Jun 10, 2024
…TERNALS_SH_DEF` was accidentally removed from `PYBIND11_INTERNALS_ID` and `PYBIND11_MODULE_LOCAL_ID`.
@rwgk rwgk marked this pull request as ready for review June 11, 2024 01:20
@rwgk rwgk requested a review from henryiii as a code owner June 11, 2024 01:20
@rwgk rwgk merged commit 21d81fc into pybind:smart_holder Jun 11, 2024
150 checks passed
@rwgk rwgk deleted the sh_manylinux branch June 11, 2024 01:20
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jun 11, 2024
rwgk pushed a commit to google/pybind11clif that referenced this pull request Jun 11, 2024
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Jun 23, 2024
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